From 8f6924a6567c9ab4348a289496801a007d979186 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Thu, 18 Sep 2025 17:33:37 -0500 Subject: [PATCH 1/3] Add system_note_metadata sharding key - Add sharding key columns - Add trigger to populate sharding key - Add NOT VALID NOT NULL constraint - Schedule sharding key indexes to be created ASYNC Changelog: other --- app/models/system_note_metadata.rb | 2 +- ...dd_sharding_key_to_system_note_metadata.rb | 12 ++++ ...ing_key_trigger_on_system_note_metadata.rb | 55 +++++++++++++++++++ ...nt_on_system_note_metadata_sharding_key.rb | 17 ++++++ ...y_indexes_on_system_note_metadata_async.rb | 20 +++++++ db/schema_migrations/20250918203528 | 1 + db/schema_migrations/20250918211138 | 1 + db/schema_migrations/20250918221702 | 1 + db/schema_migrations/20250918222201 | 1 + db/structure.sql | 42 +++++++++++++- spec/db/schema_spec.rb | 1 + spec/models/system_note_metadata_spec.rb | 26 +++++++++ 12 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20250918203528_add_sharding_key_to_system_note_metadata.rb create mode 100644 db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb create mode 100644 db/post_migrate/20250918221702_add_not_null_constraint_on_system_note_metadata_sharding_key.rb create mode 100644 db/post_migrate/20250918222201_add_sharding_key_indexes_on_system_note_metadata_async.rb create mode 100644 db/schema_migrations/20250918203528 create mode 100644 db/schema_migrations/20250918211138 create mode 100644 db/schema_migrations/20250918221702 create mode 100644 db/schema_migrations/20250918222201 diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index e3d0fef546cfb5..cb9093412c5485 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -71,4 +71,4 @@ def cross_reference_types end end -SystemNoteMetadata.prepend_mod_with('SystemNoteMetadata') +SystemNoteMetadata.prepend_mod diff --git a/db/migrate/20250918203528_add_sharding_key_to_system_note_metadata.rb b/db/migrate/20250918203528_add_sharding_key_to_system_note_metadata.rb new file mode 100644 index 00000000000000..a187045d878460 --- /dev/null +++ b/db/migrate/20250918203528_add_sharding_key_to_system_note_metadata.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +class AddShardingKeyToSystemNoteMetadata < Gitlab::Database::Migration[2.3] + milestone '18.5' + + def change + # rubocop:disable Migration/PreventAddingColumns -- Sharding key is an exception + add_column :system_note_metadata, :namespace_id, :bigint + add_column :system_note_metadata, :organization_id, :bigint + # rubocop:enable Migration/PreventAddingColumns + end +end diff --git a/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb b/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb new file mode 100644 index 00000000000000..99b8f4beab6640 --- /dev/null +++ b/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class AddShardingKeyTriggerOnSystemNoteMetadata < Gitlab::Database::Migration[2.3] + include Gitlab::Database::SchemaHelpers + + FUNCTION_NAME = 'set_sharding_key_for_system_note_metadata' + TRIGGER_NAME = 'set_sharding_key_for_system_note_metadata_on_insert' + + milestone '18.5' + + def up + execute(<<~SQL) + CREATE OR REPLACE FUNCTION #{FUNCTION_NAME}() + RETURNS TRIGGER AS + $$ + DECLARE + note_organization_id BIGINT; + note_project_id BIGINT; + note_namespace_id BIGINT; + BEGIN + IF NEW."note_id" IS NULL THEN + RETURN NEW; + END IF; + + SELECT "organization_id", "project_id", "namespace_id" + INTO note_organization_id, note_project_id, note_namespace_id + FROM "notes" + WHERE "id" = NEW."note_id"; + + IF note_organization_id IS NOT NULL THEN + NEW."organization_id" := note_organization_id; + NEW."namespace_id" := NULL; + ELSIF note_project_id IS NOT NULL THEN + SELECT "project_namespace_id" FROM "projects" + INTO NEW."namespace_id" WHERE "projects"."id" = note_project_id; + NEW."organization_id" := NULL; + ELSE + NEW."namespace_id" := note_namespace_id; + NEW."organization_id" := NULL; + END IF; + + RETURN NEW; + END + $$ LANGUAGE PLPGSQL; + SQL + + create_trigger(:system_note_metadata, TRIGGER_NAME, FUNCTION_NAME, fires: 'BEFORE INSERT') + end + + def down + drop_trigger(:system_note_metadata, TRIGGER_NAME) + + drop_function(FUNCTION_NAME) + end +end diff --git a/db/post_migrate/20250918221702_add_not_null_constraint_on_system_note_metadata_sharding_key.rb b/db/post_migrate/20250918221702_add_not_null_constraint_on_system_note_metadata_sharding_key.rb new file mode 100644 index 00000000000000..16e0f503cf5d11 --- /dev/null +++ b/db/post_migrate/20250918221702_add_not_null_constraint_on_system_note_metadata_sharding_key.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddNotNullConstraintOnSystemNoteMetadataShardingKey < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.5' + + def up + add_multi_column_not_null_constraint :system_note_metadata, + :namespace_id, + :organization_id, + validate: false + end + + def down + remove_multi_column_not_null_constraint :system_note_metadata, :namespace_id, :organization_id + end +end diff --git a/db/post_migrate/20250918222201_add_sharding_key_indexes_on_system_note_metadata_async.rb b/db/post_migrate/20250918222201_add_sharding_key_indexes_on_system_note_metadata_async.rb new file mode 100644 index 00000000000000..c7942f04e60219 --- /dev/null +++ b/db/post_migrate/20250918222201_add_sharding_key_indexes_on_system_note_metadata_async.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddShardingKeyIndexesOnSystemNoteMetadataAsync < Gitlab::Database::Migration[2.3] + NAMESPACE_INDEX = 'index_system_note_metadata_on_namespace_id' + ORGANIZATION_INDEX = 'index_system_note_metadata_on_organization_id' + + milestone '18.5' + + def up + # rubocop:disable Migration/PreventIndexCreation -- Sharding key is an exception + prepare_async_index :system_note_metadata, :namespace_id, name: NAMESPACE_INDEX + prepare_async_index :system_note_metadata, :organization_id, name: ORGANIZATION_INDEX + # rubocop:enable Migration/PreventIndexCreation + end + + def down + unprepare_async_index :system_note_metadata, :namespace_id, name: NAMESPACE_INDEX + unprepare_async_index :system_note_metadata, :organization_id, name: ORGANIZATION_INDEX + end +end diff --git a/db/schema_migrations/20250918203528 b/db/schema_migrations/20250918203528 new file mode 100644 index 00000000000000..6373b814cb0075 --- /dev/null +++ b/db/schema_migrations/20250918203528 @@ -0,0 +1 @@ +f487a5b0dbb9f19a497d45298beef4be958b97985bf8841c3d77094658afd345 \ No newline at end of file diff --git a/db/schema_migrations/20250918211138 b/db/schema_migrations/20250918211138 new file mode 100644 index 00000000000000..fd594bb6851242 --- /dev/null +++ b/db/schema_migrations/20250918211138 @@ -0,0 +1 @@ +266ec6e26909ee142252e2a45c741d0e9aa096246c2247ff56ce783be94a71ad \ No newline at end of file diff --git a/db/schema_migrations/20250918221702 b/db/schema_migrations/20250918221702 new file mode 100644 index 00000000000000..9d6c0ab8c25ce9 --- /dev/null +++ b/db/schema_migrations/20250918221702 @@ -0,0 +1 @@ +763efab11d47b78d9cd28e22d5cdf96b14b9d7c6a006b621b065ea085e17cc23 \ No newline at end of file diff --git a/db/schema_migrations/20250918222201 b/db/schema_migrations/20250918222201 new file mode 100644 index 00000000000000..5cfdbed0aebe7a --- /dev/null +++ b/db/schema_migrations/20250918222201 @@ -0,0 +1 @@ +393eb5bf8df14469f8539edeb5baf698471ccad53b7712b897d7cb4a1d0cbd24 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1dcae9fa5ab71f..0d2fb5e55d5d1f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -777,6 +777,39 @@ RETURN NULL; END $$; +CREATE FUNCTION set_sharding_key_for_system_note_metadata() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE + note_organization_id BIGINT; + note_project_id BIGINT; + note_namespace_id BIGINT; +BEGIN + IF NEW."note_id" IS NULL THEN + RETURN NEW; + END IF; + + SELECT "organization_id", "project_id", "namespace_id" + INTO note_organization_id, note_project_id, note_namespace_id + FROM "notes" + WHERE "id" = NEW."note_id"; + + IF note_organization_id IS NOT NULL THEN + NEW."organization_id" := note_organization_id; + NEW."namespace_id" := NULL; + ELSIF note_project_id IS NOT NULL THEN + SELECT "project_namespace_id" FROM "projects" + INTO NEW."namespace_id" WHERE "projects"."id" = note_project_id; + NEW."organization_id" := NULL; + ELSE + NEW."namespace_id" := note_namespace_id; + NEW."organization_id" := NULL; + END IF; + + RETURN NEW; +END +$$; + CREATE FUNCTION sync_issues_dates_with_work_item_dates_sources() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -26425,7 +26458,9 @@ CREATE TABLE system_note_metadata ( updated_at timestamp without time zone NOT NULL, description_version_id bigint, note_id bigint NOT NULL, - id bigint NOT NULL + id bigint NOT NULL, + namespace_id bigint, + organization_id bigint ); CREATE SEQUENCE system_note_metadata_id_seq @@ -32793,6 +32828,9 @@ ALTER TABLE sprints ALTER TABLE redirect_routes ADD CONSTRAINT check_e82ff70482 CHECK ((namespace_id IS NOT NULL)) NOT VALID; +ALTER TABLE system_note_metadata + ADD CONSTRAINT check_f2c4e04565 CHECK ((num_nonnulls(namespace_id, organization_id) = 1)) NOT VALID; + ALTER TABLE notes_archived ADD CONSTRAINT check_notes_archived_has_parent CHECK ((num_nonnulls(namespace_id, organization_id, project_id) >= 1)) NOT VALID; @@ -46199,6 +46237,8 @@ CREATE TRIGGER projects_loose_fk_trigger AFTER DELETE ON projects REFERENCING OL CREATE TRIGGER push_rules_loose_fk_trigger AFTER DELETE ON push_rules REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); +CREATE TRIGGER set_sharding_key_for_system_note_metadata_on_insert BEFORE INSERT ON system_note_metadata FOR EACH ROW EXECUTE FUNCTION set_sharding_key_for_system_note_metadata(); + CREATE TRIGGER sync_project_authorizations_to_migration AFTER INSERT OR DELETE OR UPDATE ON project_authorizations FOR EACH ROW EXECUTE FUNCTION sync_project_authorizations_to_migration_table(); CREATE TRIGGER sync_sent_notifications_to_part AFTER INSERT ON sent_notifications FOR EACH ROW EXECUTE FUNCTION sync_to_p_sent_notifications_table(); diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 5a0623378d213e..7d4125fb5c05b5 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -284,6 +284,7 @@ security_finding_token_statuses: %w[security_finding_id project_id], system_access_group_microsoft_graph_access_tokens: %w[temp_source_id], # temporary column that is not a foreign key system_access_group_microsoft_applications: %w[temp_source_id], # temporary column that is not a foreign key + system_note_metadata: %w[namespace_id organization_id], # Adding FK after indexes are created over the weekend subscription_user_add_on_assignment_versions: %w[item_id user_id purchase_id], # Managed by paper_trail gem, no need for FK on the historical data virtual_registries_packages_maven_cache_entries: %w[group_id], # We can't use a foreign key due to object storage references # system_defined_status_id reference to fixed items model which is stored in code diff --git a/spec/models/system_note_metadata_spec.rb b/spec/models/system_note_metadata_spec.rb index 6a770b34d3e244..65a70907d854d6 100644 --- a/spec/models/system_note_metadata_spec.rb +++ b/spec/models/system_note_metadata_spec.rb @@ -84,4 +84,30 @@ end end end + + describe 'ensure sharding key trigger' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + let_it_be(:personal_snippet) { create(:personal_snippet) } + + subject { create(:system_note_metadata, note: note).reload.values_at(:namespace_id, :organization_id) } + + context 'when associated note belongs to an organization' do + let(:note) { create(:note_on_personal_snippet, noteable: personal_snippet) } + + it { is_expected.to eq([nil, personal_snippet.organization_id]) } + end + + context 'when associated note belongs to a project and namespace' do + let(:note) { create(:note, noteable: create(:issue, project: project), project: project) } + + it { is_expected.to eq([project.project_namespace_id, nil]) } + end + + context 'when associated note belongs only to a namespace' do + let(:note) { create(:note, noteable: create(:issue, :group_level, namespace: group), project_id: nil) } + + it { is_expected.to eq([group.id, nil]) } + end + end end -- GitLab From ff2a4e0885441e6286b02ad8f7d89b24aaf8a191 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Thu, 18 Sep 2025 23:29:56 -0500 Subject: [PATCH 2/3] Add WIP to sharding key spec --- spec/lib/gitlab/database/sharding_key_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index f3da05caf6af29..2828063eb8e0e5 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -322,7 +322,8 @@ "labels" => "https://gitlab.com/gitlab-org/gitlab/-/issues/563889", "notes" => "https://gitlab.com/gitlab-org/gitlab/-/issues/569521", "notes_archived" => "https://gitlab.com/gitlab-org/gitlab/-/issues/569521", - "jira_connect_installations" => "https://gitlab.com/gitlab-org/gitlab/-/issues/524682" + "jira_connect_installations" => "https://gitlab.com/gitlab-org/gitlab/-/issues/524682", + "system_note_metadata" => "https://gitlab.com/gitlab-org/gitlab/-/issues/571215" } has_lfk = ->(lfks) { lfks.any? { |k| k.options[:column] == 'organization_id' && k.to_table == 'organizations' } } -- GitLab From ed7885740c55ab9bb20c3c40f0f6a1561d9411c8 Mon Sep 17 00:00:00 2001 From: Mario Celi Date: Fri, 19 Sep 2025 12:41:04 -0500 Subject: [PATCH 3/3] Make trigger function generic --- ...ing_key_trigger_on_system_note_metadata.rb | 4 +- db/structure.sql | 68 +++++++++---------- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb b/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb index 99b8f4beab6640..82a2497cf30c89 100644 --- a/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb +++ b/db/migrate/20250918211138_add_sharding_key_trigger_on_system_note_metadata.rb @@ -3,7 +3,7 @@ class AddShardingKeyTriggerOnSystemNoteMetadata < Gitlab::Database::Migration[2.3] include Gitlab::Database::SchemaHelpers - FUNCTION_NAME = 'set_sharding_key_for_system_note_metadata' + FUNCTION_NAME = 'get_sharding_key_from_notes_table' TRIGGER_NAME = 'set_sharding_key_for_system_note_metadata_on_insert' milestone '18.5' @@ -18,7 +18,7 @@ def up note_project_id BIGINT; note_namespace_id BIGINT; BEGIN - IF NEW."note_id" IS NULL THEN + IF NEW."note_id" IS NULL OR num_nonnulls(NEW."namespace_id", NEW."organization_id") = 1 THEN RETURN NEW; END IF; diff --git a/db/structure.sql b/db/structure.sql index 0d2fb5e55d5d1f..d7c495c9cc2b91 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -503,6 +503,39 @@ BEGIN END $$; +CREATE FUNCTION get_sharding_key_from_notes_table() RETURNS trigger + LANGUAGE plpgsql + AS $$ +DECLARE + note_organization_id BIGINT; + note_project_id BIGINT; + note_namespace_id BIGINT; +BEGIN + IF NEW."note_id" IS NULL OR num_nonnulls(NEW."namespace_id", NEW."organization_id") = 1 THEN + RETURN NEW; + END IF; + + SELECT "organization_id", "project_id", "namespace_id" + INTO note_organization_id, note_project_id, note_namespace_id + FROM "notes" + WHERE "id" = NEW."note_id"; + + IF note_organization_id IS NOT NULL THEN + NEW."organization_id" := note_organization_id; + NEW."namespace_id" := NULL; + ELSIF note_project_id IS NOT NULL THEN + SELECT "project_namespace_id" FROM "projects" + INTO NEW."namespace_id" WHERE "projects"."id" = note_project_id; + NEW."organization_id" := NULL; + ELSE + NEW."namespace_id" := note_namespace_id; + NEW."organization_id" := NULL; + END IF; + + RETURN NEW; +END +$$; + CREATE FUNCTION gitlab_schema_prevent_write() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -777,39 +810,6 @@ RETURN NULL; END $$; -CREATE FUNCTION set_sharding_key_for_system_note_metadata() RETURNS trigger - LANGUAGE plpgsql - AS $$ -DECLARE - note_organization_id BIGINT; - note_project_id BIGINT; - note_namespace_id BIGINT; -BEGIN - IF NEW."note_id" IS NULL THEN - RETURN NEW; - END IF; - - SELECT "organization_id", "project_id", "namespace_id" - INTO note_organization_id, note_project_id, note_namespace_id - FROM "notes" - WHERE "id" = NEW."note_id"; - - IF note_organization_id IS NOT NULL THEN - NEW."organization_id" := note_organization_id; - NEW."namespace_id" := NULL; - ELSIF note_project_id IS NOT NULL THEN - SELECT "project_namespace_id" FROM "projects" - INTO NEW."namespace_id" WHERE "projects"."id" = note_project_id; - NEW."organization_id" := NULL; - ELSE - NEW."namespace_id" := note_namespace_id; - NEW."organization_id" := NULL; - END IF; - - RETURN NEW; -END -$$; - CREATE FUNCTION sync_issues_dates_with_work_item_dates_sources() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -46237,7 +46237,7 @@ CREATE TRIGGER projects_loose_fk_trigger AFTER DELETE ON projects REFERENCING OL CREATE TRIGGER push_rules_loose_fk_trigger AFTER DELETE ON push_rules REFERENCING OLD TABLE AS old_table FOR EACH STATEMENT EXECUTE FUNCTION insert_into_loose_foreign_keys_deleted_records(); -CREATE TRIGGER set_sharding_key_for_system_note_metadata_on_insert BEFORE INSERT ON system_note_metadata FOR EACH ROW EXECUTE FUNCTION set_sharding_key_for_system_note_metadata(); +CREATE TRIGGER set_sharding_key_for_system_note_metadata_on_insert BEFORE INSERT ON system_note_metadata FOR EACH ROW EXECUTE FUNCTION get_sharding_key_from_notes_table(); CREATE TRIGGER sync_project_authorizations_to_migration AFTER INSERT OR DELETE OR UPDATE ON project_authorizations FOR EACH ROW EXECUTE FUNCTION sync_project_authorizations_to_migration_table(); -- GitLab