diff --git a/db/migrate/20251013100839_add_sharding_key_to_diff_note_positions.rb b/db/migrate/20251013100839_add_sharding_key_to_diff_note_positions.rb new file mode 100644 index 0000000000000000000000000000000000000000..81420a3e94778294f619ce1212f16d930265dd7e --- /dev/null +++ b/db/migrate/20251013100839_add_sharding_key_to_diff_note_positions.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddShardingKeyToDiffNotePositions < Gitlab::Database::Migration[2.3] + milestone '18.6' + + def change + add_column :diff_note_positions, :namespace_id, :bigint + end +end diff --git a/db/migrate/20251013100914_add_sharding_key_trigger_on_diff_note_positions.rb b/db/migrate/20251013100914_add_sharding_key_trigger_on_diff_note_positions.rb new file mode 100644 index 0000000000000000000000000000000000000000..d34077d27582437cc408993c839582b882ab0a83 --- /dev/null +++ b/db/migrate/20251013100914_add_sharding_key_trigger_on_diff_note_positions.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class AddShardingKeyTriggerOnDiffNotePositions < Gitlab::Database::Migration[2.3] + include Gitlab::Database::SchemaHelpers + + FUNCTION_NAME = 'sync_sharding_key_with_notes_table' + TRIGGER_NAME = 'set_sharding_key_for_diff_note_positions_on_insert_and_update' + + milestone '18.6' + + def up + execute(<<~SQL) + CREATE OR REPLACE FUNCTION #{FUNCTION_NAME}() + RETURNS TRIGGER AS + $$ + DECLARE + note_project_id BIGINT; + note_namespace_id BIGINT; + BEGIN + IF NEW."note_id" IS NULL OR NEW."namespace_id" IS NOT NULL THEN + RETURN NEW; + END IF; + + SELECT "project_id", "namespace_id" + INTO note_project_id, note_namespace_id + FROM "notes" + WHERE "id" = NEW."note_id"; + + IF note_project_id IS NOT NULL THEN + SELECT "project_namespace_id" FROM "projects" + INTO NEW."namespace_id" WHERE "projects"."id" = note_project_id; + ELSE + NEW."namespace_id" := note_namespace_id; + END IF; + + RETURN NEW; + END + $$ LANGUAGE PLPGSQL; + SQL + + create_trigger(:diff_note_positions, TRIGGER_NAME, FUNCTION_NAME, fires: 'BEFORE INSERT OR UPDATE') + end + + def down + drop_trigger(:diff_note_positions, TRIGGER_NAME) + end +end diff --git a/db/post_migrate/20251013100952_add_not_null_constraint_on_diff_note_positions_sharding_key.rb b/db/post_migrate/20251013100952_add_not_null_constraint_on_diff_note_positions_sharding_key.rb new file mode 100644 index 0000000000000000000000000000000000000000..d9db9989338ff4af353505847b0363718a308647 --- /dev/null +++ b/db/post_migrate/20251013100952_add_not_null_constraint_on_diff_note_positions_sharding_key.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class AddNotNullConstraintOnDiffNotePositionsShardingKey < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.6' + + def up + add_not_null_constraint :diff_note_positions, :namespace_id, validate: false + end + + def down + remove_not_null_constraint :diff_note_positions, :namespace_id + end +end diff --git a/db/post_migrate/20251013101021_add_sharding_key_index_on_diff_note_positions.rb b/db/post_migrate/20251013101021_add_sharding_key_index_on_diff_note_positions.rb new file mode 100644 index 0000000000000000000000000000000000000000..aa52cd05c31b8bd8b703efe4adfef568bcccac03 --- /dev/null +++ b/db/post_migrate/20251013101021_add_sharding_key_index_on_diff_note_positions.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddShardingKeyIndexOnDiffNotePositions < Gitlab::Database::Migration[2.3] + INDEX_NAME = 'index_diff_note_positions_on_namespace_id' + + milestone '18.6' + disable_ddl_transaction! + + def up + add_concurrent_index :diff_note_positions, :namespace_id, name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :diff_note_positions, INDEX_NAME + end +end diff --git a/db/post_migrate/20251013101048_add_diff_note_positions_namespace_id_foreign_key.rb b/db/post_migrate/20251013101048_add_diff_note_positions_namespace_id_foreign_key.rb new file mode 100644 index 0000000000000000000000000000000000000000..ded015f9152b98fcb3c2ce953d90f929a00a5347 --- /dev/null +++ b/db/post_migrate/20251013101048_add_diff_note_positions_namespace_id_foreign_key.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddDiffNotePositionsNamespaceIdForeignKey < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.6' + + def up + add_concurrent_foreign_key :diff_note_positions, + :namespaces, + column: :namespace_id, + reverse_lock_order: true, + validate: false + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(:diff_note_positions, column: :namespace_id) + end + end +end diff --git a/db/schema_migrations/20251013100839 b/db/schema_migrations/20251013100839 new file mode 100644 index 0000000000000000000000000000000000000000..e4fc8f8208110e8806f9d189b5a8e62296060d56 --- /dev/null +++ b/db/schema_migrations/20251013100839 @@ -0,0 +1 @@ +8862894c50fcb33c65e9fd7eadac929cde069a0930a6b2413d33c6f56d27ed6c \ No newline at end of file diff --git a/db/schema_migrations/20251013100914 b/db/schema_migrations/20251013100914 new file mode 100644 index 0000000000000000000000000000000000000000..47f1ec673ce0eabbf86de6bd5983035d660b21ca --- /dev/null +++ b/db/schema_migrations/20251013100914 @@ -0,0 +1 @@ +7fab7fb9f9370431b9032d9508723e9aee34aa36a1a09704250f6fd04fb14970 \ No newline at end of file diff --git a/db/schema_migrations/20251013100952 b/db/schema_migrations/20251013100952 new file mode 100644 index 0000000000000000000000000000000000000000..0b90e3863020f3214fa36d647809de605b01cd76 --- /dev/null +++ b/db/schema_migrations/20251013100952 @@ -0,0 +1 @@ +63528f178a1325af030cfffd987718b735be29a6b820d5b1a550dab98005c5a4 \ No newline at end of file diff --git a/db/schema_migrations/20251013101021 b/db/schema_migrations/20251013101021 new file mode 100644 index 0000000000000000000000000000000000000000..a619c2c27d9f2375ac7d6f10ecdd3653cf2b468a --- /dev/null +++ b/db/schema_migrations/20251013101021 @@ -0,0 +1 @@ +144181bf75fe8246f785a0ba7e0daf6ead69fd9c68e79f7d6e6494c09bf413bf \ No newline at end of file diff --git a/db/schema_migrations/20251013101048 b/db/schema_migrations/20251013101048 new file mode 100644 index 0000000000000000000000000000000000000000..5bb852ed171c22c11c2a263a70fbbebf526a63cd --- /dev/null +++ b/db/schema_migrations/20251013101048 @@ -0,0 +1 @@ +ffaa8e5ca6823a5c152cf556fd17b6f21b3ae85dbb9d380d3978eed18a55a852 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 64d8af58f2481beba53dfe077aa42827bfa206ed..888e298e3a720c5de5d24d1735f226d3c8483e1a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -16171,7 +16171,8 @@ CREATE TABLE diff_note_positions ( start_sha bytea NOT NULL, head_sha bytea NOT NULL, old_path text NOT NULL, - new_path text NOT NULL + new_path text NOT NULL, + namespace_id bigint ); CREATE SEQUENCE diff_note_positions_id_seq @@ -33216,6 +33217,9 @@ ALTER TABLE push_event_payloads ALTER TABLE todos ADD CONSTRAINT check_3c13ed1c7a CHECK ((num_nonnulls(group_id, organization_id, project_id) = 1)) NOT VALID; +ALTER TABLE diff_note_positions + ADD CONSTRAINT check_4c86140f48 CHECK ((namespace_id IS NOT NULL)) NOT VALID; + ALTER TABLE ONLY instance_type_ci_runners ADD CONSTRAINT check_5c34a3c1db UNIQUE (id); @@ -39711,6 +39715,8 @@ CREATE UNIQUE INDEX index_design_user_mentions_on_note_id ON design_user_mention CREATE INDEX index_designated_beneficiaries_on_user_id ON designated_beneficiaries USING btree (user_id); +CREATE INDEX index_diff_note_positions_on_namespace_id ON diff_note_positions USING btree (namespace_id); + CREATE UNIQUE INDEX index_diff_note_positions_on_note_id_and_diff_type ON diff_note_positions USING btree (note_id, diff_type); CREATE INDEX index_dingtalk_tracker_data_on_integration_id ON dingtalk_tracker_data USING btree (integration_id); @@ -46923,6 +46929,8 @@ CREATE TRIGGER set_namespace_for_system_note_metadata_on_insert BEFORE INSERT ON CREATE TRIGGER set_sharding_key_for_commit_user_mentions_on_insert_and_update BEFORE INSERT OR UPDATE ON commit_user_mentions FOR EACH ROW EXECUTE FUNCTION sync_sharding_key_with_notes_table(); +CREATE TRIGGER set_sharding_key_for_diff_note_positions_on_insert_and_update BEFORE INSERT OR UPDATE ON diff_note_positions FOR EACH ROW EXECUTE FUNCTION sync_sharding_key_with_notes_table(); + CREATE TRIGGER set_sharding_key_for_note_metadata_on_insert_and_update BEFORE INSERT OR UPDATE ON note_metadata FOR EACH ROW EXECUTE FUNCTION sync_sharding_key_with_notes_table(); CREATE TRIGGER set_sharding_key_for_suggestions_on_insert_and_update BEFORE INSERT OR UPDATE ON suggestions FOR EACH ROW EXECUTE FUNCTION sync_sharding_key_with_notes_table(); @@ -48976,6 +48984,9 @@ ALTER TABLE ONLY issues ALTER TABLE ONLY clusters_managed_resources ADD CONSTRAINT fk_9c7b561962 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY diff_note_positions + ADD CONSTRAINT fk_9ccec9c22a FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY packages_conan_recipe_revisions ADD CONSTRAINT fk_9cdec8a86b FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE; diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index 40bb3be257f6e93ea8aab2aa3c826efa46aea2c7..078876aa5af39818d3f4a2cba34ad88a02e51131 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -335,7 +335,8 @@ "suggestions" => "https://gitlab.com/gitlab-org/gitlab/-/issues/550696", "commit_user_mentions" => "https://gitlab.com/gitlab-org/gitlab/-/issues/550692", "note_metadata" => "https://gitlab.com/gitlab-org/gitlab/-/issues/550695", - "slack_integrations" => "https://gitlab.com/gitlab-org/gitlab/-/issues/524680" + "slack_integrations" => "https://gitlab.com/gitlab-org/gitlab/-/issues/524680", + "diff_note_positions" => "https://gitlab.com/gitlab-org/gitlab/-/issues/550693" } has_lfk = ->(lfks) { lfks.any? { |k| k.options[:column] == 'organization_id' && k.to_table == 'organizations' } } diff --git a/spec/models/diff_note_position_spec.rb b/spec/models/diff_note_position_spec.rb index 5ccf5d69ca70a2b33531f3302a7bada0461b95a7..78ce16928909bfb130c6b5cbb86940d2d05885a4 100644 --- a/spec/models/diff_note_position_spec.rb +++ b/spec/models/diff_note_position_spec.rb @@ -19,6 +19,22 @@ expect(diff_note_position.line_code).to eq(line_code) expect(diff_note_position.diff_content_type).to eq('text') end + + context 'with sharding key' do + it 'uses the note namespace_id as sharding key' do + note.update_column(:namespace_id, nil) + described_class.create_or_update_for(note, params) + + expect(diff_note_position.namespace_id).to eq(note.project.project_namespace_id) + end + + it 'uses the note project_id as sharding key' do + note.update_column(:project_id, nil) + described_class.create_or_update_for(note, params) + + expect(diff_note_position.namespace_id).to eq(note.namespace_id) + end + end end context 'has a diff note position' do @@ -30,6 +46,26 @@ expect(diff_note_position.position).to eq(diff_position) expect(diff_note_position.line_code).to eq(line_code) end + + context 'with sharding key' do + it 'uses the note project_id as sharding key' do + create(:diff_note_position, note: note) + note.update_column(:namespace_id, nil) + diff_note_position.update_column(:namespace_id, nil) + described_class.create_or_update_for(note, params) + + expect(diff_note_position.reload.namespace_id).to eq(note.project.project_namespace_id) + end + + it 'uses the note namespace_id as sharding key' do + create(:diff_note_position, note: note) + note.update_column(:project_id, nil) + diff_note_position.update_column(:namespace_id, nil) + described_class.create_or_update_for(note, params) + + expect(diff_note_position.reload.namespace_id).to eq(note.namespace_id) + end + end end end end diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index 8d1dc64397de105822013c3005250b254712fca3..6452ba30ecaa199c8a3a65e5f2dcd15c26c2e6a6 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -195,6 +195,8 @@ def publish(draft: nil) notes = merge_request.notes.order(id: :asc) expect(notes.first.diff_note_positions).to be_any expect(notes.last.diff_note_positions).to be_any + expect(notes.flat_map(&:diff_note_positions).pluck(:namespace_id).uniq) + .to contain_exactly(project.project_namespace_id) end it 'keeps around the commits of each published note' do