diff --git a/db/docs/batched_background_migrations/backfill_web_hook_logs_daily_sharding_keys.yml b/db/docs/batched_background_migrations/backfill_web_hook_logs_daily_sharding_keys.yml new file mode 100644 index 0000000000000000000000000000000000000000..a14529037fea39bbe734d5a83b2c9761b6773122 --- /dev/null +++ b/db/docs/batched_background_migrations/backfill_web_hook_logs_daily_sharding_keys.yml @@ -0,0 +1,7 @@ +--- +migration_job_name: BackfillWebHookLogsDailyShardingKeys +description: Backfill web_hook_logs_daily sharding key +feature_category: webhooks +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215236 +milestone: '18.7' +queued_migration_version: 20251202100012 diff --git a/db/docs/web_hook_logs_daily.yml b/db/docs/web_hook_logs_daily.yml index 1f152804d1bd5f5d3c2f88833850d8404738db30..ab6045654b6cabc4c47e9ff90e5b154dfcd0e45b 100644 --- a/db/docs/web_hook_logs_daily.yml +++ b/db/docs/web_hook_logs_daily.yml @@ -9,5 +9,8 @@ description: Webhooks logs data partitioned by day. introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/175379 milestone: '17.8' gitlab_schema: gitlab_main_org -sharding_key_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/524820 +sharding_key: + project_id: projects + group_id: namespaces + organization_id: organizations table_size: small diff --git a/db/post_migrate/20251202100004_add_index_web_hook_logs_daily_on_organization_id.rb b/db/post_migrate/20251202100004_add_index_web_hook_logs_daily_on_organization_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..aea4f3452932e850f96e7812d9519c3dc6a9898c --- /dev/null +++ b/db/post_migrate/20251202100004_add_index_web_hook_logs_daily_on_organization_id.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddIndexWebHookLogsDailyOnOrganizationId < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.7' + disable_ddl_transaction! + + INDEX_NAME = 'index_web_hook_logs_daily_on_organization_id' + + def up + # rubocop:disable Migration/PreventIndexCreation -- required for sharding + add_concurrent_partitioned_index :web_hook_logs_daily, :organization_id, name: INDEX_NAME + # rubocop:enable Migration/PreventIndexCreation + end + + def down + remove_concurrent_partitioned_index_by_name :web_hook_logs_daily, INDEX_NAME + end +end diff --git a/db/post_migrate/20251202100005_add_index_web_hook_logs_daily_on_project_id.rb b/db/post_migrate/20251202100005_add_index_web_hook_logs_daily_on_project_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..cd1f20151f704b4b7aef4d1102ef7d6b4423b7ec --- /dev/null +++ b/db/post_migrate/20251202100005_add_index_web_hook_logs_daily_on_project_id.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddIndexWebHookLogsDailyOnProjectId < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.7' + disable_ddl_transaction! + + INDEX_NAME = 'index_web_hook_logs_daily_on_project_id' + + def up + # rubocop:disable Migration/PreventIndexCreation -- required for sharding + add_concurrent_partitioned_index :web_hook_logs_daily, :project_id, name: INDEX_NAME + # rubocop:enable Migration/PreventIndexCreation + end + + def down + remove_concurrent_partitioned_index_by_name :web_hook_logs_daily, INDEX_NAME + end +end diff --git a/db/post_migrate/20251202100006_add_index_web_hook_logs_daily_on_group_id.rb b/db/post_migrate/20251202100006_add_index_web_hook_logs_daily_on_group_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..9aa9e07e6b542c7665a20f33f437e6cf61bd1345 --- /dev/null +++ b/db/post_migrate/20251202100006_add_index_web_hook_logs_daily_on_group_id.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddIndexWebHookLogsDailyOnGroupId < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.7' + disable_ddl_transaction! + + INDEX_NAME = 'index_web_hook_logs_daily_on_group_id' + + def up + # rubocop:disable Migration/PreventIndexCreation -- required for sharding + add_concurrent_partitioned_index :web_hook_logs_daily, :group_id, name: INDEX_NAME + # rubocop:enable Migration/PreventIndexCreation + end + + def down + remove_concurrent_partitioned_index_by_name :web_hook_logs_daily, INDEX_NAME + end +end diff --git a/db/post_migrate/20251202100007_add_fk_web_hook_logs_daily_organization_id.rb b/db/post_migrate/20251202100007_add_fk_web_hook_logs_daily_organization_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..c5d74c7ded876257b5ce06df9aa90f69def3d7e1 --- /dev/null +++ b/db/post_migrate/20251202100007_add_fk_web_hook_logs_daily_organization_id.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddFkWebHookLogsDailyOrganizationId < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.7' + disable_ddl_transaction! + + def up + add_concurrent_partitioned_foreign_key( + :web_hook_logs_daily, + :organizations, + column: :organization_id, + on_delete: :cascade, + validate: false, + reverse_lock_order: true + ) + end + + def down + remove_partitioned_foreign_key :web_hook_logs_daily, column: :organization_id, reverse_lock_order: true + end +end diff --git a/db/post_migrate/20251202100008_add_fk_web_hook_logs_daily_project_id.rb b/db/post_migrate/20251202100008_add_fk_web_hook_logs_daily_project_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..64be68f7bfcec9bbbcbc021f6ffcd9615cfb143d --- /dev/null +++ b/db/post_migrate/20251202100008_add_fk_web_hook_logs_daily_project_id.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddFkWebHookLogsDailyProjectId < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.7' + disable_ddl_transaction! + + def up + add_concurrent_partitioned_foreign_key( + :web_hook_logs_daily, + :projects, + column: :project_id, + on_delete: :cascade, + validate: false, + reverse_lock_order: true + ) + end + + def down + remove_partitioned_foreign_key :web_hook_logs_daily, column: :project_id, reverse_lock_order: true + end +end diff --git a/db/post_migrate/20251202100009_add_fk_web_hook_logs_daily_group_id.rb b/db/post_migrate/20251202100009_add_fk_web_hook_logs_daily_group_id.rb new file mode 100644 index 0000000000000000000000000000000000000000..222f28141d81f65ad1600c99abdee6d4fbc092a6 --- /dev/null +++ b/db/post_migrate/20251202100009_add_fk_web_hook_logs_daily_group_id.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class AddFkWebHookLogsDailyGroupId < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + + milestone '18.7' + disable_ddl_transaction! + + def up + add_concurrent_partitioned_foreign_key( + :web_hook_logs_daily, + :namespaces, + column: :group_id, + on_delete: :cascade, + validate: false, + reverse_lock_order: true + ) + end + + def down + remove_partitioned_foreign_key :web_hook_logs_daily, column: :group_id, reverse_lock_order: true + end +end diff --git a/db/post_migrate/20251202100010_add_sharding_key_trigger_to_web_hook_logs_daily.rb b/db/post_migrate/20251202100010_add_sharding_key_trigger_to_web_hook_logs_daily.rb new file mode 100644 index 0000000000000000000000000000000000000000..ef76d972dcee958d65a3fcba2bc2357f869b97f4 --- /dev/null +++ b/db/post_migrate/20251202100010_add_sharding_key_trigger_to_web_hook_logs_daily.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class AddShardingKeyTriggerToWebHookLogsDaily < Gitlab::Database::Migration[2.3] + include Gitlab::Database::SchemaHelpers + + milestone '18.7' + + TABLE_NAME = :web_hook_logs_daily + PARENT_TABLE = :web_hooks + FOREIGN_KEY = :web_hook_id + TRIGGER_NAME = 'trigger_web_hook_logs_daily_assign_sharding_keys' + + def up + create_trigger_function(TRIGGER_NAME) do + <<~SQL + IF num_nonnulls(NEW.organization_id, NEW.project_id, NEW.group_id) <> 1 THEN + SELECT organization_id, project_id, group_id + INTO NEW.organization_id, NEW.project_id, NEW.group_id + FROM web_hooks + WHERE web_hooks.id = NEW.web_hook_id; + END IF; + + RETURN NEW; + SQL + end + + create_trigger(TABLE_NAME, TRIGGER_NAME, TRIGGER_NAME, fires: 'BEFORE INSERT OR UPDATE') + end + + def down + drop_trigger(TABLE_NAME, TRIGGER_NAME) + drop_function(TRIGGER_NAME) + end +end diff --git a/db/post_migrate/20251202100011_add_not_null_constraint_to_web_hook_logs_daily_sharding_keys.rb b/db/post_migrate/20251202100011_add_not_null_constraint_to_web_hook_logs_daily_sharding_keys.rb new file mode 100644 index 0000000000000000000000000000000000000000..a57d37dd2ccd944908b910bcef866a4d1e3b85fb --- /dev/null +++ b/db/post_migrate/20251202100011_add_not_null_constraint_to_web_hook_logs_daily_sharding_keys.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class AddNotNullConstraintToWebHookLogsDailyShardingKeys < Gitlab::Database::Migration[2.3] + milestone '18.7' + disable_ddl_transaction! + + def up + add_multi_column_not_null_constraint( + :web_hook_logs_daily, + :organization_id, + :project_id, + :group_id, + validate: false + ) + end + + def down + remove_multi_column_not_null_constraint( + :web_hook_logs_daily, + :organization_id, + :project_id, + :group_id + ) + end +end diff --git a/db/post_migrate/20251202100012_queue_backfill_web_hook_logs_daily_sharding_keys.rb b/db/post_migrate/20251202100012_queue_backfill_web_hook_logs_daily_sharding_keys.rb new file mode 100644 index 0000000000000000000000000000000000000000..a02b57295c0d971abb12347a3021e8bf75380fb9 --- /dev/null +++ b/db/post_migrate/20251202100012_queue_backfill_web_hook_logs_daily_sharding_keys.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class QueueBackfillWebHookLogsDailyShardingKeys < Gitlab::Database::Migration[2.3] + milestone '18.7' + + restrict_gitlab_migration gitlab_schema: :gitlab_main_org + + MIGRATION = "BackfillWebHookLogsDailyShardingKeys" + BATCH_SIZE = 10_000 + SUB_BATCH_SIZE = 500 + + def up + return if Gitlab.com_except_jh? + + queue_batched_background_migration( + MIGRATION, + :web_hook_logs_daily, + :id, + batch_size: BATCH_SIZE, + sub_batch_size: SUB_BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :web_hook_logs_daily, :id, []) + end +end diff --git a/db/schema_migrations/20251202100001 b/db/schema_migrations/20251202100001 deleted file mode 100644 index b4e4103110cda868ef7d58ab207223dcececf959..0000000000000000000000000000000000000000 --- a/db/schema_migrations/20251202100001 +++ /dev/null @@ -1 +0,0 @@ -bf0118b1987e962bedcb480aa858efc2c24c310cfb1c35367645122f356c3b6c \ No newline at end of file diff --git a/db/schema_migrations/20251202100004 b/db/schema_migrations/20251202100004 new file mode 100644 index 0000000000000000000000000000000000000000..d7f627f43e1afa543ea42d2cf2de6db9d9541438 --- /dev/null +++ b/db/schema_migrations/20251202100004 @@ -0,0 +1 @@ +6bfa533b252c2930cb2c4e0ff5851f9a37998b0a9c220b3fe96c3dd3978098e5 \ No newline at end of file diff --git a/db/schema_migrations/20251202100005 b/db/schema_migrations/20251202100005 new file mode 100644 index 0000000000000000000000000000000000000000..48af32b34c36b4790fbf4c974d1a2ec5ae199339 --- /dev/null +++ b/db/schema_migrations/20251202100005 @@ -0,0 +1 @@ +226999b5fb2195256992d8659c668c82de4bd4f3013ad82fc1e8470d5e0154c0 \ No newline at end of file diff --git a/db/schema_migrations/20251202100006 b/db/schema_migrations/20251202100006 new file mode 100644 index 0000000000000000000000000000000000000000..b425420512da0bb6a092ea9c384e62065420012f --- /dev/null +++ b/db/schema_migrations/20251202100006 @@ -0,0 +1 @@ +bb772c0dac78a98dc08a8aaad358b70879c41f3c998f0614155e036519cc12a4 \ No newline at end of file diff --git a/db/schema_migrations/20251202100007 b/db/schema_migrations/20251202100007 new file mode 100644 index 0000000000000000000000000000000000000000..9ffb4a9953e6760de39b2c097971f4382cc400bf --- /dev/null +++ b/db/schema_migrations/20251202100007 @@ -0,0 +1 @@ +8a1d71d79c3b77c3bd3db154dbbef071e13a2e90910e5738e6a134dbced4cac9 \ No newline at end of file diff --git a/db/schema_migrations/20251202100008 b/db/schema_migrations/20251202100008 new file mode 100644 index 0000000000000000000000000000000000000000..4500fab902283f53b407323c32c02e10b63ef564 --- /dev/null +++ b/db/schema_migrations/20251202100008 @@ -0,0 +1 @@ +f67e4b79c7cae82af0fa0534f6d9cc915d4999cce1025d4f7ab557190c66d525 \ No newline at end of file diff --git a/db/schema_migrations/20251202100009 b/db/schema_migrations/20251202100009 new file mode 100644 index 0000000000000000000000000000000000000000..92ff5863db799c4a2e035dc0b1a7281691991f7a --- /dev/null +++ b/db/schema_migrations/20251202100009 @@ -0,0 +1 @@ +7a00a7b7c76350638885ee700225d818415a62b5b24c1688bc4adf7621e66ffa \ No newline at end of file diff --git a/db/schema_migrations/20251202100010 b/db/schema_migrations/20251202100010 new file mode 100644 index 0000000000000000000000000000000000000000..b26f99d0679a19c138fe5e5bac0c2d8a2d6ceec0 --- /dev/null +++ b/db/schema_migrations/20251202100010 @@ -0,0 +1 @@ +1496cf40bbcb3b9bc484b5043f0d3bcff0218d9b6c707b131ec96f21cc3b622e \ No newline at end of file diff --git a/db/schema_migrations/20251202100011 b/db/schema_migrations/20251202100011 new file mode 100644 index 0000000000000000000000000000000000000000..685a4d932df295fb16e3a6f5c38181adf9196dcd --- /dev/null +++ b/db/schema_migrations/20251202100011 @@ -0,0 +1 @@ +31cf44e2f4f4dd77829bffaec4db85e52be7f891aa818ea7956b519b6099fb29 \ No newline at end of file diff --git a/db/schema_migrations/20251202100012 b/db/schema_migrations/20251202100012 new file mode 100644 index 0000000000000000000000000000000000000000..cea00561b51aea994af09d47a59d5485eacd35f5 --- /dev/null +++ b/db/schema_migrations/20251202100012 @@ -0,0 +1 @@ +49865bde2c2d4fca03c2a4623a6629a690942d8e67394ff9bcb329ff57e949dc \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 6746648605376e984366abac822589d883ff7cbb..8efaceddd30b2f7eb2ba7b33114beca9af57decd 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5064,6 +5064,22 @@ RETURN NEW; END $$; +CREATE FUNCTION trigger_web_hook_logs_daily_assign_sharding_keys() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN +IF num_nonnulls(NEW.organization_id, NEW.project_id, NEW.group_id) <> 1 THEN + SELECT organization_id, project_id, group_id + INTO NEW.organization_id, NEW.project_id, NEW.group_id + FROM web_hooks + WHERE web_hooks.id = NEW.web_hook_id; +END IF; + +RETURN NEW; + +END +$$; + CREATE FUNCTION unset_has_issues_on_vulnerability_reads() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -35057,6 +35073,9 @@ ALTER TABLE epic_issues ALTER TABLE bulk_import_batch_trackers ADD CONSTRAINT check_13004cd9a8 CHECK ((num_nonnulls(namespace_id, organization_id, project_id) = 1)) NOT VALID; +ALTER TABLE web_hook_logs_daily + ADD CONSTRAINT check_19dc80d658 CHECK ((num_nonnulls(group_id, organization_id, project_id) = 1)) NOT VALID; + ALTER TABLE workspaces ADD CONSTRAINT check_2a89035b04 CHECK ((personal_access_token_id IS NOT NULL)) NOT VALID; @@ -45229,6 +45248,12 @@ CREATE UNIQUE INDEX index_vulns_user_mentions_on_vulnerability_id ON vulnerabili CREATE UNIQUE INDEX index_vulns_user_mentions_on_vulnerability_id_and_note_id ON vulnerability_user_mentions USING btree (vulnerability_id, note_id); +CREATE INDEX index_web_hook_logs_daily_on_group_id ON ONLY web_hook_logs_daily USING btree (group_id); + +CREATE INDEX index_web_hook_logs_daily_on_organization_id ON ONLY web_hook_logs_daily USING btree (organization_id); + +CREATE INDEX index_web_hook_logs_daily_on_project_id ON ONLY web_hook_logs_daily USING btree (project_id); + CREATE INDEX index_web_hook_logs_daily_on_web_hook_id_and_created_at ON ONLY web_hook_logs_daily USING btree (web_hook_id, created_at); CREATE INDEX index_web_hook_logs_daily_part_on_created_at_and_web_hook_id ON ONLY web_hook_logs_daily USING btree (created_at, web_hook_id); @@ -49807,6 +49832,8 @@ CREATE TRIGGER trigger_update_location_on_vulnerability_occurrences_update AFTER CREATE TRIGGER trigger_update_vulnerability_reads_on_vulnerability_update AFTER UPDATE ON vulnerabilities FOR EACH ROW WHEN (((old.present_on_default_branch IS TRUE) AND ((old.severity IS DISTINCT FROM new.severity) OR (old.state IS DISTINCT FROM new.state) OR (old.resolved_on_default_branch IS DISTINCT FROM new.resolved_on_default_branch)))) EXECUTE FUNCTION update_vulnerability_reads_from_vulnerability(); +CREATE TRIGGER trigger_web_hook_logs_daily_assign_sharding_keys BEFORE INSERT OR UPDATE ON web_hook_logs_daily FOR EACH ROW EXECUTE FUNCTION trigger_web_hook_logs_daily_assign_sharding_keys(); + CREATE TRIGGER users_loose_fk_trigger AFTER DELETE ON users REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); CREATE TRIGGER virtual_registries_container_upstreams_loose_fk_trigger AFTER DELETE ON virtual_registries_container_upstreams REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); diff --git a/lib/gitlab/background_migration/backfill_web_hook_logs_daily_sharding_keys.rb b/lib/gitlab/background_migration/backfill_web_hook_logs_daily_sharding_keys.rb new file mode 100644 index 0000000000000000000000000000000000000000..94bcf66677d2092b304ece9c0cf4fa834b85c4e8 --- /dev/null +++ b/lib/gitlab/background_migration/backfill_web_hook_logs_daily_sharding_keys.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + class BackfillWebHookLogsDailyShardingKeys < BatchedMigrationJob + operation_name :backfill_web_hook_logs_daily_sharding_keys + feature_category :webhooks + + def perform + each_sub_batch do |sub_batch| + records_to_update = sub_batch.where( + organization_id: nil, + project_id: nil, + group_id: nil + ) + + connection.execute(update_sql(records_to_update)) + end + end + + private + + def update_sql(sub_batch) + <<~SQL + UPDATE + web_hook_logs_daily + SET + organization_id = web_hooks.organization_id, + project_id = web_hooks.project_id, + group_id = web_hooks.group_id + FROM + web_hooks + WHERE + web_hooks.id = web_hook_logs_daily.web_hook_id + AND + web_hook_logs_daily.id IN (#{sub_batch.select(:id).to_sql}) + SQL + end + end + end +end diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 6a231a38f810957e6b9368bea38c99fe7b403079..8c8de59bc71fb6966c213bb5a69c48ef818c042f 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -247,7 +247,7 @@ # See: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87584 # Fixes performance issues with the deletion of web-hooks with many log entries web_hook_logs: %w[web_hook_id], - web_hook_logs_daily: %w[web_hook_id], + web_hook_logs_daily: %w[web_hook_id organization_id group_id project_id], ml_candidates: %w[internal_id], value_stream_dashboard_counts: %w[namespace_id], vulnerability_export_parts: %w[start_id end_id], diff --git a/spec/lib/gitlab/background_migration/backfill_web_hook_logs_daily_sharding_keys_spec.rb b/spec/lib/gitlab/background_migration/backfill_web_hook_logs_daily_sharding_keys_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..9542f82d8d52c72bd9b131c1da231663bbdaf943 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_web_hook_logs_daily_sharding_keys_spec.rb @@ -0,0 +1,196 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillWebHookLogsDailyShardingKeys, feature_category: :webhooks do + let(:connection) { ApplicationRecord.connection } + let(:function_name) { 'trigger_web_hook_logs_daily_assign_sharding_keys' } + let(:trigger_name) { function_name } + let(:constraint_name) { 'check_19dc80d658' } + + let(:organizations) { table(:organizations) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:web_hooks) { table(:web_hooks) } + let(:web_hook_logs_daily) { table(:web_hook_logs_daily) } + + let(:start_id) { web_hook_logs_daily.minimum(:id) } + let(:end_id) { web_hook_logs_daily.maximum(:id) } + + let!(:organization) { organizations.create!(name: 'Default', path: 'default') } + + let!(:group) do + namespaces.create!( + name: 'test-group', + path: 'test-group', + type: 'Group', + organization_id: organization.id + ) + end + + let!(:project_namespace) do + namespaces.create!( + name: 'test-project', + path: 'test-project', + organization_id: organization.id + ) + end + + let!(:project) do + projects.create!( + name: 'test-project', + path: 'test-project', + organization_id: organization.id, + namespace_id: group.id, + project_namespace_id: project_namespace.id + ) + end + + let!(:system_hook) do + web_hooks.create!( + type: 'SystemHook', + organization_id: organization.id + ) + end + + let!(:project_hook) do + web_hooks.create!( + type: 'ProjectHook', + project_id: project.id + ) + end + + let!(:group_hook) do + web_hooks.create!( + type: 'GroupHook', + group_id: group.id + ) + end + + let!(:system_log) do + drop_constraint_and_trigger + record = web_hook_logs_daily.create!( + web_hook_id: system_hook.id, + trigger: 'push_hooks', + url: 'http://example.com', + request_headers: {}, + request_data: {}, + response_headers: {}, + response_body: '', + response_status: '200', + execution_duration: 0.1, + internal_error_message: '' + ) + add_constraint_and_trigger + record + end + + let!(:project_log) do + drop_constraint_and_trigger + record = web_hook_logs_daily.create!( + web_hook_id: project_hook.id, + trigger: 'push_hooks', + url: 'http://example.com', + request_headers: {}, + request_data: {}, + response_headers: {}, + response_body: '', + response_status: '200', + execution_duration: 0.1, + internal_error_message: '' + ) + add_constraint_and_trigger + record + end + + let!(:group_log) do + drop_constraint_and_trigger + record = web_hook_logs_daily.create!( + web_hook_id: group_hook.id, + trigger: 'push_hooks', + url: 'http://example.com', + request_headers: {}, + request_data: {}, + response_headers: {}, + response_body: '', + response_status: '200', + execution_duration: 0.1, + internal_error_message: '' + ) + add_constraint_and_trigger + record + end + + subject(:migration) do + described_class.new( + start_id: start_id, + end_id: end_id, + batch_table: :web_hook_logs_daily, + batch_column: :id, + sub_batch_size: 2, + pause_ms: 0, + connection: ApplicationRecord.connection + ) + end + + describe '#perform' do + it 'backfills sharding keys from web_hooks table' do + expect(system_log.reload.organization_id).to be_nil + expect(system_log.reload.project_id).to be_nil + expect(system_log.reload.group_id).to be_nil + + expect(project_log.reload.organization_id).to be_nil + expect(project_log.reload.project_id).to be_nil + expect(project_log.reload.group_id).to be_nil + + expect(group_log.reload.organization_id).to be_nil + expect(group_log.reload.project_id).to be_nil + expect(group_log.reload.group_id).to be_nil + + migration.perform + + expect(system_log.reload.organization_id).to eq(organization.id) + expect(system_log.reload.project_id).to be_nil + expect(system_log.reload.group_id).to be_nil + + expect(project_log.reload.project_id).to eq(project.id) + expect(project_log.reload.organization_id).to be_nil + expect(project_log.reload.group_id).to be_nil + + expect(group_log.reload.group_id).to eq(group.id) + expect(group_log.reload.organization_id).to be_nil + expect(group_log.reload.project_id).to be_nil + end + + it 'does not update records that already have sharding keys' do + # Manually set sharding keys + system_log.update!(organization_id: organization.id) + project_log.update!(project_id: project.id) + group_log.update!(group_id: group.id) + + expect { migration.perform }.not_to change { system_log.reload.updated_at } + end + end + + private + + def drop_constraint_and_trigger + connection.execute( + <<~SQL + DROP TRIGGER IF EXISTS #{trigger_name} ON web_hook_logs_daily; + + ALTER TABLE web_hook_logs_daily DROP CONSTRAINT IF EXISTS #{constraint_name}; + SQL + ) + end + + def add_constraint_and_trigger + connection.execute( + <<~SQL + ALTER TABLE web_hook_logs_daily ADD CONSTRAINT #{constraint_name} CHECK ((num_nonnulls(group_id, organization_id, project_id) = 1)) NOT VALID; + + CREATE TRIGGER #{trigger_name} BEFORE INSERT OR UPDATE ON web_hook_logs_daily FOR EACH ROW EXECUTE FUNCTION #{function_name}(); + SQL + ) + end +end diff --git a/spec/lib/gitlab/organizations/sharding_key_spec.rb b/spec/lib/gitlab/organizations/sharding_key_spec.rb index f37a510e271a37e187065b25452a48f4d3b6c67b..d7d5a0a82282b75329025058cda8c9e4d4a21626 100644 --- a/spec/lib/gitlab/organizations/sharding_key_spec.rb +++ b/spec/lib/gitlab/organizations/sharding_key_spec.rb @@ -10,7 +10,6 @@ let(:allowed_to_be_missing_sharding_key) do [ 'ai_settings', # https://gitlab.com/gitlab-org/gitlab/-/issues/531356 - 'web_hook_logs_daily', # temporary copy of web_hook_logs 'uploads_9ba88c4165', # https://gitlab.com/gitlab-org/gitlab/-/issues/398199 'merge_request_diff_files_99208b8fac', # has a desired sharding key instead 'award_emoji_archived', # temp table: https://gitlab.com/gitlab-org/gitlab/-/issues/580326 @@ -49,6 +48,11 @@ bulk_import_batch_trackers.namespace_id bulk_import_batch_trackers.project_id ], # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/213933 + *%w[ + web_hook_logs_daily.organization_id + web_hook_logs_daily.project_id + web_hook_logs_daily.group_id + ], # https://gitlab.com/gitlab-org/gitlab/-/work_items/524820 'security_scans.project_id', # NOT NULL constraint NOT VALID 'keys.organization_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/577246 'oauth_applications.organization_id' # https://gitlab.com/gitlab-org/gitlab/-/issues/579291 @@ -106,7 +110,11 @@ 'merge_request_commits_metadata.project_id', 'sbom_vulnerability_scans.project_id', 'p_duo_workflows_checkpoints.project_id', - 'p_duo_workflows_checkpoints.namespace_id' + 'p_duo_workflows_checkpoints.namespace_id', + # `web_hook_logs_daily` -> https://gitlab.com/gitlab-org/gitlab/-/work_items/524820 + 'web_hook_logs_daily.organization_id', + 'web_hook_logs_daily.group_id', + 'web_hook_logs_daily.project_id' ] end @@ -257,6 +265,7 @@ "fork_networks" => "https://gitlab.com/gitlab-org/gitlab/-/issues/522958", "bulk_import_configurations" => "https://gitlab.com/gitlab-org/gitlab/-/issues/536521", "pool_repositories" => "https://gitlab.com/gitlab-org/gitlab/-/issues/490484", + "web_hook_logs_daily" => "https://gitlab.com/gitlab-org/gitlab/-/work_items/524820", # All the tables below related to uploads are part of the same work to # add sharding key to the table "admin_roles" => "https://gitlab.com/gitlab-org/gitlab/-/issues/553437", diff --git a/spec/migrations/add_sharding_key_trigger_to_web_hook_logs_daily_spec.rb b/spec/migrations/add_sharding_key_trigger_to_web_hook_logs_daily_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..4590a34e2116127165f80ecb0197fcb02482490d --- /dev/null +++ b/spec/migrations/add_sharding_key_trigger_to_web_hook_logs_daily_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require_migration! + +RSpec.describe AddShardingKeyTriggerToWebHookLogsDaily, :migration, feature_category: :webhooks do + let(:organizations) { table(:organizations) } + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:web_hooks) { table(:web_hooks) } + let(:web_hook_logs_daily) { table(:web_hook_logs_daily) } + + let!(:organization) { organizations.create!(name: 'Default', path: 'default') } + + let!(:group) do + namespaces.create!( + name: 'test-group', + path: 'test-group', + type: 'Group', + organization_id: organization.id + ) + end + + let!(:project_namespace) do + namespaces.create!( + name: 'test-project', + path: 'test-project', + organization_id: organization.id + ) + end + + let!(:project) do + projects.create!( + name: 'test-project', + path: 'test-project', + organization_id: organization.id, + namespace_id: group.id, + project_namespace_id: project_namespace.id + ) + end + + let!(:system_hook) do + web_hooks.create!( + type: 'SystemHook', + organization_id: organization.id + ) + end + + let!(:project_hook) do + web_hooks.create!( + type: 'ProjectHook', + project_id: project.id + ) + end + + let!(:group_hook) do + web_hooks.create!( + type: 'GroupHook', + group_id: group.id + ) + end + + describe '#up' do + it 'installs triggers that assign sharding keys from web_hooks table' do + migrate! + + # Create log for system hook + system_log = web_hook_logs_daily.create!( + web_hook_id: system_hook.id, + trigger: 'push_hooks', + url: 'http://example.com', + request_headers: {}, + request_data: {}, + response_headers: {}, + response_body: '', + response_status: '200', + execution_duration: 0.1, + internal_error_message: '' + ) + + # Create log for project hook + project_log = web_hook_logs_daily.create!( + web_hook_id: project_hook.id, + trigger: 'push_hooks', + url: 'http://example.com', + request_headers: {}, + request_data: {}, + response_headers: {}, + response_body: '', + response_status: '200', + execution_duration: 0.1, + internal_error_message: '' + ) + + # Create log for group hook + group_log = web_hook_logs_daily.create!( + web_hook_id: group_hook.id, + trigger: 'push_hooks', + url: 'http://example.com', + request_headers: {}, + request_data: {}, + response_headers: {}, + response_body: '', + response_status: '200', + execution_duration: 0.1, + internal_error_message: '' + ) + + # Verify sharding keys were assigned correctly + expect(system_log.reload.organization_id).to eq(organization.id) + expect(system_log.reload.project_id).to be_nil + expect(system_log.reload.group_id).to be_nil + + expect(project_log.reload.project_id).to eq(project.id) + expect(project_log.reload.organization_id).to be_nil + expect(project_log.reload.group_id).to be_nil + + expect(group_log.reload.group_id).to eq(group.id) + expect(group_log.reload.organization_id).to be_nil + expect(group_log.reload.project_id).to be_nil + end + end +end diff --git a/spec/migrations/queue_backfill_web_hook_logs_daily_sharding_keys_spec.rb b/spec/migrations/queue_backfill_web_hook_logs_daily_sharding_keys_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..c58459e1479b414c7e9ddb5de81c68b3ad96ab45 --- /dev/null +++ b/spec/migrations/queue_backfill_web_hook_logs_daily_sharding_keys_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueBackfillWebHookLogsDailyShardingKeys, migration: :gitlab_main_org, feature_category: :webhooks do + let!(:batched_migration) { described_class::MIGRATION } + + context 'when not on GitLab.com' do + before do + allow(Gitlab).to receive(:com_except_jh?).and_return(false) + end + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + gitlab_schema: :gitlab_main_org, + table_name: :web_hook_logs_daily, + column_name: :id, + batch_size: described_class::BATCH_SIZE, + sub_batch_size: described_class::SUB_BATCH_SIZE + ) + } + end + end + end + + context 'when on GitLab.com' do + before do + allow(Gitlab).to receive(:com_except_jh?).and_return(true) + end + + it 'does not schedule a batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + end + end + end +end