From c8d2b5d0216af2e4072d0073c4c3e02fcf774d63 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 18:15:17 +0200 Subject: [PATCH 01/74] Initial version of train ref commit tracking --- app/finders/merge_requests_finder.rb | 17 ++++++ app/models/merge_request.rb | 7 +++ app/models/train_ref_commit.rb | 13 ++++ .../merge_requests/create_ref_service.rb | 60 +++++++++++++++++++ db/docs/train_ref_commits.yml | 13 ++++ .../20250626095521_add_train_ref_commit.rb | 41 +++++++++++++ ...equest_foreign_key_to_train_ref_commits.rb | 39 ++++++++++++ ...roject_foreign_key_to_train_ref_commits.rb | 37 ++++++++++++ ...1_add_unique_index_to_train_ref_commits.rb | 18 ++++++ db/schema_migrations/20250626095521 | 1 + db/schema_migrations/20250626123746 | 1 + db/schema_migrations/20250626123902 | 1 + db/schema_migrations/20250626124251 | 1 + db/structure.sql | 38 ++++++++---- spec/factories/train_ref_commits.rb | 9 +++ 15 files changed, 285 insertions(+), 11 deletions(-) create mode 100644 app/models/train_ref_commit.rb create mode 100644 db/docs/train_ref_commits.yml create mode 100644 db/migrate/20250626095521_add_train_ref_commit.rb create mode 100644 db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb create mode 100644 db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb create mode 100644 db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb create mode 100644 db/schema_migrations/20250626095521 create mode 100644 db/schema_migrations/20250626123746 create mode 100644 db/schema_migrations/20250626123902 create mode 100644 db/schema_migrations/20250626124251 create mode 100644 spec/factories/train_ref_commits.rb diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index fd71182f29951c..a7c9737e2aa782 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -95,6 +95,7 @@ def filter_items(_items) items = by_blob_path(items) items = by_no_review_requested_or_only_user(items) items = by_review_states_or_no_reviewer(items) + items = by_train_ref_commit_sha(items) by_valid_or_no_reviewers(items) end @@ -120,6 +121,22 @@ def sort(items) items.group(grouping_columns) # rubocop:disable CodeReuse/ActiveRecord end + # This method is used as a fallback for now. + # We have the problem that whenever a merge_request gets merged and + # its commit_shas get altered e.g. by a rebase, these "new" commits do not belong + # to a merge request anymore. Nevertheless, we have the functionality that whenever we show + # a commit from e.g. the main branch, we also show the corresponding merge request. + def by_train_ref_commit_sha(items) + return items unless params[:project_id].present? && params[:commit_sha].present? + + return items unless items.empty? + + MergeRequest.by_project_and_commit_sha_via_train_ref( + params[:project_id], + params[:commit_sha] + ) + end + def by_author(items) MergeRequests::AuthorFilter.new( current_user: current_user, diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5fac020b6ab45f..20355c47664151 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -329,6 +329,13 @@ def public_merge_status scope :by_source_or_target_branch, ->(branch_name) do where("source_branch = :branch OR target_branch = :branch", branch: branch_name) end + + scope :by_project_and_commit_sha_via_train_ref, ->(project_id, sha) { + joins("INNER JOIN train_ref_commits ON train_ref_commits.merge_request_id = merge_requests.id") + .where(merge_requests: { project_id: project_id }) + .where(train_ref_commits: { commit_sha: Gitlab::Database::ShaAttribute.new.serialize(sha) }) + } + scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :of_projects, ->(ids) { where(target_project_id: ids) } scope :from_project, ->(project) { where(source_project_id: project) } diff --git a/app/models/train_ref_commit.rb b/app/models/train_ref_commit.rb new file mode 100644 index 00000000000000..20a33014852860 --- /dev/null +++ b/app/models/train_ref_commit.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +# rubocop:disable Gitlab/BoundedContexts, Gitlab/NamespacedClass -- Will be decided during review +class TrainRefCommit < ApplicationRecord + include ShaAttribute + + sha_attribute :commit_sha + + belongs_to :merge_request + belongs_to :project + validates :commit_sha, presence: true +end +# rubocop:enable Gitlab/BoundedContexts, Gitlab/NamespacedClass -- Will be decided during review diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 1e6555485b588a..46402e3a826277 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,6 +39,12 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) + # Store commits in train_ref_commits table using the final commit_sha + store_train_ref_commits(result[:commit_sha]) + + # Add commits data to result for external use + result[:commits_data] = collect_commits_data(result[:commit_sha]) + ServiceResponse.success(payload: result) rescue CreateRefError => error ServiceResponse.error(message: error.message) @@ -52,6 +58,60 @@ def execute delegate :target_project, to: :merge_request delegate :repository, to: :target_project + def store_train_ref_commits(final_commit_sha) + commit_shas = commit_shas_between_refs(final_commit_sha) + return unless commit_shas.any? + + # Prepare records for bulk insert + records = commit_shas.map do |commit_sha| + { + merge_request_id: merge_request.id, + commit_sha: commit_sha, + project_id: merge_request.project_id, + created_at: Time.current, + updated_at: Time.current + } + end + + # Use upsert to handle potential duplicates + TrainRefCommit.upsert_all( + records, + unique_by: [:merge_request_id, :commit_sha] + ) + rescue StandardError => e + ServiceResponse.error(message: "Failed to store train_ref_commits for MR #{merge_request.id}: #{e.message}") + # Don't raise error to avoid breaking the main flow + end + + def collect_commits_data(final_commit_sha) + return [] unless final_commit_sha && first_parent_sha + + commits_between_refs(final_commit_sha).map do |commit| + { + commit_sha: commit.sha, + merge_request_id: merge_request.id, + message: commit.message, + author_name: commit.author_name, + author_email: commit.author_email, + authored_date: commit.authored_date, + committed_date: commit.committed_date + } + end + rescue StandardError => e + ServiceResponse.error(message: "Failed to collect commits data for MR #{merge_request.id}: #{e.message}") + [] + end + + def commits_between_refs(final_commit_sha) + return [] unless final_commit_sha && first_parent_sha + + repository.commits_between(first_parent_sha, final_commit_sha) + end + + def commit_shas_between_refs(final_commit_sha) + commits_between_refs(final_commit_sha).map(&:sha) + end + def maybe_squash!(commit_sha:, **rest) if merge_request.squash_on_merge? squash_result = MergeRequests::SquashService.new( diff --git a/db/docs/train_ref_commits.yml b/db/docs/train_ref_commits.yml new file mode 100644 index 00000000000000..06861348538947 --- /dev/null +++ b/db/docs/train_ref_commits.yml @@ -0,0 +1,13 @@ +--- +table_name: train_ref_commits +classes: +- TrainRefCommit +feature_categories: +- merge_trains +description: Stores the new commit shas for a merged merge request +introduced_by_url: +milestone: '18.3' +gitlab_schema: gitlab_main_cell +sharding_key: + project_id: projects +table_size: small diff --git a/db/migrate/20250626095521_add_train_ref_commit.rb b/db/migrate/20250626095521_add_train_ref_commit.rb new file mode 100644 index 00000000000000..fe10925b454802 --- /dev/null +++ b/db/migrate/20250626095521_add_train_ref_commit.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddTrainRefCommit < Gitlab::Database::Migration[2.3] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + # + # Configure the `gitlab_schema` to perform data manipulation (DML). + # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html + # restrict_gitlab_migration gitlab_schema: :gitlab_main + + # Add dependent 'batched_background_migrations.queued_migration_version' values. + # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] + milestone '18.2' + disable_ddl_transaction! + + def change + # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found + create_table :train_ref_commits do |t| + t.binary :commit_sha, limit: 20 + t.bigint :project_id, null: false + t.bigint :merge_request_id, null: false + t.timestamps_with_timezone + + t.index :commit_sha + end + # rubocop:enable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found + end +end diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb new file mode 100644 index 00000000000000..631ae7590e94ef --- /dev/null +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddMergeRequestForeignKeyToTrainRefCommits < Gitlab::Database::Migration[2.3] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + # + # Configure the `gitlab_schema` to perform data manipulation (DML). + # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html + # restrict_gitlab_migration gitlab_schema: :gitlab_main + + # Add dependent 'batched_background_migrations.queued_migration_version' values. + # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] + milestone '18.2' + + def up + add_concurrent_foreign_key :train_ref_commits, :merge_requests, + column: :merge_request_id, + on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(:train_ref_commits, column: :merge_request_id) + end + end +end diff --git a/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb b/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb new file mode 100644 index 00000000000000..7c77748c73a8bb --- /dev/null +++ b/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddProjectForeignKeyToTrainRefCommits < Gitlab::Database::Migration[2.3] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + disable_ddl_transaction! + # + # Configure the `gitlab_schema` to perform data manipulation (DML). + # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html + # restrict_gitlab_migration gitlab_schema: :gitlab_main + + # Add dependent 'batched_background_migrations.queued_migration_version' values. + # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] + milestone '18.2' + + def up + add_concurrent_foreign_key :train_ref_commits, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :train_ref_commits, column: :project_id + end + end +end diff --git a/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb b/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb new file mode 100644 index 00000000000000..ce2f2049dbacdf --- /dev/null +++ b/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddUniqueIndexToTrainRefCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + def up + add_concurrent_index :train_ref_commits, %i[merge_request_id commit_sha], + name: 'index_train_ref_commits_on_project_mr_sha_unique', unique: true + end + + def down + remove_concurrent_index_by_name :train_ref_commits, 'index_train_ref_commits_on_project_mr_sha_unique' + end +end diff --git a/db/schema_migrations/20250626095521 b/db/schema_migrations/20250626095521 new file mode 100644 index 00000000000000..26ced1d89db9d4 --- /dev/null +++ b/db/schema_migrations/20250626095521 @@ -0,0 +1 @@ +e546b14642c5f3f9fa1151304647db7e920624572f3f59eef3635e136ce00f04 \ No newline at end of file diff --git a/db/schema_migrations/20250626123746 b/db/schema_migrations/20250626123746 new file mode 100644 index 00000000000000..5d30083cdb9275 --- /dev/null +++ b/db/schema_migrations/20250626123746 @@ -0,0 +1 @@ +ba029f8111ee33ff7de9504cddff255cad3054fdccc669eebb57f715f3f607be \ No newline at end of file diff --git a/db/schema_migrations/20250626123902 b/db/schema_migrations/20250626123902 new file mode 100644 index 00000000000000..7f31d0d9b44a56 --- /dev/null +++ b/db/schema_migrations/20250626123902 @@ -0,0 +1 @@ +00eaa073c518276fe40f7d405010fe8b47999ced66f4986b37ec2cc2977b89ef \ No newline at end of file diff --git a/db/schema_migrations/20250626124251 b/db/schema_migrations/20250626124251 new file mode 100644 index 00000000000000..39ca8396f8a5f7 --- /dev/null +++ b/db/schema_migrations/20250626124251 @@ -0,0 +1 @@ +b1d06ed4e73075c98e19290a469a9acbf8a00557634a0b6a8f17e383af1e5efe \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ff9d8c660c484d..d066aae492aba4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4988,7 +4988,7 @@ PARTITION BY RANGE (created_at); CREATE TABLE loose_foreign_keys_deleted_records ( id bigint NOT NULL, - partition bigint DEFAULT 1 NOT NULL, + partition bigint DEFAULT 7 NOT NULL, primary_key_value bigint NOT NULL, status smallint DEFAULT 1 NOT NULL, created_at timestamp with time zone DEFAULT now() NOT NULL, @@ -5063,7 +5063,7 @@ PARTITION BY LIST (partition_id); CREATE TABLE p_ci_finished_build_ch_sync_events ( build_id bigint NOT NULL, - partition bigint DEFAULT 1 NOT NULL, + partition bigint DEFAULT 13 NOT NULL, build_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL, project_id bigint NOT NULL @@ -5073,7 +5073,7 @@ PARTITION BY LIST (partition); CREATE TABLE p_ci_finished_pipeline_ch_sync_events ( pipeline_id bigint NOT NULL, project_namespace_id bigint NOT NULL, - partition bigint DEFAULT 1 NOT NULL, + partition bigint DEFAULT 13 NOT NULL, pipeline_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL ) @@ -9614,30 +9614,30 @@ CREATE TABLE application_settings ( identity_verification_settings jsonb DEFAULT '{}'::jsonb NOT NULL, integrations jsonb DEFAULT '{}'::jsonb NOT NULL, user_seat_management jsonb DEFAULT '{}'::jsonb NOT NULL, + resource_usage_limits jsonb DEFAULT '{}'::jsonb NOT NULL, + show_migrate_from_jenkins_banner boolean DEFAULT true NOT NULL, secret_detection_service_url text DEFAULT ''::text NOT NULL, encrypted_secret_detection_service_auth_token bytea, encrypted_secret_detection_service_auth_token_iv bytea, - resource_usage_limits jsonb DEFAULT '{}'::jsonb NOT NULL, - show_migrate_from_jenkins_banner boolean DEFAULT true NOT NULL, encrypted_ci_job_token_signing_key bytea, encrypted_ci_job_token_signing_key_iv bytea, elasticsearch jsonb DEFAULT '{}'::jsonb NOT NULL, oauth_provider jsonb DEFAULT '{}'::jsonb NOT NULL, observability_settings jsonb DEFAULT '{}'::jsonb NOT NULL, search jsonb DEFAULT '{}'::jsonb NOT NULL, - anti_abuse_settings jsonb DEFAULT '{}'::jsonb NOT NULL, secret_push_protection_available boolean DEFAULT false, + anti_abuse_settings jsonb DEFAULT '{}'::jsonb NOT NULL, vscode_extension_marketplace jsonb DEFAULT '{}'::jsonb NOT NULL, token_prefixes jsonb DEFAULT '{}'::jsonb NOT NULL, - ci_cd_settings jsonb DEFAULT '{}'::jsonb NOT NULL, database_reindexing jsonb DEFAULT '{}'::jsonb NOT NULL, duo_chat jsonb DEFAULT '{}'::jsonb NOT NULL, + ci_cd_settings jsonb DEFAULT '{}'::jsonb NOT NULL, group_settings jsonb DEFAULT '{}'::jsonb NOT NULL, model_prompt_cache_enabled boolean DEFAULT true NOT NULL, lock_model_prompt_cache_enabled boolean DEFAULT false NOT NULL, - response_limits jsonb DEFAULT '{}'::jsonb NOT NULL, web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, lock_web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, + response_limits jsonb DEFAULT '{}'::jsonb NOT NULL, tmp_asset_proxy_secret_key jsonb, editor_extensions jsonb DEFAULT '{}'::jsonb NOT NULL, security_and_compliance_settings jsonb DEFAULT '{}'::jsonb NOT NULL, @@ -18608,9 +18608,9 @@ CREATE TABLE namespace_settings ( job_token_policies_enabled boolean DEFAULT false NOT NULL, security_policies jsonb DEFAULT '{}'::jsonb NOT NULL, duo_nano_features_enabled boolean, + disable_invite_members boolean DEFAULT false NOT NULL, model_prompt_cache_enabled boolean, lock_model_prompt_cache_enabled boolean DEFAULT false NOT NULL, - disable_invite_members boolean DEFAULT false NOT NULL, web_based_commit_signing_enabled boolean, lock_web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, allow_enterprise_bypass_placeholder_confirmation boolean DEFAULT false NOT NULL, @@ -24824,6 +24824,15 @@ CREATE SEQUENCE topics_id_seq ALTER SEQUENCE topics_id_seq OWNED BY topics.id; +CREATE TABLE train_ref_commits ( + id bigint NOT NULL, + commit_sha bytea, + project_id bigint NOT NULL, + merge_request_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +); + CREATE TABLE trending_projects ( id bigint NOT NULL, project_id bigint NOT NULL @@ -25284,9 +25293,9 @@ CREATE TABLE user_preferences ( organization_groups_projects_display smallint DEFAULT 1 NOT NULL, dpop_enabled boolean DEFAULT false NOT NULL, use_work_items_view boolean DEFAULT false NOT NULL, - text_editor_type smallint DEFAULT 2 NOT NULL, - merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, + text_editor_type smallint DEFAULT 0 NOT NULL, extensions_marketplace_opt_in_url text, + merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, dark_color_scheme_id smallint, work_items_display_settings jsonb DEFAULT '{}'::jsonb NOT NULL, default_duo_add_on_assignment_id bigint, @@ -37461,6 +37470,8 @@ CREATE INDEX index_p_catalog_resource_sync_events_on_id_where_pending ON ONLY p_ CREATE INDEX index_p_ci_build_names_on_project_id_and_build_id ON ONLY p_ci_build_names USING btree (project_id, build_id); +CREATE INDEX index_p_ci_build_names_on_search_columns ON ONLY p_ci_build_names USING btree (project_id, name, build_id, partition_id); + CREATE INDEX index_p_ci_build_names_on_search_vector ON ONLY p_ci_build_names USING gin (search_vector); CREATE INDEX index_p_ci_build_sources_on_pipeline_source ON ONLY p_ci_build_sources USING btree (pipeline_source); @@ -38749,6 +38760,8 @@ CREATE INDEX index_topics_on_slug ON topics USING btree (slug) WHERE (slug IS NO CREATE INDEX index_topics_total_projects_count ON topics USING btree (total_projects_count DESC, id); +CREATE UNIQUE INDEX index_train_ref_commits_on_project_mr_sha_unique ON train_ref_commits USING btree (merge_request_id, commit_sha); + CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id); CREATE INDEX index_unarchived_occurrences_for_aggregations_component_name ON sbom_occurrences USING btree (traversal_ids, component_name, component_id, component_version_id) WHERE (archived = false); @@ -44067,6 +44080,9 @@ ALTER TABLE ONLY merge_requests_approval_rules_merge_requests ALTER TABLE ONLY software_license_policies ADD CONSTRAINT fk_74f6d8328a FOREIGN KEY (custom_software_license_id) REFERENCES custom_software_licenses(id) ON DELETE CASCADE; +ALTER TABLE ONLY train_ref_commits + ADD CONSTRAINT fk_74fa3f61ab FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + ALTER TABLE ONLY cluster_agent_tokens ADD CONSTRAINT fk_75008f3553 FOREIGN KEY (created_by_user_id) REFERENCES users(id) ON DELETE SET NULL; diff --git a/spec/factories/train_ref_commits.rb b/spec/factories/train_ref_commits.rb new file mode 100644 index 00000000000000..e0fe4650ebf124 --- /dev/null +++ b/spec/factories/train_ref_commits.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :train_ref_commit, class: 'TrainRefCommit' do + commit_sha { Digest::SHA256.hexdigest(SecureRandom.hex)[1..40] } + association :project + association :merge_request + end +end -- GitLab From 9c9fc7b9dd0f42cbba687c599ef2d5be47499fd8 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 18:19:48 +0200 Subject: [PATCH 02/74] Fix structure.sql layout --- db/structure.sql | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/db/structure.sql b/db/structure.sql index d066aae492aba4..c3f0203e02d8df 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4988,7 +4988,7 @@ PARTITION BY RANGE (created_at); CREATE TABLE loose_foreign_keys_deleted_records ( id bigint NOT NULL, - partition bigint DEFAULT 7 NOT NULL, + partition bigint DEFAULT 1 NOT NULL, primary_key_value bigint NOT NULL, status smallint DEFAULT 1 NOT NULL, created_at timestamp with time zone DEFAULT now() NOT NULL, @@ -5063,7 +5063,7 @@ PARTITION BY LIST (partition_id); CREATE TABLE p_ci_finished_build_ch_sync_events ( build_id bigint NOT NULL, - partition bigint DEFAULT 13 NOT NULL, + partition bigint DEFAULT 1 NOT NULL, build_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL, project_id bigint NOT NULL @@ -5073,7 +5073,7 @@ PARTITION BY LIST (partition); CREATE TABLE p_ci_finished_pipeline_ch_sync_events ( pipeline_id bigint NOT NULL, project_namespace_id bigint NOT NULL, - partition bigint DEFAULT 13 NOT NULL, + partition bigint DEFAULT 1 NOT NULL, pipeline_finished_at timestamp without time zone NOT NULL, processed boolean DEFAULT false NOT NULL ) @@ -9614,30 +9614,30 @@ CREATE TABLE application_settings ( identity_verification_settings jsonb DEFAULT '{}'::jsonb NOT NULL, integrations jsonb DEFAULT '{}'::jsonb NOT NULL, user_seat_management jsonb DEFAULT '{}'::jsonb NOT NULL, - resource_usage_limits jsonb DEFAULT '{}'::jsonb NOT NULL, - show_migrate_from_jenkins_banner boolean DEFAULT true NOT NULL, secret_detection_service_url text DEFAULT ''::text NOT NULL, encrypted_secret_detection_service_auth_token bytea, encrypted_secret_detection_service_auth_token_iv bytea, + resource_usage_limits jsonb DEFAULT '{}'::jsonb NOT NULL, + show_migrate_from_jenkins_banner boolean DEFAULT true NOT NULL, encrypted_ci_job_token_signing_key bytea, encrypted_ci_job_token_signing_key_iv bytea, elasticsearch jsonb DEFAULT '{}'::jsonb NOT NULL, oauth_provider jsonb DEFAULT '{}'::jsonb NOT NULL, observability_settings jsonb DEFAULT '{}'::jsonb NOT NULL, search jsonb DEFAULT '{}'::jsonb NOT NULL, - secret_push_protection_available boolean DEFAULT false, anti_abuse_settings jsonb DEFAULT '{}'::jsonb NOT NULL, + secret_push_protection_available boolean DEFAULT false, vscode_extension_marketplace jsonb DEFAULT '{}'::jsonb NOT NULL, token_prefixes jsonb DEFAULT '{}'::jsonb NOT NULL, + ci_cd_settings jsonb DEFAULT '{}'::jsonb NOT NULL, database_reindexing jsonb DEFAULT '{}'::jsonb NOT NULL, duo_chat jsonb DEFAULT '{}'::jsonb NOT NULL, - ci_cd_settings jsonb DEFAULT '{}'::jsonb NOT NULL, group_settings jsonb DEFAULT '{}'::jsonb NOT NULL, model_prompt_cache_enabled boolean DEFAULT true NOT NULL, lock_model_prompt_cache_enabled boolean DEFAULT false NOT NULL, + response_limits jsonb DEFAULT '{}'::jsonb NOT NULL, web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, lock_web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, - response_limits jsonb DEFAULT '{}'::jsonb NOT NULL, tmp_asset_proxy_secret_key jsonb, editor_extensions jsonb DEFAULT '{}'::jsonb NOT NULL, security_and_compliance_settings jsonb DEFAULT '{}'::jsonb NOT NULL, @@ -18608,9 +18608,9 @@ CREATE TABLE namespace_settings ( job_token_policies_enabled boolean DEFAULT false NOT NULL, security_policies jsonb DEFAULT '{}'::jsonb NOT NULL, duo_nano_features_enabled boolean, - disable_invite_members boolean DEFAULT false NOT NULL, model_prompt_cache_enabled boolean, lock_model_prompt_cache_enabled boolean DEFAULT false NOT NULL, + disable_invite_members boolean DEFAULT false NOT NULL, web_based_commit_signing_enabled boolean, lock_web_based_commit_signing_enabled boolean DEFAULT false NOT NULL, allow_enterprise_bypass_placeholder_confirmation boolean DEFAULT false NOT NULL, @@ -25285,6 +25285,7 @@ CREATE TABLE user_preferences ( enabled_zoekt boolean DEFAULT true NOT NULL, keyboard_shortcuts_enabled boolean DEFAULT true NOT NULL, time_display_format smallint DEFAULT 0 NOT NULL, + merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, home_organization_id bigint, early_access_program_participant boolean DEFAULT false NOT NULL, early_access_program_tracking boolean DEFAULT false NOT NULL, @@ -25295,7 +25296,6 @@ CREATE TABLE user_preferences ( use_work_items_view boolean DEFAULT false NOT NULL, text_editor_type smallint DEFAULT 0 NOT NULL, extensions_marketplace_opt_in_url text, - merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, dark_color_scheme_id smallint, work_items_display_settings jsonb DEFAULT '{}'::jsonb NOT NULL, default_duo_add_on_assignment_id bigint, @@ -37470,8 +37470,6 @@ CREATE INDEX index_p_catalog_resource_sync_events_on_id_where_pending ON ONLY p_ CREATE INDEX index_p_ci_build_names_on_project_id_and_build_id ON ONLY p_ci_build_names USING btree (project_id, build_id); -CREATE INDEX index_p_ci_build_names_on_search_columns ON ONLY p_ci_build_names USING btree (project_id, name, build_id, partition_id); - CREATE INDEX index_p_ci_build_names_on_search_vector ON ONLY p_ci_build_names USING gin (search_vector); CREATE INDEX index_p_ci_build_sources_on_pipeline_source ON ONLY p_ci_build_sources USING btree (pipeline_source); -- GitLab From b7995197b69f4ff0bdeb3b5565b96762f514a2a1 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 18:24:46 +0200 Subject: [PATCH 03/74] Remove comments after disabling transactions --- .../20250626095521_add_train_ref_commit.rb | 19 ------------------- ...equest_foreign_key_to_train_ref_commits.rb | 18 ------------------ ...roject_foreign_key_to_train_ref_commits.rb | 18 ------------------ 3 files changed, 55 deletions(-) diff --git a/db/migrate/20250626095521_add_train_ref_commit.rb b/db/migrate/20250626095521_add_train_ref_commit.rb index fe10925b454802..d47496471b6600 100644 --- a/db/migrate/20250626095521_add_train_ref_commit.rb +++ b/db/migrate/20250626095521_add_train_ref_commit.rb @@ -4,25 +4,6 @@ # for more information on how to write migrations for GitLab. class AddTrainRefCommit < Gitlab::Database::Migration[2.3] - # When using the methods "add_concurrent_index" or "remove_concurrent_index" - # you must disable the use of transactions - # as these methods can not run in an existing transaction. - # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure - # that either of them is the _only_ method called in the migration, - # any other changes should go in a separate migration. - # This ensures that upon failure _only_ the index creation or removing fails - # and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - # - # Configure the `gitlab_schema` to perform data manipulation (DML). - # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html - # restrict_gitlab_migration gitlab_schema: :gitlab_main - - # Add dependent 'batched_background_migrations.queued_migration_version' values. - # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] milestone '18.2' disable_ddl_transaction! diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb index 631ae7590e94ef..5d407351bf9cdf 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb @@ -4,25 +4,7 @@ # for more information on how to write migrations for GitLab. class AddMergeRequestForeignKeyToTrainRefCommits < Gitlab::Database::Migration[2.3] - # When using the methods "add_concurrent_index" or "remove_concurrent_index" - # you must disable the use of transactions - # as these methods can not run in an existing transaction. - # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure - # that either of them is the _only_ method called in the migration, - # any other changes should go in a separate migration. - # This ensures that upon failure _only_ the index creation or removing fails - # and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: disable_ddl_transaction! - # - # Configure the `gitlab_schema` to perform data manipulation (DML). - # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html - # restrict_gitlab_migration gitlab_schema: :gitlab_main - - # Add dependent 'batched_background_migrations.queued_migration_version' values. - # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] milestone '18.2' def up diff --git a/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb b/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb index 7c77748c73a8bb..0a945759fa9f15 100644 --- a/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb +++ b/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb @@ -4,25 +4,7 @@ # for more information on how to write migrations for GitLab. class AddProjectForeignKeyToTrainRefCommits < Gitlab::Database::Migration[2.3] - # When using the methods "add_concurrent_index" or "remove_concurrent_index" - # you must disable the use of transactions - # as these methods can not run in an existing transaction. - # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure - # that either of them is the _only_ method called in the migration, - # any other changes should go in a separate migration. - # This ensures that upon failure _only_ the index creation or removing fails - # and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: disable_ddl_transaction! - # - # Configure the `gitlab_schema` to perform data manipulation (DML). - # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html - # restrict_gitlab_migration gitlab_schema: :gitlab_main - - # Add dependent 'batched_background_migrations.queued_migration_version' values. - # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] milestone '18.2' def up -- GitLab From 9206cc1c4599053ddca60ae0930c4e42c4bf6c77 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 18:31:43 +0200 Subject: [PATCH 04/74] Update structure.sql order --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index c3f0203e02d8df..278e0a917c29ef 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -25285,7 +25285,6 @@ CREATE TABLE user_preferences ( enabled_zoekt boolean DEFAULT true NOT NULL, keyboard_shortcuts_enabled boolean DEFAULT true NOT NULL, time_display_format smallint DEFAULT 0 NOT NULL, - merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, home_organization_id bigint, early_access_program_participant boolean DEFAULT false NOT NULL, early_access_program_tracking boolean DEFAULT false NOT NULL, @@ -25295,6 +25294,7 @@ CREATE TABLE user_preferences ( dpop_enabled boolean DEFAULT false NOT NULL, use_work_items_view boolean DEFAULT false NOT NULL, text_editor_type smallint DEFAULT 0 NOT NULL, + merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, extensions_marketplace_opt_in_url text, dark_color_scheme_id smallint, work_items_display_settings jsonb DEFAULT '{}'::jsonb NOT NULL, -- GitLab From a47c6acaa5d8f1dc4231d04b68b162c5f45880f7 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 18:40:44 +0200 Subject: [PATCH 05/74] Add index to project_id --- ...dd_index_for_project_to_train_ref_commits.rb | 17 +++++++++++++++++ db/schema_migrations/20250626163546 | 1 + db/structure.sql | 2 ++ 3 files changed, 20 insertions(+) create mode 100644 db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb create mode 100644 db/schema_migrations/20250626163546 diff --git a/db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb b/db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb new file mode 100644 index 00000000000000..c980dbbe1cf228 --- /dev/null +++ b/db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexForProjectToTrainRefCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + def up + add_concurrent_index :train_ref_commits, :project_id + end + + def down + remove_concurrent_index :train_ref_commits, :project_id, name: 'index_train_ref_commits_on_project_id' + end +end diff --git a/db/schema_migrations/20250626163546 b/db/schema_migrations/20250626163546 new file mode 100644 index 00000000000000..059e4548378c13 --- /dev/null +++ b/db/schema_migrations/20250626163546 @@ -0,0 +1 @@ +39536c8fdebf269ab239b280c81de92fac4582464afaebca713cf9f2884c0c12 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 278e0a917c29ef..0ac71a0566897b 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -38758,6 +38758,8 @@ CREATE INDEX index_topics_on_slug ON topics USING btree (slug) WHERE (slug IS NO CREATE INDEX index_topics_total_projects_count ON topics USING btree (total_projects_count DESC, id); +CREATE INDEX index_train_ref_commits_on_project_id ON train_ref_commits USING btree (project_id); + CREATE UNIQUE INDEX index_train_ref_commits_on_project_mr_sha_unique ON train_ref_commits USING btree (merge_request_id, commit_sha); CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id); -- GitLab From bf66f71b5da448a3eabd6d411f48a36245a62eba Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 18:54:52 +0200 Subject: [PATCH 06/74] Remove unnecessary indexes and create one for project_id and sha --- ...251_add_unique_index_to_train_ref_commits.rb | 6 +++--- ...dd_index_for_project_to_train_ref_commits.rb | 17 ----------------- db/schema_migrations/20250626163546 | 1 - db/structure.sql | 4 +--- 4 files changed, 4 insertions(+), 24 deletions(-) delete mode 100644 db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb delete mode 100644 db/schema_migrations/20250626163546 diff --git a/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb b/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb index ce2f2049dbacdf..a660853c97e7ad 100644 --- a/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb +++ b/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb @@ -8,11 +8,11 @@ class AddUniqueIndexToTrainRefCommits < Gitlab::Database::Migration[2.3] milestone '18.2' def up - add_concurrent_index :train_ref_commits, %i[merge_request_id commit_sha], - name: 'index_train_ref_commits_on_project_mr_sha_unique', unique: true + add_concurrent_index :train_ref_commits, %i[project_id commit_sha], + name: 'index_train_ref_commits_on_project_sha_unique', unique: true end def down - remove_concurrent_index_by_name :train_ref_commits, 'index_train_ref_commits_on_project_mr_sha_unique' + remove_concurrent_index_by_name :train_ref_commits, 'index_train_ref_commits_on_project_sha_unique' end end diff --git a/db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb b/db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb deleted file mode 100644 index c980dbbe1cf228..00000000000000 --- a/db/migrate/20250626163546_add_index_for_project_to_train_ref_commits.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddIndexForProjectToTrainRefCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def up - add_concurrent_index :train_ref_commits, :project_id - end - - def down - remove_concurrent_index :train_ref_commits, :project_id, name: 'index_train_ref_commits_on_project_id' - end -end diff --git a/db/schema_migrations/20250626163546 b/db/schema_migrations/20250626163546 deleted file mode 100644 index 059e4548378c13..00000000000000 --- a/db/schema_migrations/20250626163546 +++ /dev/null @@ -1 +0,0 @@ -39536c8fdebf269ab239b280c81de92fac4582464afaebca713cf9f2884c0c12 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0ac71a0566897b..303426e94092b3 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -38758,9 +38758,7 @@ CREATE INDEX index_topics_on_slug ON topics USING btree (slug) WHERE (slug IS NO CREATE INDEX index_topics_total_projects_count ON topics USING btree (total_projects_count DESC, id); -CREATE INDEX index_train_ref_commits_on_project_id ON train_ref_commits USING btree (project_id); - -CREATE UNIQUE INDEX index_train_ref_commits_on_project_mr_sha_unique ON train_ref_commits USING btree (merge_request_id, commit_sha); +CREATE UNIQUE INDEX index_train_ref_commits_on_project_sha_unique ON train_ref_commits USING btree (project_id, commit_sha); CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id); -- GitLab From ea86052c7e8fa36c0b767ca13a836719b09df35d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 19:02:13 +0200 Subject: [PATCH 07/74] Add index for merge_request_id --- ...r_merge_request_id_to_train_ref_commits.rb | 19 +++++++++++++++++++ db/schema_migrations/20250626165904 | 1 + db/structure.sql | 2 ++ 3 files changed, 22 insertions(+) create mode 100644 db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb create mode 100644 db/schema_migrations/20250626165904 diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb new file mode 100644 index 00000000000000..e121ef09241e77 --- /dev/null +++ b/db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexForMergeRequestIdToTrainRefCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + disable_ddl_transaction! + + def up + add_concurrent_index :train_ref_commits, :merge_request_id, name: 'index_train_ref_commits_on_merge_request_id' + end + + def down + remove_concurrent_index_by_name :train_ref_commits, 'index_train_ref_commits_on_merge_request_id' + end +end diff --git a/db/schema_migrations/20250626165904 b/db/schema_migrations/20250626165904 new file mode 100644 index 00000000000000..53fe87eb1f3090 --- /dev/null +++ b/db/schema_migrations/20250626165904 @@ -0,0 +1 @@ +09917fc70b944a621a6abba4f29e9c45abe5d2e655aca343453f22e24eaf59b0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 303426e94092b3..9c9aaf45e1e5a5 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -38758,6 +38758,8 @@ CREATE INDEX index_topics_on_slug ON topics USING btree (slug) WHERE (slug IS NO CREATE INDEX index_topics_total_projects_count ON topics USING btree (total_projects_count DESC, id); +CREATE INDEX index_train_ref_commits_on_merge_request_id ON train_ref_commits USING btree (merge_request_id); + CREATE UNIQUE INDEX index_train_ref_commits_on_project_sha_unique ON train_ref_commits USING btree (project_id, commit_sha); CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id); -- GitLab From a2b3f8f88b36b396badef39d11ec356ee9f9d15d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 26 Jun 2025 19:04:55 +0200 Subject: [PATCH 08/74] Use SHA1 and disable warning --- spec/factories/train_ref_commits.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories/train_ref_commits.rb b/spec/factories/train_ref_commits.rb index e0fe4650ebf124..9e28bb951f8a30 100644 --- a/spec/factories/train_ref_commits.rb +++ b/spec/factories/train_ref_commits.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :train_ref_commit, class: 'TrainRefCommit' do - commit_sha { Digest::SHA256.hexdigest(SecureRandom.hex)[1..40] } + commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } # rubocop:disable Fips/SHA1 -- test data association :project association :merge_request end -- GitLab From bd1f54de32af0344ce7ac23a2b30f6e4e0cf1fd4 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 10:13:41 +0200 Subject: [PATCH 09/74] Change milestone back to 18.2 --- db/docs/train_ref_commits.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/docs/train_ref_commits.yml b/db/docs/train_ref_commits.yml index 06861348538947..85279c29998b9e 100644 --- a/db/docs/train_ref_commits.yml +++ b/db/docs/train_ref_commits.yml @@ -6,7 +6,7 @@ feature_categories: - merge_trains description: Stores the new commit shas for a merged merge request introduced_by_url: -milestone: '18.3' +milestone: '18.2' gitlab_schema: gitlab_main_cell sharding_key: project_id: projects -- GitLab From fa1be84443850dab709538c2faf98af871483a68 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 10:56:13 +0200 Subject: [PATCH 10/74] Rename train ref commits --- app/finders/merge_requests_finder.rb | 6 ++-- ... => generated_ref_merge_request_commit.rb} | 2 +- app/models/merge_request.rb | 6 ++-- .../merge_requests/create_ref_service.rb | 10 +++--- ...> generated_ref_merge_request_commits.yml} | 4 +-- ...ename_train_ref_commits_to_generic_name.rb | 31 +++++++++++++++++++ ...ame_train_ref_indexes_to_new_table_name.rb | 15 +++++++++ db/schema_migrations/20250627081145 | 1 + db/schema_migrations/20250627083026 | 1 + db/structure.sql | 28 ++++++++--------- ...=> generated_ref_merge_request_commits.rb} | 2 +- 11 files changed, 77 insertions(+), 29 deletions(-) rename app/models/{train_ref_commit.rb => generated_ref_merge_request_commit.rb} (86%) rename db/docs/{train_ref_commits.yml => generated_ref_merge_request_commits.yml} (74%) create mode 100644 db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb create mode 100644 db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb create mode 100644 db/schema_migrations/20250627081145 create mode 100644 db/schema_migrations/20250627083026 rename spec/factories/{train_ref_commits.rb => generated_ref_merge_request_commits.rb} (70%) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index a7c9737e2aa782..2f154e08e09d31 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -95,7 +95,7 @@ def filter_items(_items) items = by_blob_path(items) items = by_no_review_requested_or_only_user(items) items = by_review_states_or_no_reviewer(items) - items = by_train_ref_commit_sha(items) + items = by_generated_ref_merge_request_commit(items) by_valid_or_no_reviewers(items) end @@ -126,12 +126,12 @@ def sort(items) # its commit_shas get altered e.g. by a rebase, these "new" commits do not belong # to a merge request anymore. Nevertheless, we have the functionality that whenever we show # a commit from e.g. the main branch, we also show the corresponding merge request. - def by_train_ref_commit_sha(items) + def by_generated_ref_merge_request_commit(items) return items unless params[:project_id].present? && params[:commit_sha].present? return items unless items.empty? - MergeRequest.by_project_and_commit_sha_via_train_ref( + MergeRequest.by_project_and_commit_sha_via_generated_ref_merge_request_commit( params[:project_id], params[:commit_sha] ) diff --git a/app/models/train_ref_commit.rb b/app/models/generated_ref_merge_request_commit.rb similarity index 86% rename from app/models/train_ref_commit.rb rename to app/models/generated_ref_merge_request_commit.rb index 20a33014852860..7756d9b9a65bc8 100644 --- a/app/models/train_ref_commit.rb +++ b/app/models/generated_ref_merge_request_commit.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # rubocop:disable Gitlab/BoundedContexts, Gitlab/NamespacedClass -- Will be decided during review -class TrainRefCommit < ApplicationRecord +class GeneratedRefMergeRequestCommit < ApplicationRecord include ShaAttribute sha_attribute :commit_sha diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 20355c47664151..dc079deb9e12c8 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -330,10 +330,10 @@ def public_merge_status where("source_branch = :branch OR target_branch = :branch", branch: branch_name) end - scope :by_project_and_commit_sha_via_train_ref, ->(project_id, sha) { - joins("INNER JOIN train_ref_commits ON train_ref_commits.merge_request_id = merge_requests.id") + scope :by_project_and_commit_sha_via_generated_ref_merge_request_commit, ->(project_id, sha) { + joins("INNER JOIN generated_ref_merge_request_commits ON generated_ref_merge_request_commits.merge_request_id = merge_requests.id") .where(merge_requests: { project_id: project_id }) - .where(train_ref_commits: { commit_sha: Gitlab::Database::ShaAttribute.new.serialize(sha) }) + .where(generated_ref_merge_request_commits: { commit_sha: Gitlab::Database::ShaAttribute.new.serialize(sha) }) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 46402e3a826277..fbeeee87cc4c34 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,8 +39,8 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) - # Store commits in train_ref_commits table using the final commit_sha - store_train_ref_commits(result[:commit_sha]) + # Store commits in generated_ref_merge_request_commits table using the final commit_sha + store_generated_ref_merge_request_commits(result[:commit_sha]) # Add commits data to result for external use result[:commits_data] = collect_commits_data(result[:commit_sha]) @@ -58,7 +58,7 @@ def execute delegate :target_project, to: :merge_request delegate :repository, to: :target_project - def store_train_ref_commits(final_commit_sha) + def store_generated_ref_merge_request_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha) return unless commit_shas.any? @@ -74,12 +74,12 @@ def store_train_ref_commits(final_commit_sha) end # Use upsert to handle potential duplicates - TrainRefCommit.upsert_all( + GeneratedRefMergeRequestCommit.upsert_all( records, unique_by: [:merge_request_id, :commit_sha] ) rescue StandardError => e - ServiceResponse.error(message: "Failed to store train_ref_commits for MR #{merge_request.id}: #{e.message}") + ServiceResponse.error(message: "Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") # Don't raise error to avoid breaking the main flow end diff --git a/db/docs/train_ref_commits.yml b/db/docs/generated_ref_merge_request_commits.yml similarity index 74% rename from db/docs/train_ref_commits.yml rename to db/docs/generated_ref_merge_request_commits.yml index 85279c29998b9e..21338971280c47 100644 --- a/db/docs/train_ref_commits.yml +++ b/db/docs/generated_ref_merge_request_commits.yml @@ -1,7 +1,7 @@ --- -table_name: train_ref_commits +table_name: generated_ref_merge_request_commits classes: -- TrainRefCommit +- GeneratedRefMergeRequestCommit feature_categories: - merge_trains description: Stores the new commit shas for a merged merge request diff --git a/db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb b/db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb new file mode 100644 index 00000000000000..916fae8488fbd6 --- /dev/null +++ b/db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameTrainRefCommitsToGenericName < Gitlab::Database::Migration[2.3] + # When using the methods "add_concurrent_index" or "remove_concurrent_index" + # you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + # + # Configure the `gitlab_schema` to perform data manipulation (DML). + # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html + # restrict_gitlab_migration gitlab_schema: :gitlab_main + + # Add dependent 'batched_background_migrations.queued_migration_version' values. + # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] + milestone '18.2' + + def change + rename_table :train_ref_commits, :generated_ref_merge_request_commits + end +end diff --git a/db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb b/db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb new file mode 100644 index 00000000000000..9a85cb39533d5c --- /dev/null +++ b/db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameTrainRefIndexesToNewTableName < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + def change + rename_index :generated_ref_merge_request_commits, + :index_train_ref_commits_on_project_sha_unique, + :index_generated_ref_merge_request_commits_on_project_sha_unique + end +end diff --git a/db/schema_migrations/20250627081145 b/db/schema_migrations/20250627081145 new file mode 100644 index 00000000000000..b8bd43a5e8c41b --- /dev/null +++ b/db/schema_migrations/20250627081145 @@ -0,0 +1 @@ +e441a7b4cadccdcfd00899c2c0678328880e4cb462c4a328cbf3fa773b21dcdc \ No newline at end of file diff --git a/db/schema_migrations/20250627083026 b/db/schema_migrations/20250627083026 new file mode 100644 index 00000000000000..fffc81952bad17 --- /dev/null +++ b/db/schema_migrations/20250627083026 @@ -0,0 +1 @@ +187d6711dc2f043dff2fd7e0097f1ddf5597c2f0ad5e35841a2514e2bb648348 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9c9aaf45e1e5a5..1126035238e3d4 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15133,6 +15133,15 @@ CREATE SEQUENCE fork_networks_id_seq ALTER SEQUENCE fork_networks_id_seq OWNED BY fork_networks.id; +CREATE TABLE generated_ref_merge_request_commits ( + id bigint NOT NULL, + commit_sha bytea, + project_id bigint NOT NULL, + merge_request_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +); + CREATE TABLE geo_cache_invalidation_events ( id bigint NOT NULL, key character varying NOT NULL @@ -24824,15 +24833,6 @@ CREATE SEQUENCE topics_id_seq ALTER SEQUENCE topics_id_seq OWNED BY topics.id; -CREATE TABLE train_ref_commits ( - id bigint NOT NULL, - commit_sha bytea, - project_id bigint NOT NULL, - merge_request_id bigint NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL -); - CREATE TABLE trending_projects ( id bigint NOT NULL, project_id bigint NOT NULL @@ -36270,6 +36270,10 @@ CREATE INDEX index_fork_networks_on_organization_id ON fork_networks USING btree CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USING btree (root_project_id); +CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON generated_ref_merge_request_commits USING btree (merge_request_id); + +CREATE UNIQUE INDEX index_generated_ref_merge_request_commits_on_project_sha_unique ON generated_ref_merge_request_commits USING btree (project_id, commit_sha); + CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); CREATE INDEX index_geo_event_log_on_geo_event_id ON geo_event_log USING btree (geo_event_id) WHERE (geo_event_id IS NOT NULL); @@ -38758,10 +38762,6 @@ CREATE INDEX index_topics_on_slug ON topics USING btree (slug) WHERE (slug IS NO CREATE INDEX index_topics_total_projects_count ON topics USING btree (total_projects_count DESC, id); -CREATE INDEX index_train_ref_commits_on_merge_request_id ON train_ref_commits USING btree (merge_request_id); - -CREATE UNIQUE INDEX index_train_ref_commits_on_project_sha_unique ON train_ref_commits USING btree (project_id, commit_sha); - CREATE UNIQUE INDEX index_trending_projects_on_project_id ON trending_projects USING btree (project_id); CREATE INDEX index_unarchived_occurrences_for_aggregations_component_name ON sbom_occurrences USING btree (traversal_ids, component_name, component_id, component_version_id) WHERE (archived = false); @@ -44080,7 +44080,7 @@ ALTER TABLE ONLY merge_requests_approval_rules_merge_requests ALTER TABLE ONLY software_license_policies ADD CONSTRAINT fk_74f6d8328a FOREIGN KEY (custom_software_license_id) REFERENCES custom_software_licenses(id) ON DELETE CASCADE; -ALTER TABLE ONLY train_ref_commits +ALTER TABLE ONLY generated_ref_merge_request_commits ADD CONSTRAINT fk_74fa3f61ab FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; ALTER TABLE ONLY cluster_agent_tokens diff --git a/spec/factories/train_ref_commits.rb b/spec/factories/generated_ref_merge_request_commits.rb similarity index 70% rename from spec/factories/train_ref_commits.rb rename to spec/factories/generated_ref_merge_request_commits.rb index 9e28bb951f8a30..7b549143ac0e46 100644 --- a/spec/factories/train_ref_commits.rb +++ b/spec/factories/generated_ref_merge_request_commits.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :train_ref_commit, class: 'TrainRefCommit' do + factory :generated_ref_merge_request_commit, class: 'GeneratedRefMergeRequestCommit' do commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } # rubocop:disable Fips/SHA1 -- test data association :project association :merge_request -- GitLab From ec819ea1b2975244865b652ba8a8fb3774713715 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 11:22:13 +0200 Subject: [PATCH 11/74] Fix table renaming migrations --- ...add_generated_ref_merge_request_commit.rb} | 4 +-- ...to_generated_ref_merge_request_commits.rb} | 6 ++-- ..._to_generated_ref_merge_request_commits.rb | 19 ++++++++++++ ...roject_foreign_key_to_train_ref_commits.rb | 19 ------------ ..._to_generated_ref_merge_request_commits.rb | 21 +++++++++++++ ...1_add_unique_index_to_train_ref_commits.rb | 18 ----------- ..._to_generated_ref_merge_request_commits.rb | 26 ++++++++++++++++ ...r_merge_request_id_to_train_ref_commits.rb | 19 ------------ ...ename_train_ref_commits_to_generic_name.rb | 31 ------------------- ...ame_train_ref_indexes_to_new_table_name.rb | 15 --------- db/schema_migrations/20250627081145 | 1 - db/schema_migrations/20250627083026 | 1 - db/structure.sql | 25 +++++++++++++-- 13 files changed, 93 insertions(+), 112 deletions(-) rename db/migrate/{20250626095521_add_train_ref_commit.rb => 20250626095521_add_generated_ref_merge_request_commit.rb} (82%) rename db/migrate/{20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb => 20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb} (54%) create mode 100644 db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb delete mode 100644 db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb create mode 100644 db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb delete mode 100644 db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb create mode 100644 db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb delete mode 100644 db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb delete mode 100644 db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb delete mode 100644 db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb delete mode 100644 db/schema_migrations/20250627081145 delete mode 100644 db/schema_migrations/20250627083026 diff --git a/db/migrate/20250626095521_add_train_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb similarity index 82% rename from db/migrate/20250626095521_add_train_ref_commit.rb rename to db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb index d47496471b6600..ac219a3685b3d7 100644 --- a/db/migrate/20250626095521_add_train_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb @@ -3,13 +3,13 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddTrainRefCommit < Gitlab::Database::Migration[2.3] +class AddGeneratedRefMergeRequestCommit < Gitlab::Database::Migration[2.3] milestone '18.2' disable_ddl_transaction! def change # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found - create_table :train_ref_commits do |t| + create_table :generated_ref_merge_request_commits do |t| t.binary :commit_sha, limit: 20 t.bigint :project_id, null: false t.bigint :merge_request_id, null: false diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb similarity index 54% rename from db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb rename to db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb index 5d407351bf9cdf..e561c7029b330c 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_train_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb @@ -3,19 +3,19 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddMergeRequestForeignKeyToTrainRefCommits < Gitlab::Database::Migration[2.3] +class AddMergeRequestForeignKeyToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' def up - add_concurrent_foreign_key :train_ref_commits, :merge_requests, + add_concurrent_foreign_key :generated_ref_merge_request_commits, :merge_requests, column: :merge_request_id, on_delete: :cascade end def down with_lock_retries do - remove_foreign_key_if_exists(:train_ref_commits, column: :merge_request_id) + remove_foreign_key_if_exists(:generated_ref_merge_request_commits, column: :merge_request_id) end end end diff --git a/db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb new file mode 100644 index 00000000000000..ebb4088bea7d36 --- /dev/null +++ b/db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddProjectForeignKeyToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + def up + add_concurrent_foreign_key :generated_ref_merge_request_commits, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :generated_ref_merge_request_commits, column: :project_id + end + end +end diff --git a/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb b/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb deleted file mode 100644 index 0a945759fa9f15..00000000000000 --- a/db/migrate/20250626123902_add_project_foreign_key_to_train_ref_commits.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddProjectForeignKeyToTrainRefCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def up - add_concurrent_foreign_key :train_ref_commits, :projects, column: :project_id, on_delete: :cascade - end - - def down - with_lock_retries do - remove_foreign_key :train_ref_commits, column: :project_id - end - end -end diff --git a/db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb new file mode 100644 index 00000000000000..48103921e790e9 --- /dev/null +++ b/db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddUniqueIndexToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + def up + add_concurrent_index :generated_ref_merge_request_commits, %i[project_id commit_sha], + name: 'index_generated_ref_merge_request_commits_on_project_sha_unique', unique: true + end + + def down + remove_concurrent_index_by_name( + :generated_ref_merge_request_commits, + 'index_generated_ref_merge_request_commits_on_project_sha_unique' + ) + end +end diff --git a/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb b/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb deleted file mode 100644 index a660853c97e7ad..00000000000000 --- a/db/migrate/20250626124251_add_unique_index_to_train_ref_commits.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddUniqueIndexToTrainRefCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def up - add_concurrent_index :train_ref_commits, %i[project_id commit_sha], - name: 'index_train_ref_commits_on_project_sha_unique', unique: true - end - - def down - remove_concurrent_index_by_name :train_ref_commits, 'index_train_ref_commits_on_project_sha_unique' - end -end diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb new file mode 100644 index 00000000000000..36e0fb9c94e340 --- /dev/null +++ b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddIndexForMergeRequestIdToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + disable_ddl_transaction! + + def up + add_concurrent_index( + :generated_ref_merge_request_commits, + :merge_request_id, + name: 'index_generated_ref_merge_request_commits_on_merge_request_id' + ) + end + + def down + remove_concurrent_index_by_name( + :generated_ref_merge_request_commits, + 'index_generated_ref_merge_request_commits_on_merge_request_id' + ) + end +end diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb deleted file mode 100644 index e121ef09241e77..00000000000000 --- a/db/migrate/20250626165904_add_index_for_merge_request_id_to_train_ref_commits.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddIndexForMergeRequestIdToTrainRefCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - disable_ddl_transaction! - - def up - add_concurrent_index :train_ref_commits, :merge_request_id, name: 'index_train_ref_commits_on_merge_request_id' - end - - def down - remove_concurrent_index_by_name :train_ref_commits, 'index_train_ref_commits_on_merge_request_id' - end -end diff --git a/db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb b/db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb deleted file mode 100644 index 916fae8488fbd6..00000000000000 --- a/db/migrate/20250627081145_rename_train_ref_commits_to_generic_name.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class RenameTrainRefCommitsToGenericName < Gitlab::Database::Migration[2.3] - # When using the methods "add_concurrent_index" or "remove_concurrent_index" - # you must disable the use of transactions - # as these methods can not run in an existing transaction. - # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure - # that either of them is the _only_ method called in the migration, - # any other changes should go in a separate migration. - # This ensures that upon failure _only_ the index creation or removing fails - # and can be retried or reverted easily. - # - # To disable transactions uncomment the following line and remove these - # comments: - # disable_ddl_transaction! - # - # Configure the `gitlab_schema` to perform data manipulation (DML). - # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html - # restrict_gitlab_migration gitlab_schema: :gitlab_main - - # Add dependent 'batched_background_migrations.queued_migration_version' values. - # DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS = [] - milestone '18.2' - - def change - rename_table :train_ref_commits, :generated_ref_merge_request_commits - end -end diff --git a/db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb b/db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb deleted file mode 100644 index 9a85cb39533d5c..00000000000000 --- a/db/migrate/20250627083026_rename_train_ref_indexes_to_new_table_name.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class RenameTrainRefIndexesToNewTableName < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def change - rename_index :generated_ref_merge_request_commits, - :index_train_ref_commits_on_project_sha_unique, - :index_generated_ref_merge_request_commits_on_project_sha_unique - end -end diff --git a/db/schema_migrations/20250627081145 b/db/schema_migrations/20250627081145 deleted file mode 100644 index b8bd43a5e8c41b..00000000000000 --- a/db/schema_migrations/20250627081145 +++ /dev/null @@ -1 +0,0 @@ -e441a7b4cadccdcfd00899c2c0678328880e4cb462c4a328cbf3fa773b21dcdc \ No newline at end of file diff --git a/db/schema_migrations/20250627083026 b/db/schema_migrations/20250627083026 deleted file mode 100644 index fffc81952bad17..00000000000000 --- a/db/schema_migrations/20250627083026 +++ /dev/null @@ -1 +0,0 @@ -187d6711dc2f043dff2fd7e0097f1ddf5597c2f0ad5e35841a2514e2bb648348 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 1126035238e3d4..5c1ecba2fe6cb2 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15142,6 +15142,15 @@ CREATE TABLE generated_ref_merge_request_commits ( updated_at timestamp with time zone NOT NULL ); +CREATE SEQUENCE generated_ref_merge_request_commits_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE generated_ref_merge_request_commits_id_seq OWNED BY generated_ref_merge_request_commits.id; + CREATE TABLE geo_cache_invalidation_events ( id bigint NOT NULL, key character varying NOT NULL @@ -28261,6 +28270,8 @@ ALTER TABLE ONLY fork_network_members ALTER COLUMN id SET DEFAULT nextval('fork_ ALTER TABLE ONLY fork_networks ALTER COLUMN id SET DEFAULT nextval('fork_networks_id_seq'::regclass); +ALTER TABLE ONLY generated_ref_merge_request_commits ALTER COLUMN id SET DEFAULT nextval('generated_ref_merge_request_commits_id_seq'::regclass); + ALTER TABLE ONLY geo_cache_invalidation_events ALTER COLUMN id SET DEFAULT nextval('geo_cache_invalidation_events_id_seq'::regclass); ALTER TABLE ONLY geo_event_log ALTER COLUMN id SET DEFAULT nextval('geo_event_log_id_seq'::regclass); @@ -30798,6 +30809,9 @@ ALTER TABLE ONLY fork_network_members ALTER TABLE ONLY fork_networks ADD CONSTRAINT fork_networks_pkey PRIMARY KEY (id); +ALTER TABLE ONLY generated_ref_merge_request_commits + ADD CONSTRAINT generated_ref_merge_request_commits_pkey PRIMARY KEY (id); + ALTER TABLE ONLY geo_cache_invalidation_events ADD CONSTRAINT geo_cache_invalidation_events_pkey PRIMARY KEY (id); @@ -36270,6 +36284,8 @@ CREATE INDEX index_fork_networks_on_organization_id ON fork_networks USING btree CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USING btree (root_project_id); +CREATE INDEX index_generated_ref_merge_request_commits_on_commit_sha ON generated_ref_merge_request_commits USING btree (commit_sha); + CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON generated_ref_merge_request_commits USING btree (merge_request_id); CREATE UNIQUE INDEX index_generated_ref_merge_request_commits_on_project_sha_unique ON generated_ref_merge_request_commits USING btree (project_id, commit_sha); @@ -43588,6 +43604,9 @@ ALTER TABLE ONLY incident_management_timeline_events ALTER TABLE ONLY import_export_uploads ADD CONSTRAINT fk_38e11735aa FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY generated_ref_merge_request_commits + ADD CONSTRAINT fk_3910965286 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY approval_group_rules_users ADD CONSTRAINT fk_3995d73930 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; @@ -44080,9 +44099,6 @@ ALTER TABLE ONLY merge_requests_approval_rules_merge_requests ALTER TABLE ONLY software_license_policies ADD CONSTRAINT fk_74f6d8328a FOREIGN KEY (custom_software_license_id) REFERENCES custom_software_licenses(id) ON DELETE CASCADE; -ALTER TABLE ONLY generated_ref_merge_request_commits - ADD CONSTRAINT fk_74fa3f61ab FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; - ALTER TABLE ONLY cluster_agent_tokens ADD CONSTRAINT fk_75008f3553 FOREIGN KEY (created_by_user_id) REFERENCES users(id) ON DELETE SET NULL; @@ -44701,6 +44717,9 @@ ALTER TABLE ONLY description_versions ALTER TABLE ONLY bulk_import_entities ADD CONSTRAINT fk_b69fa2b2df FOREIGN KEY (bulk_import_id) REFERENCES bulk_imports(id) ON DELETE CASCADE; +ALTER TABLE ONLY generated_ref_merge_request_commits + ADD CONSTRAINT fk_b6cc4921de FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + ALTER TABLE ONLY security_policy_requirements ADD CONSTRAINT fk_b6e48e3428 FOREIGN KEY (compliance_framework_security_policy_id) REFERENCES compliance_framework_security_policies(id) ON DELETE CASCADE; -- GitLab From e1822cafec6bbee24354110138627e974982b2e8 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 11:38:22 +0200 Subject: [PATCH 12/74] Add comment for lookup method; track exception for dev --- app/finders/merge_requests_finder.rb | 14 +++++++++----- app/services/merge_requests/create_ref_service.rb | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 2f154e08e09d31..aa7905cf1388ca 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -121,11 +121,15 @@ def sort(items) items.group(grouping_columns) # rubocop:disable CodeReuse/ActiveRecord end - # This method is used as a fallback for now. - # We have the problem that whenever a merge_request gets merged and - # its commit_shas get altered e.g. by a rebase, these "new" commits do not belong - # to a merge request anymore. Nevertheless, we have the functionality that whenever we show - # a commit from e.g. the main branch, we also show the corresponding merge request. + # This method handles commit lookup for gitlab generated refs used to run pipelines and merge code and associated with + # merge requests. When merge requests are processed through merge trains, new commit SHAs are generated that + # are not stored in the original merge request diff. This method provides a way to find the + # merge request associated with GitLab-generated references for MR pipelines. These commits + # are merged/rebased into the target branch via MergeRequests::MergeStrategies::FromSourceBranch + # and MergeRequests::MergeStrategies::FromTrainRef. + # Lookup of Refs generated via `MergeRequests::MergeToRefService` is not currently supported + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/421025 + def by_generated_ref_merge_request_commit(items) return items unless params[:project_id].present? && params[:commit_sha].present? diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index fbeeee87cc4c34..5e1fda87b92e5a 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -99,6 +99,7 @@ def collect_commits_data(final_commit_sha) end rescue StandardError => e ServiceResponse.error(message: "Failed to collect commits data for MR #{merge_request.id}: #{e.message}") + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) [] end -- GitLab From 694a6f58b6ef259694f319e48d660c16f3c51c92 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 12:41:55 +0200 Subject: [PATCH 13/74] Remove project id; cleanup migrations; add back unique index --- .../merge_requests/create_ref_service.rb | 3 +-- ..._add_generated_ref_merge_request_commit.rb | 3 --- ..._to_generated_ref_merge_request_commits.rb | 19 ---------------- ..._to_generated_ref_merge_request_commits.rb | 21 ------------------ ...27103145_add_unique_index_on_commit_sha.rb | 22 +++++++++++++++++++ db/schema_migrations/20250626123902 | 1 - db/schema_migrations/20250626124251 | 1 - db/schema_migrations/20250627103145 | 1 + db/structure.sql | 10 ++------- 9 files changed, 26 insertions(+), 55 deletions(-) delete mode 100644 db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb delete mode 100644 db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb create mode 100644 db/migrate/20250627103145_add_unique_index_on_commit_sha.rb delete mode 100644 db/schema_migrations/20250626123902 delete mode 100644 db/schema_migrations/20250626124251 create mode 100644 db/schema_migrations/20250627103145 diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 5e1fda87b92e5a..b24865c18ea6b9 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -67,7 +67,6 @@ def store_generated_ref_merge_request_commits(final_commit_sha) { merge_request_id: merge_request.id, commit_sha: commit_sha, - project_id: merge_request.project_id, created_at: Time.current, updated_at: Time.current } @@ -76,7 +75,7 @@ def store_generated_ref_merge_request_commits(final_commit_sha) # Use upsert to handle potential duplicates GeneratedRefMergeRequestCommit.upsert_all( records, - unique_by: [:merge_request_id, :commit_sha] + unique_by: :commit_sha ) rescue StandardError => e ServiceResponse.error(message: "Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") diff --git a/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb b/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb index ac219a3685b3d7..540eb5e3b6f635 100644 --- a/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb @@ -11,11 +11,8 @@ def change # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found create_table :generated_ref_merge_request_commits do |t| t.binary :commit_sha, limit: 20 - t.bigint :project_id, null: false t.bigint :merge_request_id, null: false t.timestamps_with_timezone - - t.index :commit_sha end # rubocop:enable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found end diff --git a/db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb deleted file mode 100644 index ebb4088bea7d36..00000000000000 --- a/db/migrate/20250626123902_add_project_foreign_key_to_generated_ref_merge_request_commits.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddProjectForeignKeyToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def up - add_concurrent_foreign_key :generated_ref_merge_request_commits, :projects, column: :project_id, on_delete: :cascade - end - - def down - with_lock_retries do - remove_foreign_key :generated_ref_merge_request_commits, column: :project_id - end - end -end diff --git a/db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb deleted file mode 100644 index 48103921e790e9..00000000000000 --- a/db/migrate/20250626124251_add_unique_index_to_generated_ref_merge_request_commits.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddUniqueIndexToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def up - add_concurrent_index :generated_ref_merge_request_commits, %i[project_id commit_sha], - name: 'index_generated_ref_merge_request_commits_on_project_sha_unique', unique: true - end - - def down - remove_concurrent_index_by_name( - :generated_ref_merge_request_commits, - 'index_generated_ref_merge_request_commits_on_project_sha_unique' - ) - end -end diff --git a/db/migrate/20250627103145_add_unique_index_on_commit_sha.rb b/db/migrate/20250627103145_add_unique_index_on_commit_sha.rb new file mode 100644 index 00000000000000..47778fbe653145 --- /dev/null +++ b/db/migrate/20250627103145_add_unique_index_on_commit_sha.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddUniqueIndexOnCommitSha < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + INDEX_NAME = :index_grmrc_on_commit_sha + + def up + add_concurrent_index :generated_ref_merge_request_commits, + :commit_sha, + unique: true, + name: INDEX_NAME + end + + def down + remove_concurrent_index_by_name :generated_ref_merge_request_commits, INDEX_NAME + end +end diff --git a/db/schema_migrations/20250626123902 b/db/schema_migrations/20250626123902 deleted file mode 100644 index 7f31d0d9b44a56..00000000000000 --- a/db/schema_migrations/20250626123902 +++ /dev/null @@ -1 +0,0 @@ -00eaa073c518276fe40f7d405010fe8b47999ced66f4986b37ec2cc2977b89ef \ No newline at end of file diff --git a/db/schema_migrations/20250626124251 b/db/schema_migrations/20250626124251 deleted file mode 100644 index 39ca8396f8a5f7..00000000000000 --- a/db/schema_migrations/20250626124251 +++ /dev/null @@ -1 +0,0 @@ -b1d06ed4e73075c98e19290a469a9acbf8a00557634a0b6a8f17e383af1e5efe \ No newline at end of file diff --git a/db/schema_migrations/20250627103145 b/db/schema_migrations/20250627103145 new file mode 100644 index 00000000000000..d026134fd938f0 --- /dev/null +++ b/db/schema_migrations/20250627103145 @@ -0,0 +1 @@ +297abd260ff7f8efb8f8e85bdc7353eb7cb55c845705b5e9d9c9765fb781fdc1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 5c1ecba2fe6cb2..fad15d9bc1bdee 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15136,7 +15136,6 @@ ALTER SEQUENCE fork_networks_id_seq OWNED BY fork_networks.id; CREATE TABLE generated_ref_merge_request_commits ( id bigint NOT NULL, commit_sha bytea, - project_id bigint NOT NULL, merge_request_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL @@ -36284,12 +36283,8 @@ CREATE INDEX index_fork_networks_on_organization_id ON fork_networks USING btree CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USING btree (root_project_id); -CREATE INDEX index_generated_ref_merge_request_commits_on_commit_sha ON generated_ref_merge_request_commits USING btree (commit_sha); - CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON generated_ref_merge_request_commits USING btree (merge_request_id); -CREATE UNIQUE INDEX index_generated_ref_merge_request_commits_on_project_sha_unique ON generated_ref_merge_request_commits USING btree (project_id, commit_sha); - CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); CREATE INDEX index_geo_event_log_on_geo_event_id ON geo_event_log USING btree (geo_event_id) WHERE (geo_event_id IS NOT NULL); @@ -36354,6 +36349,8 @@ CREATE INDEX index_grafana_integrations_on_enabled ON grafana_integrations USING CREATE INDEX index_grafana_integrations_on_project_id ON grafana_integrations USING btree (project_id); +CREATE UNIQUE INDEX index_grmrc_on_commit_sha ON generated_ref_merge_request_commits USING btree (commit_sha); + CREATE INDEX index_group_crm_settings_on_group_id ON group_crm_settings USING btree (group_id); CREATE INDEX index_group_crm_settings_on_source_group_id ON group_crm_settings USING btree (source_group_id); @@ -43604,9 +43601,6 @@ ALTER TABLE ONLY incident_management_timeline_events ALTER TABLE ONLY import_export_uploads ADD CONSTRAINT fk_38e11735aa FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; -ALTER TABLE ONLY generated_ref_merge_request_commits - ADD CONSTRAINT fk_3910965286 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; - ALTER TABLE ONLY approval_group_rules_users ADD CONSTRAINT fk_3995d73930 FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; -- GitLab From 046402490ad88f9a6869da8254e8a4421621814c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 13:07:13 +0200 Subject: [PATCH 14/74] Fix sharding key --- db/docs/generated_ref_merge_request_commits.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/docs/generated_ref_merge_request_commits.yml b/db/docs/generated_ref_merge_request_commits.yml index 21338971280c47..2365169a3582e0 100644 --- a/db/docs/generated_ref_merge_request_commits.yml +++ b/db/docs/generated_ref_merge_request_commits.yml @@ -9,5 +9,5 @@ introduced_by_url: milestone: '18.2' gitlab_schema: gitlab_main_cell sharding_key: - project_id: projects + merge_request_id: merge_requests table_size: small -- GitLab From cddb7a7bb898bdd8e082ad81ac34ba7ee4a99631 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 27 Jun 2025 19:24:46 +0200 Subject: [PATCH 15/74] Add back project_id as a sharding key --- app/models/merge_request.rb | 4 +- .../merge_requests/create_ref_service.rb | 1 + .../generated_ref_merge_request_commits.yml | 2 +- ..._to_generated_ref_merge_request_commits.rb | 37 +++++++++++++++++++ db/schema_migrations/20250627165139 | 1 + db/structure.sql | 8 +++- 6 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb create mode 100644 db/schema_migrations/20250627165139 diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index dc079deb9e12c8..da2fb0d1671b9a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -333,7 +333,9 @@ def public_merge_status scope :by_project_and_commit_sha_via_generated_ref_merge_request_commit, ->(project_id, sha) { joins("INNER JOIN generated_ref_merge_request_commits ON generated_ref_merge_request_commits.merge_request_id = merge_requests.id") .where(merge_requests: { project_id: project_id }) - .where(generated_ref_merge_request_commits: { commit_sha: Gitlab::Database::ShaAttribute.new.serialize(sha) }) + .where(generated_ref_merge_request_commits: { + commit_sha: Gitlab::Database::ShaAttribute.new.serialize(sha) + }) } scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index b24865c18ea6b9..7addf4992175c4 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -67,6 +67,7 @@ def store_generated_ref_merge_request_commits(final_commit_sha) { merge_request_id: merge_request.id, commit_sha: commit_sha, + project_id: merge_request.project_id, created_at: Time.current, updated_at: Time.current } diff --git a/db/docs/generated_ref_merge_request_commits.yml b/db/docs/generated_ref_merge_request_commits.yml index 2365169a3582e0..21338971280c47 100644 --- a/db/docs/generated_ref_merge_request_commits.yml +++ b/db/docs/generated_ref_merge_request_commits.yml @@ -9,5 +9,5 @@ introduced_by_url: milestone: '18.2' gitlab_schema: gitlab_main_cell sharding_key: - merge_request_id: merge_requests + project_id: projects table_size: small diff --git a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb b/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb new file mode 100644 index 00000000000000..d4b661e11f7a33 --- /dev/null +++ b/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddProjectIdToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.2' + + INDEX_NAME = 'index_generated_ref_mr_commits_on_project_id' + FK_NAME = 'fk_generated_ref_mr_commits_project_id' + + def up + # rubocop:disable Rails/NotNullColumn -- empty table + add_column :generated_ref_merge_request_commits, :project_id, :bigint, null: false + # rubocop:enable Rails/NotNullColumn -- empty table + add_concurrent_index :generated_ref_merge_request_commits, :project_id, name: INDEX_NAME + + add_concurrent_foreign_key :generated_ref_merge_request_commits, + :projects, + column: :project_id, + name: FK_NAME + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :generated_ref_merge_request_commits, + column: :project_id + end + + remove_concurrent_index :generated_ref_merge_request_commits, + :project_id, + name: INDEX_NAME + + remove_column :generated_ref_merge_request_commits, :project_id + end +end diff --git a/db/schema_migrations/20250627165139 b/db/schema_migrations/20250627165139 new file mode 100644 index 00000000000000..ed9f2f889081d8 --- /dev/null +++ b/db/schema_migrations/20250627165139 @@ -0,0 +1 @@ +2a93d8e609944e024fde46f3383fe71f0534e9e083a03706e924b3d4b447280f \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index fad15d9bc1bdee..39c9f7ddf53f99 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15138,7 +15138,8 @@ CREATE TABLE generated_ref_merge_request_commits ( commit_sha bytea, merge_request_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL + updated_at timestamp with time zone NOT NULL, + project_id bigint NOT NULL ); CREATE SEQUENCE generated_ref_merge_request_commits_id_seq @@ -36285,6 +36286,8 @@ CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USIN CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON generated_ref_merge_request_commits USING btree (merge_request_id); +CREATE INDEX index_generated_ref_mr_commits_on_project_id ON generated_ref_merge_request_commits USING btree (project_id); + CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); CREATE INDEX index_geo_event_log_on_geo_event_id ON geo_event_log USING btree (geo_event_id) WHERE (geo_event_id IS NOT NULL); @@ -45383,6 +45386,9 @@ ALTER TABLE ONLY packages_conan_package_revisions ALTER TABLE ONLY issues ADD CONSTRAINT fk_ffed080f01 FOREIGN KEY (updated_by_id) REFERENCES users(id) ON DELETE SET NULL; +ALTER TABLE ONLY generated_ref_merge_request_commits + ADD CONSTRAINT fk_generated_ref_mr_commits_project_id FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY geo_event_log ADD CONSTRAINT fk_geo_event_log_on_geo_event_id FOREIGN KEY (geo_event_id) REFERENCES geo_events(id) ON DELETE CASCADE; -- GitLab From c65aa33afe4b5d13ba00977797d557a78941dc3c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Sat, 28 Jun 2025 10:12:09 +0200 Subject: [PATCH 16/74] Add track for dev error tracking --- app/services/merge_requests/create_ref_service.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 7addf4992175c4..486660583d98e2 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -80,6 +80,7 @@ def store_generated_ref_merge_request_commits(final_commit_sha) ) rescue StandardError => e ServiceResponse.error(message: "Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) # Don't raise error to avoid breaking the main flow end -- GitLab From 6fe56d47a2c9d460e70410564db8a26a3716c5d4 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Sat, 28 Jun 2025 10:56:14 +0200 Subject: [PATCH 17/74] Add guard clause to only save commits when necessary --- app/models/project.rb | 4 ++++ app/services/merge_requests/create_ref_service.rb | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 3e648b46af5227..ea6f7cc5f20c46 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3001,6 +3001,10 @@ def merge_method end end + def generate_ref_commits? + merge_method != :merge + end + def merge_method=(method) case method.to_s when "ff" diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 486660583d98e2..8d68c98ccc1d11 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -60,7 +60,7 @@ def execute def store_generated_ref_merge_request_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha) - return unless commit_shas.any? + return unless commit_shas.any? && target_project.generate_ref_commits? # Prepare records for bulk insert records = commit_shas.map do |commit_sha| -- GitLab From c606dbc229fbe8cfc27b63be6ee0aa8dffc65243 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Sat, 28 Jun 2025 12:28:19 +0200 Subject: [PATCH 18/74] Add uniqueness constraint and basic specs --- .../generated_ref_merge_request_commit.rb | 2 +- .../merge_trains/create_ref_service_spec.rb | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/app/models/generated_ref_merge_request_commit.rb b/app/models/generated_ref_merge_request_commit.rb index 7756d9b9a65bc8..edfe62eae66f81 100644 --- a/app/models/generated_ref_merge_request_commit.rb +++ b/app/models/generated_ref_merge_request_commit.rb @@ -8,6 +8,6 @@ class GeneratedRefMergeRequestCommit < ApplicationRecord belongs_to :merge_request belongs_to :project - validates :commit_sha, presence: true + validates :commit_sha, presence: true, uniqueness: true end # rubocop:enable Gitlab/BoundedContexts, Gitlab/NamespacedClass -- Will be decided during review diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index 532935b9cfb651..9593f2a185dee9 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -129,9 +129,29 @@ end end + shared_examples 'generate ref merge request commits' do |commit_count| + it 'created generated ref merge request commits' do + result + + expect(GeneratedRefMergeRequestCommit.where.not(merge_request_id: merge_request.id)).to be_empty + expect(GeneratedRefMergeRequestCommit.where(merge_request_id: merge_request.id).count).to eq( + commit_count + ) + end + end + + shared_examples 'does not generate ref merge request commits' do + it 'created generated ref merge request commits' do + result + + expect(GeneratedRefMergeRequestCommit.exists?).to be false + end + end + context 'when merged commit strategy' do include_examples 'merge commits without squash' include_examples 'merge commits with squash' + include_examples 'does not generate ref merge request commits' end context 'when semi-linear merge strategy' do @@ -142,6 +162,7 @@ include_examples 'merge commits without squash' include_examples 'merge commits with squash' + include_examples 'generate ref merge request commits', 3 end context 'when fast-forward merge strategy' do @@ -151,6 +172,7 @@ end it_behaves_like 'writing without a merge commit' + it_behaves_like 'generate ref merge request commits', 2 context 'when squash set' do let(:squash) { true } -- GitLab From 5dae7ce60087b3614d3cf79e4822eb1c19b77e06 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 1 Jul 2025 16:18:09 +0200 Subject: [PATCH 19/74] Add removal of refs for not merge merge requests --- ee/app/models/merge_trains/car.rb | 3 +++ ee/spec/models/merge_trains/car_spec.rb | 34 +++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 7b5fe4e872efa7..8c3dc359b649b9 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -204,6 +204,9 @@ def train private def cleanup_ref(async: true) + # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. + GeneratedRefMergeRequestCommit.where(merge_request: merge_request).delete_all unless merge_request.merged? + if async merge_request.schedule_cleanup_refs(only: :train) else diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 64526f1c6a9c0f..151d43421c8a40 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -519,6 +519,40 @@ describe '#try_cleanup_ref' do let(:train_car) { create(:merge_train_car) } + let(:generated_ref_merge_request_commits) { class_double(GeneratedRefMergeRequestCommit) } + + context 'when a merge request is merged' do + subject { train_car.try_cleanup_ref } + + before do + stub_const('GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) + allow(generated_ref_merge_request_commits).to receive(:where) + .and_return(generated_ref_merge_request_commits) + allow(generated_ref_merge_request_commits).to receive(:delete_all) + end + + it 'does not delete created ref merge request commits' do + train_car.merge_request.mark_as_merged + expect(generated_ref_merge_request_commits).not_to receive(:delete_all) + subject + end + end + + context 'when a merge request is not merged' do + subject { train_car.try_cleanup_ref } + + before do + stub_const('GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) + allow(generated_ref_merge_request_commits).to receive(:where) + .and_return(generated_ref_merge_request_commits) + allow(generated_ref_merge_request_commits).to receive(:delete_all) + end + + it 'does delete created ref merge request commits' do + expect(generated_ref_merge_request_commits).to receive(:delete_all) + subject + end + end context 'when running async' do subject { train_car.try_cleanup_ref } -- GitLab From 3baa2b5b67a8ae2fe9acebf48f88d4a06b5402d5 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 1 Jul 2025 16:53:43 +0200 Subject: [PATCH 20/74] Add spec for generated ref merge request commit --- ...generated_ref_merge_request_commit_spec.rb | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 spec/models/generated_ref_merge_request_commit_spec.rb diff --git a/spec/models/generated_ref_merge_request_commit_spec.rb b/spec/models/generated_ref_merge_request_commit_spec.rb new file mode 100644 index 00000000000000..40e7d41d7eca50 --- /dev/null +++ b/spec/models/generated_ref_merge_request_commit_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe GeneratedRefMergeRequestCommit do # rubocop:disable RSpec/FeatureCategory -- to be determined during review + let_it_be(:project) { create(:project) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + + subject(:ref_commit) do + build(:generated_ref_merge_request_commit, project: project, merge_request: merge_request) + end + + describe 'associations' do + it { is_expected.to belong_to(:merge_request) } + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:commit_sha) } + # Since git gives us a lowercase hash, we should never encouter the case, where a commit sha + # with upper case letters will be saved. We only "fix" the shoulda matcher here. + it { is_expected.to validate_uniqueness_of(:commit_sha).case_insensitive } + end + + describe 'factory behavior' do + it 'is valid with valid attributes' do + expect(ref_commit).to be_valid + end + + it 'is not valid without a commit_sha' do + ref_commit.commit_sha = nil + expect(ref_commit).not_to be_valid + end + + it 'is not valid with duplicate commit_sha' do + described_class.create!( + commit_sha: ref_commit.commit_sha, + merge_request: merge_request, + project: project + ) + + expect(ref_commit).not_to be_valid + end + end +end -- GitLab From 4f8757dcb0076149ce5d0d933cd81c04c1664d74 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 10:32:20 +0200 Subject: [PATCH 21/74] Move files to merge_requests namespace --- app/models/generated_ref_merge_request_commit.rb | 13 ------------- .../generated_ref_merge_request_commit.rb | 13 +++++++++++++ db/docs/generated_ref_merge_request_commits.yml | 2 +- ee/app/models/merge_trains/car.rb | 4 +++- ee/spec/models/merge_trains/car_spec.rb | 6 +++--- .../merge_trains/create_ref_service_spec.rb | 7 ++++--- .../generated_ref_merge_request_commits.rb | 2 +- .../generated_ref_merge_request_commit_spec.rb | 2 +- 8 files changed, 26 insertions(+), 23 deletions(-) delete mode 100644 app/models/generated_ref_merge_request_commit.rb create mode 100644 app/models/merge_requests/generated_ref_merge_request_commit.rb rename spec/models/{ => merge_requests}/generated_ref_merge_request_commit_spec.rb (91%) diff --git a/app/models/generated_ref_merge_request_commit.rb b/app/models/generated_ref_merge_request_commit.rb deleted file mode 100644 index edfe62eae66f81..00000000000000 --- a/app/models/generated_ref_merge_request_commit.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -# rubocop:disable Gitlab/BoundedContexts, Gitlab/NamespacedClass -- Will be decided during review -class GeneratedRefMergeRequestCommit < ApplicationRecord - include ShaAttribute - - sha_attribute :commit_sha - - belongs_to :merge_request - belongs_to :project - validates :commit_sha, presence: true, uniqueness: true -end -# rubocop:enable Gitlab/BoundedContexts, Gitlab/NamespacedClass -- Will be decided during review diff --git a/app/models/merge_requests/generated_ref_merge_request_commit.rb b/app/models/merge_requests/generated_ref_merge_request_commit.rb new file mode 100644 index 00000000000000..6d8a344c451f91 --- /dev/null +++ b/app/models/merge_requests/generated_ref_merge_request_commit.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module MergeRequests + class GeneratedRefMergeRequestCommit < ApplicationRecord + include ShaAttribute + + sha_attribute :commit_sha + + belongs_to :merge_request + belongs_to :project + validates :commit_sha, presence: true, uniqueness: true + end +end diff --git a/db/docs/generated_ref_merge_request_commits.yml b/db/docs/generated_ref_merge_request_commits.yml index 21338971280c47..3f9538c11b9d55 100644 --- a/db/docs/generated_ref_merge_request_commits.yml +++ b/db/docs/generated_ref_merge_request_commits.yml @@ -3,7 +3,7 @@ table_name: generated_ref_merge_request_commits classes: - GeneratedRefMergeRequestCommit feature_categories: -- merge_trains +- code_review_workflow description: Stores the new commit shas for a merged merge request introduced_by_url: milestone: '18.2' diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 8c3dc359b649b9..3f7c92184fc8c2 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -205,7 +205,9 @@ def train def cleanup_ref(async: true) # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. - GeneratedRefMergeRequestCommit.where(merge_request: merge_request).delete_all unless merge_request.merged? + unless merge_request.merged? + MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request: merge_request).delete_all + end if async merge_request.schedule_cleanup_refs(only: :train) diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 151d43421c8a40..c32d12e6cb9a78 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -519,13 +519,13 @@ describe '#try_cleanup_ref' do let(:train_car) { create(:merge_train_car) } - let(:generated_ref_merge_request_commits) { class_double(GeneratedRefMergeRequestCommit) } + let(:generated_ref_merge_request_commits) { class_double(MergeRequests::GeneratedRefMergeRequestCommit) } context 'when a merge request is merged' do subject { train_car.try_cleanup_ref } before do - stub_const('GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) + stub_const('MergeRequests::GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) allow(generated_ref_merge_request_commits).to receive(:where) .and_return(generated_ref_merge_request_commits) allow(generated_ref_merge_request_commits).to receive(:delete_all) @@ -542,7 +542,7 @@ subject { train_car.try_cleanup_ref } before do - stub_const('GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) + stub_const('MergeRequests::GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) allow(generated_ref_merge_request_commits).to receive(:where) .and_return(generated_ref_merge_request_commits) allow(generated_ref_merge_request_commits).to receive(:delete_all) diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index 9593f2a185dee9..4e80babd8d2cd4 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -133,8 +133,9 @@ it 'created generated ref merge request commits' do result - expect(GeneratedRefMergeRequestCommit.where.not(merge_request_id: merge_request.id)).to be_empty - expect(GeneratedRefMergeRequestCommit.where(merge_request_id: merge_request.id).count).to eq( + expect(MergeRequests::GeneratedRefMergeRequestCommit.where.not(merge_request_id: merge_request.id)) + .to be_empty + expect(MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request_id: merge_request.id).count).to eq( commit_count ) end @@ -144,7 +145,7 @@ it 'created generated ref merge request commits' do result - expect(GeneratedRefMergeRequestCommit.exists?).to be false + expect(MergeRequests::GeneratedRefMergeRequestCommit.exists?).to be false end end diff --git a/spec/factories/generated_ref_merge_request_commits.rb b/spec/factories/generated_ref_merge_request_commits.rb index 7b549143ac0e46..75677182c72236 100644 --- a/spec/factories/generated_ref_merge_request_commits.rb +++ b/spec/factories/generated_ref_merge_request_commits.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :generated_ref_merge_request_commit, class: 'GeneratedRefMergeRequestCommit' do + factory :generated_ref_merge_request_commit, class: 'MergeRequests::GeneratedRefMergeRequestCommit' do commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } # rubocop:disable Fips/SHA1 -- test data association :project association :merge_request diff --git a/spec/models/generated_ref_merge_request_commit_spec.rb b/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb similarity index 91% rename from spec/models/generated_ref_merge_request_commit_spec.rb rename to spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb index 40e7d41d7eca50..87cce0e59e2114 100644 --- a/spec/models/generated_ref_merge_request_commit_spec.rb +++ b/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GeneratedRefMergeRequestCommit do # rubocop:disable RSpec/FeatureCategory -- to be determined during review +RSpec.describe MergeRequests::GeneratedRefMergeRequestCommit, feature_category: :code_review_workflow do let_it_be(:project) { create(:project) } let_it_be(:merge_request) { create(:merge_request, source_project: project) } -- GitLab From 88c22c44d2c3282aca7c2cf5f793bf24e903bd59 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 10:40:20 +0200 Subject: [PATCH 22/74] Tie deletion of ref merge request commits to train status --- ee/app/models/merge_trains/car.rb | 4 +--- ee/spec/models/merge_trains/car_spec.rb | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 3f7c92184fc8c2..2c15da5f5cd353 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -205,9 +205,7 @@ def train def cleanup_ref(async: true) # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. - unless merge_request.merged? - MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request: merge_request).delete_all - end + MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request: merge_request).delete_all unless merged? if async merge_request.schedule_cleanup_refs(only: :train) diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index c32d12e6cb9a78..4a85e0967b47bb 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -532,7 +532,9 @@ end it 'does not delete created ref merge request commits' do - train_car.merge_request.mark_as_merged + train_car.refresh_pipeline! + train_car.start_merge! + train_car.finish_merge! expect(generated_ref_merge_request_commits).not_to receive(:delete_all) subject end -- GitLab From b7f9bafc12701c2b14b2719942dc3047867cf0ed Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 13:33:22 +0200 Subject: [PATCH 23/74] Refactor usage of MergeRequest by commit sha --- app/finders/merge_requests_finder.rb | 9 +++++---- app/models/merge_request.rb | 10 +--------- .../generated_ref_merge_request_commit.rb | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index aa7905cf1388ca..6acf51826d2e95 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -135,10 +135,11 @@ def by_generated_ref_merge_request_commit(items) return items unless items.empty? - MergeRequest.by_project_and_commit_sha_via_generated_ref_merge_request_commit( - params[:project_id], - params[:commit_sha] - ) + ::MergeRequests::GeneratedRefMergeRequestCommit + .merge_request_for_sha( + params[:project_id], + params[:commit_sha] + ) end def by_author(items) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index da2fb0d1671b9a..059397f67abcb3 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -72,7 +72,7 @@ class MergeRequest < ApplicationRecord -> { regular }, inverse_of: :merge_request has_many :merge_request_context_commits, inverse_of: :merge_request has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files - + has_many :generated_ref_merge_request_commits, class_name: 'MergeRequests::GeneratedRefMergeRequestCommit' has_one :merge_request_diff, -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request has_one :merge_head_diff, @@ -330,14 +330,6 @@ def public_merge_status where("source_branch = :branch OR target_branch = :branch", branch: branch_name) end - scope :by_project_and_commit_sha_via_generated_ref_merge_request_commit, ->(project_id, sha) { - joins("INNER JOIN generated_ref_merge_request_commits ON generated_ref_merge_request_commits.merge_request_id = merge_requests.id") - .where(merge_requests: { project_id: project_id }) - .where(generated_ref_merge_request_commits: { - commit_sha: Gitlab::Database::ShaAttribute.new.serialize(sha) - }) - } - scope :by_milestone, ->(milestone) { where(milestone_id: milestone) } scope :of_projects, ->(ids) { where(target_project_id: ids) } scope :from_project, ->(project) { where(source_project_id: project) } diff --git a/app/models/merge_requests/generated_ref_merge_request_commit.rb b/app/models/merge_requests/generated_ref_merge_request_commit.rb index 6d8a344c451f91..3a814faefd2110 100644 --- a/app/models/merge_requests/generated_ref_merge_request_commit.rb +++ b/app/models/merge_requests/generated_ref_merge_request_commit.rb @@ -9,5 +9,20 @@ class GeneratedRefMergeRequestCommit < ApplicationRecord belongs_to :merge_request belongs_to :project validates :commit_sha, presence: true, uniqueness: true + + # Since the MergeRequestsFinder expects us to provide a relation instead of a single object, + # we are returning here a relation, but limited to 1 since we only care about the merge request + # with the given commit shat and project id, which will be unique. + def self.merge_request_for_sha(project_id, sha) + MergeRequest + .joins(:generated_ref_merge_request_commits) + .where( + merge_requests: { project_id: project_id }, + generated_ref_merge_request_commits: { + commit_sha: sha + } + ) + .limit(1) + end end end -- GitLab From 2527fede7c6597f672fbf942e04defaf6d5f18e0 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 14:02:11 +0200 Subject: [PATCH 24/74] Scope ref commit generation to merge trains --- app/services/merge_requests/create_ref_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 8d68c98ccc1d11..e2633870e4ea5d 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -40,7 +40,7 @@ def execute result = maybe_merge!(**result) # Store commits in generated_ref_merge_request_commits table using the final commit_sha - store_generated_ref_merge_request_commits(result[:commit_sha]) + store_generated_ref_merge_request_commits(result[:commit_sha]) if merge_request.merge_train_car.present? # Add commits data to result for external use result[:commits_data] = collect_commits_data(result[:commit_sha]) -- GitLab From c3ba9e2883fdf9b19de397c9584cb085b1aeaf2e Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 14:04:22 +0200 Subject: [PATCH 25/74] Add new model to all models definition --- spec/lib/gitlab/import_export/all_models.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 55f815da231d9c..14aeefcdf91d0f 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -288,6 +288,7 @@ merge_requests: - failed_scan_result_policy_violations - approval_metrics - approval_policy_merge_request_bypass_events +- generated_ref_merge_request_commits external_pull_requests: - project merge_request_diff: -- GitLab From f9f5cff327b2e2c5aeaad220079d230c21c63fc1 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 14:26:43 +0200 Subject: [PATCH 26/74] Update db docs --- db/docs/generated_ref_merge_request_commits.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/docs/generated_ref_merge_request_commits.yml b/db/docs/generated_ref_merge_request_commits.yml index 3f9538c11b9d55..984c208e3ca23c 100644 --- a/db/docs/generated_ref_merge_request_commits.yml +++ b/db/docs/generated_ref_merge_request_commits.yml @@ -2,10 +2,11 @@ table_name: generated_ref_merge_request_commits classes: - GeneratedRefMergeRequestCommit +- MergeRequests::GeneratedRefMergeRequestCommit feature_categories: - code_review_workflow description: Stores the new commit shas for a merged merge request -introduced_by_url: +introduced_by_url: milestone: '18.2' gitlab_schema: gitlab_main_cell sharding_key: -- GitLab From f0ca0868c6a269f31c435204b95d111f88d6235d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 14:45:40 +0200 Subject: [PATCH 27/74] Fix spec for merge train car --- .../merge_trains/create_ref_service_spec.rb | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index 4e80babd8d2cd4..ad89479b150fd2 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -130,14 +130,18 @@ end shared_examples 'generate ref merge request commits' do |commit_count| - it 'created generated ref merge request commits' do - result - - expect(MergeRequests::GeneratedRefMergeRequestCommit.where.not(merge_request_id: merge_request.id)) - .to be_empty - expect(MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request_id: merge_request.id).count).to eq( - commit_count - ) + context 'when coming from a merge train' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + it 'created generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefMergeRequestCommit.where.not(merge_request_id: merge_request.id)) + .to be_empty + expect(MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request_id: merge_request.id).count).to eq( + commit_count + ) + end end end -- GitLab From 405e930b1e9d4099fd3c0f48eaaaf153a0d908e3 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 16:17:21 +0200 Subject: [PATCH 28/74] Add composite index --- .../generated_ref_merge_request_commit.rb | 1 + ...mposite_index_project_id_and_commit_sha.rb | 20 +++++++++++++++++++ db/schema_migrations/20250702141038 | 1 + db/structure.sql | 2 ++ 4 files changed, 24 insertions(+) create mode 100644 db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb create mode 100644 db/schema_migrations/20250702141038 diff --git a/app/models/merge_requests/generated_ref_merge_request_commit.rb b/app/models/merge_requests/generated_ref_merge_request_commit.rb index 3a814faefd2110..9b0dd035e5f3af 100644 --- a/app/models/merge_requests/generated_ref_merge_request_commit.rb +++ b/app/models/merge_requests/generated_ref_merge_request_commit.rb @@ -19,6 +19,7 @@ def self.merge_request_for_sha(project_id, sha) .where( merge_requests: { project_id: project_id }, generated_ref_merge_request_commits: { + project_id: project_id, commit_sha: sha } ) diff --git a/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb b/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb new file mode 100644 index 00000000000000..325f799d115b99 --- /dev/null +++ b/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# See https://docs.gitlab.com/ee/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddCompositeIndexProjectIdAndCommitSha < Gitlab::Database::Migration[2.3] + milestone '18.2' + disable_ddl_transaction! + + INDEX_NAME = 'index_generated_ref_mr_commits_on_project_id_and_commit_sha' + + def up + add_concurrent_index :generated_ref_merge_request_commits, [:project_id, :commit_sha], name: INDEX_NAME, + using: :btree + end + + def down + remove_concurrent_index_by_name :generated_ref_merge_request_commits, INDEX_NAME + end +end diff --git a/db/schema_migrations/20250702141038 b/db/schema_migrations/20250702141038 new file mode 100644 index 00000000000000..1bc484479ca0fb --- /dev/null +++ b/db/schema_migrations/20250702141038 @@ -0,0 +1 @@ +e31da45a2e6e83524a79a058c8d7b9bfe4e8166147b129e17e6d8b42d05faef9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 39c9f7ddf53f99..4d0dd4559075ff 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -36288,6 +36288,8 @@ CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON ge CREATE INDEX index_generated_ref_mr_commits_on_project_id ON generated_ref_merge_request_commits USING btree (project_id); +CREATE INDEX index_generated_ref_mr_commits_on_project_id_and_commit_sha ON generated_ref_merge_request_commits USING btree (project_id, commit_sha); + CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); CREATE INDEX index_geo_event_log_on_geo_event_id ON geo_event_log USING btree (geo_event_id) WHERE (geo_event_id IS NOT NULL); -- GitLab From 0d592cd4ccaf71f5d005625c076dda15f3b91d13 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 17:35:09 +0200 Subject: [PATCH 29/74] Remove uniquenuess constraint; remove duplicate index --- .../generated_ref_merge_request_commit.rb | 2 +- .../merge_requests/create_ref_service.rb | 5 +---- ...27103145_add_unique_index_on_commit_sha.rb | 22 ------------------- ..._to_generated_ref_merge_request_commits.rb | 5 ----- db/schema_migrations/20250627103145 | 1 - db/structure.sql | 4 ---- ...generated_ref_merge_request_commit_spec.rb | 13 ----------- 7 files changed, 2 insertions(+), 50 deletions(-) delete mode 100644 db/migrate/20250627103145_add_unique_index_on_commit_sha.rb delete mode 100644 db/schema_migrations/20250627103145 diff --git a/app/models/merge_requests/generated_ref_merge_request_commit.rb b/app/models/merge_requests/generated_ref_merge_request_commit.rb index 9b0dd035e5f3af..d15481efcbd468 100644 --- a/app/models/merge_requests/generated_ref_merge_request_commit.rb +++ b/app/models/merge_requests/generated_ref_merge_request_commit.rb @@ -8,7 +8,7 @@ class GeneratedRefMergeRequestCommit < ApplicationRecord belongs_to :merge_request belongs_to :project - validates :commit_sha, presence: true, uniqueness: true + validates :commit_sha, presence: true # Since the MergeRequestsFinder expects us to provide a relation instead of a single object, # we are returning here a relation, but limited to 1 since we only care about the merge request diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index e2633870e4ea5d..c8bb01ecca2b82 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -74,10 +74,7 @@ def store_generated_ref_merge_request_commits(final_commit_sha) end # Use upsert to handle potential duplicates - GeneratedRefMergeRequestCommit.upsert_all( - records, - unique_by: :commit_sha - ) + GeneratedRefMergeRequestCommit.upsert_all(records) rescue StandardError => e ServiceResponse.error(message: "Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) diff --git a/db/migrate/20250627103145_add_unique_index_on_commit_sha.rb b/db/migrate/20250627103145_add_unique_index_on_commit_sha.rb deleted file mode 100644 index 47778fbe653145..00000000000000 --- a/db/migrate/20250627103145_add_unique_index_on_commit_sha.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddUniqueIndexOnCommitSha < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - INDEX_NAME = :index_grmrc_on_commit_sha - - def up - add_concurrent_index :generated_ref_merge_request_commits, - :commit_sha, - unique: true, - name: INDEX_NAME - end - - def down - remove_concurrent_index_by_name :generated_ref_merge_request_commits, INDEX_NAME - end -end diff --git a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb b/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb index d4b661e11f7a33..44cf0ad9e1d742 100644 --- a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb +++ b/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb @@ -14,7 +14,6 @@ def up # rubocop:disable Rails/NotNullColumn -- empty table add_column :generated_ref_merge_request_commits, :project_id, :bigint, null: false # rubocop:enable Rails/NotNullColumn -- empty table - add_concurrent_index :generated_ref_merge_request_commits, :project_id, name: INDEX_NAME add_concurrent_foreign_key :generated_ref_merge_request_commits, :projects, @@ -28,10 +27,6 @@ def down column: :project_id end - remove_concurrent_index :generated_ref_merge_request_commits, - :project_id, - name: INDEX_NAME - remove_column :generated_ref_merge_request_commits, :project_id end end diff --git a/db/schema_migrations/20250627103145 b/db/schema_migrations/20250627103145 deleted file mode 100644 index d026134fd938f0..00000000000000 --- a/db/schema_migrations/20250627103145 +++ /dev/null @@ -1 +0,0 @@ -297abd260ff7f8efb8f8e85bdc7353eb7cb55c845705b5e9d9c9765fb781fdc1 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 4d0dd4559075ff..c05c25226ab3a1 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -36286,8 +36286,6 @@ CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USIN CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON generated_ref_merge_request_commits USING btree (merge_request_id); -CREATE INDEX index_generated_ref_mr_commits_on_project_id ON generated_ref_merge_request_commits USING btree (project_id); - CREATE INDEX index_generated_ref_mr_commits_on_project_id_and_commit_sha ON generated_ref_merge_request_commits USING btree (project_id, commit_sha); CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); @@ -36354,8 +36352,6 @@ CREATE INDEX index_grafana_integrations_on_enabled ON grafana_integrations USING CREATE INDEX index_grafana_integrations_on_project_id ON grafana_integrations USING btree (project_id); -CREATE UNIQUE INDEX index_grmrc_on_commit_sha ON generated_ref_merge_request_commits USING btree (commit_sha); - CREATE INDEX index_group_crm_settings_on_group_id ON group_crm_settings USING btree (group_id); CREATE INDEX index_group_crm_settings_on_source_group_id ON group_crm_settings USING btree (source_group_id); diff --git a/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb b/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb index 87cce0e59e2114..c04ca6ac327518 100644 --- a/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb +++ b/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb @@ -17,9 +17,6 @@ describe 'validations' do it { is_expected.to validate_presence_of(:commit_sha) } - # Since git gives us a lowercase hash, we should never encouter the case, where a commit sha - # with upper case letters will be saved. We only "fix" the shoulda matcher here. - it { is_expected.to validate_uniqueness_of(:commit_sha).case_insensitive } end describe 'factory behavior' do @@ -31,15 +28,5 @@ ref_commit.commit_sha = nil expect(ref_commit).not_to be_valid end - - it 'is not valid with duplicate commit_sha' do - described_class.create!( - commit_sha: ref_commit.commit_sha, - merge_request: merge_request, - project: project - ) - - expect(ref_commit).not_to be_valid - end end end -- GitLab From 02f19e170a236c7459bf20024bc6924bb4fc72b7 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 23:06:42 +0200 Subject: [PATCH 30/74] Remove unused index --- ...5139_add_project_id_to_generated_ref_merge_request_commits.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb b/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb index 44cf0ad9e1d742..8ad606ff5817b6 100644 --- a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb +++ b/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb @@ -7,7 +7,6 @@ class AddProjectIdToGeneratedRefMergeRequestCommits < Gitlab::Database::Migratio disable_ddl_transaction! milestone '18.2' - INDEX_NAME = 'index_generated_ref_mr_commits_on_project_id' FK_NAME = 'fk_generated_ref_mr_commits_project_id' def up -- GitLab From 04b2f590f83bf370af70cfd0917984a3d3dfe13d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 23:35:33 +0200 Subject: [PATCH 31/74] Rename table appropriately --- app/finders/merge_requests_finder.rb | 6 ++--- app/models/merge_request.rb | 2 +- ...uest_commit.rb => generated_ref_commit.rb} | 6 ++--- .../merge_requests/create_ref_service.rb | 8 +++---- ..._commits.yml => generated_ref_commits.yml} | 6 ++--- ...0250626095521_add_generated_ref_commit.rb} | 4 ++-- ...t_foreign_key_to_generated_ref_commits.rb} | 6 ++--- ...ge_request_id_to_generated_ref_commits.rb} | 10 ++++----- ...dd_project_id_to_generated_ref_commits.rb} | 13 ++++++----- ...mposite_index_project_id_and_commit_sha.rb | 6 ++--- db/structure.sql | 22 +++++++++---------- ee/app/models/merge_trains/car.rb | 2 +- ee/spec/models/merge_trains/car_spec.rb | 22 +++++++++---------- .../merge_trains/create_ref_service_spec.rb | 6 ++--- ...st_commits.rb => generated_ref_commits.rb} | 2 +- spec/lib/gitlab/import_export/all_models.yml | 2 +- ...t_spec.rb => generated_ref_commit_spec.rb} | 4 ++-- 17 files changed, 64 insertions(+), 63 deletions(-) rename app/models/merge_requests/{generated_ref_merge_request_commit.rb => generated_ref_commit.rb} (82%) rename db/docs/{generated_ref_merge_request_commits.yml => generated_ref_commits.yml} (65%) rename db/migrate/{20250626095521_add_generated_ref_merge_request_commit.rb => 20250626095521_add_generated_ref_commit.rb} (81%) rename db/migrate/{20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb => 20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb} (54%) rename db/migrate/{20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb => 20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb} (53%) rename db/migrate/{20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb => 20250627165139_add_project_id_to_generated_ref_commits.rb} (52%) rename spec/factories/{generated_ref_merge_request_commits.rb => generated_ref_commits.rb} (67%) rename spec/models/merge_requests/{generated_ref_merge_request_commit_spec.rb => generated_ref_commit_spec.rb} (77%) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 6acf51826d2e95..ef4046c324624d 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -95,7 +95,7 @@ def filter_items(_items) items = by_blob_path(items) items = by_no_review_requested_or_only_user(items) items = by_review_states_or_no_reviewer(items) - items = by_generated_ref_merge_request_commit(items) + items = by_generated_ref_commit(items) by_valid_or_no_reviewers(items) end @@ -130,12 +130,12 @@ def sort(items) # Lookup of Refs generated via `MergeRequests::MergeToRefService` is not currently supported # See: https://gitlab.com/gitlab-org/gitlab/-/issues/421025 - def by_generated_ref_merge_request_commit(items) + def by_generated_ref_commit(items) return items unless params[:project_id].present? && params[:commit_sha].present? return items unless items.empty? - ::MergeRequests::GeneratedRefMergeRequestCommit + ::MergeRequests::GeneratedRefCommit .merge_request_for_sha( params[:project_id], params[:commit_sha] diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 059397f67abcb3..d848540bb59773 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -72,7 +72,7 @@ class MergeRequest < ApplicationRecord -> { regular }, inverse_of: :merge_request has_many :merge_request_context_commits, inverse_of: :merge_request has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files - has_many :generated_ref_merge_request_commits, class_name: 'MergeRequests::GeneratedRefMergeRequestCommit' + has_many :generated_ref_commits, class_name: 'MergeRequests::GeneratedRefCommit' has_one :merge_request_diff, -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request has_one :merge_head_diff, diff --git a/app/models/merge_requests/generated_ref_merge_request_commit.rb b/app/models/merge_requests/generated_ref_commit.rb similarity index 82% rename from app/models/merge_requests/generated_ref_merge_request_commit.rb rename to app/models/merge_requests/generated_ref_commit.rb index d15481efcbd468..a0d480bec95751 100644 --- a/app/models/merge_requests/generated_ref_merge_request_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module MergeRequests - class GeneratedRefMergeRequestCommit < ApplicationRecord + class GeneratedRefCommit < ApplicationRecord include ShaAttribute sha_attribute :commit_sha @@ -15,10 +15,10 @@ class GeneratedRefMergeRequestCommit < ApplicationRecord # with the given commit shat and project id, which will be unique. def self.merge_request_for_sha(project_id, sha) MergeRequest - .joins(:generated_ref_merge_request_commits) + .joins(:generated_ref_commits) .where( merge_requests: { project_id: project_id }, - generated_ref_merge_request_commits: { + generated_ref_commits: { project_id: project_id, commit_sha: sha } diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index c8bb01ecca2b82..64dbd434745ac5 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,8 +39,8 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) - # Store commits in generated_ref_merge_request_commits table using the final commit_sha - store_generated_ref_merge_request_commits(result[:commit_sha]) if merge_request.merge_train_car.present? + # Store commits in generated_ref_commits table using the final commit_sha + store_generated_ref_commits(result[:commit_sha]) if merge_request.merge_train_car.present? # Add commits data to result for external use result[:commits_data] = collect_commits_data(result[:commit_sha]) @@ -58,7 +58,7 @@ def execute delegate :target_project, to: :merge_request delegate :repository, to: :target_project - def store_generated_ref_merge_request_commits(final_commit_sha) + def store_generated_ref_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha) return unless commit_shas.any? && target_project.generate_ref_commits? @@ -74,7 +74,7 @@ def store_generated_ref_merge_request_commits(final_commit_sha) end # Use upsert to handle potential duplicates - GeneratedRefMergeRequestCommit.upsert_all(records) + GeneratedRefCommit.upsert_all(records) rescue StandardError => e ServiceResponse.error(message: "Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) diff --git a/db/docs/generated_ref_merge_request_commits.yml b/db/docs/generated_ref_commits.yml similarity index 65% rename from db/docs/generated_ref_merge_request_commits.yml rename to db/docs/generated_ref_commits.yml index 984c208e3ca23c..134ea6ab67796a 100644 --- a/db/docs/generated_ref_merge_request_commits.yml +++ b/db/docs/generated_ref_commits.yml @@ -1,8 +1,8 @@ --- -table_name: generated_ref_merge_request_commits +table_name: generated_ref_commits classes: -- GeneratedRefMergeRequestCommit -- MergeRequests::GeneratedRefMergeRequestCommit +- GeneratedRefCommit +- MergeRequests::GeneratedRefCommit feature_categories: - code_review_workflow description: Stores the new commit shas for a merged merge request diff --git a/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb similarity index 81% rename from db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb rename to db/migrate/20250626095521_add_generated_ref_commit.rb index 540eb5e3b6f635..7f10e98a8de912 100644 --- a/db/migrate/20250626095521_add_generated_ref_merge_request_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -3,13 +3,13 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddGeneratedRefMergeRequestCommit < Gitlab::Database::Migration[2.3] +class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] milestone '18.2' disable_ddl_transaction! def change # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found - create_table :generated_ref_merge_request_commits do |t| + create_table :generated_ref_commits do |t| t.binary :commit_sha, limit: 20 t.bigint :merge_request_id, null: false t.timestamps_with_timezone diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb similarity index 54% rename from db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb rename to db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index e561c7029b330c..2367540cefb819 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_merge_request_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -3,19 +3,19 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddMergeRequestForeignKeyToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] +class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' def up - add_concurrent_foreign_key :generated_ref_merge_request_commits, :merge_requests, + add_concurrent_foreign_key :generated_ref_commits, :merge_requests, column: :merge_request_id, on_delete: :cascade end def down with_lock_retries do - remove_foreign_key_if_exists(:generated_ref_merge_request_commits, column: :merge_request_id) + remove_foreign_key_if_exists(:generated_ref_commits, column: :merge_request_id) end end end diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb similarity index 53% rename from db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb rename to db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb index 36e0fb9c94e340..3e24ab8648dcc5 100644 --- a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_merge_request_commits.rb +++ b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb @@ -3,7 +3,7 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddIndexForMergeRequestIdToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] +class AddIndexForMergeRequestIdToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' @@ -11,16 +11,16 @@ class AddIndexForMergeRequestIdToGeneratedRefMergeRequestCommits < Gitlab::Datab def up add_concurrent_index( - :generated_ref_merge_request_commits, + :generated_ref_commits, :merge_request_id, - name: 'index_generated_ref_merge_request_commits_on_merge_request_id' + name: 'index_generated_ref_commits_on_merge_request_id' ) end def down remove_concurrent_index_by_name( - :generated_ref_merge_request_commits, - 'index_generated_ref_merge_request_commits_on_merge_request_id' + :generated_ref_commits, + 'index_generated_ref_commits_on_merge_request_id' ) end end diff --git a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb b/db/migrate/20250627165139_add_project_id_to_generated_ref_commits.rb similarity index 52% rename from db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb rename to db/migrate/20250627165139_add_project_id_to_generated_ref_commits.rb index 8ad606ff5817b6..2d6dd70031a124 100644 --- a/db/migrate/20250627165139_add_project_id_to_generated_ref_merge_request_commits.rb +++ b/db/migrate/20250627165139_add_project_id_to_generated_ref_commits.rb @@ -3,18 +3,19 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddProjectIdToGeneratedRefMergeRequestCommits < Gitlab::Database::Migration[2.3] +class AddProjectIdToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' - FK_NAME = 'fk_generated_ref_mr_commits_project_id' + INDEX_NAME = 'index_generated_ref_commits_on_project_id' + FK_NAME = 'fk_generated_ref_commits_project_id' def up # rubocop:disable Rails/NotNullColumn -- empty table - add_column :generated_ref_merge_request_commits, :project_id, :bigint, null: false + add_column :generated_ref_commits, :project_id, :bigint, null: false # rubocop:enable Rails/NotNullColumn -- empty table - add_concurrent_foreign_key :generated_ref_merge_request_commits, + add_concurrent_foreign_key :generated_ref_commits, :projects, column: :project_id, name: FK_NAME @@ -22,10 +23,10 @@ def up def down with_lock_retries do - remove_foreign_key_if_exists :generated_ref_merge_request_commits, + remove_foreign_key_if_exists :generated_ref_commits, column: :project_id end - remove_column :generated_ref_merge_request_commits, :project_id + remove_column :generated_ref_commits, :project_id end end diff --git a/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb b/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb index 325f799d115b99..566d4dfe1cccfd 100644 --- a/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb +++ b/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb @@ -7,14 +7,14 @@ class AddCompositeIndexProjectIdAndCommitSha < Gitlab::Database::Migration[2.3] milestone '18.2' disable_ddl_transaction! - INDEX_NAME = 'index_generated_ref_mr_commits_on_project_id_and_commit_sha' + INDEX_NAME = 'index_generated_ref_commits_on_project_id_and_commit_sha' def up - add_concurrent_index :generated_ref_merge_request_commits, [:project_id, :commit_sha], name: INDEX_NAME, + add_concurrent_index :generated_ref_commits, [:project_id, :commit_sha], name: INDEX_NAME, using: :btree end def down - remove_concurrent_index_by_name :generated_ref_merge_request_commits, INDEX_NAME + remove_concurrent_index_by_name :generated_ref_commits, INDEX_NAME end end diff --git a/db/structure.sql b/db/structure.sql index c05c25226ab3a1..11d084d23aaf73 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15133,7 +15133,7 @@ CREATE SEQUENCE fork_networks_id_seq ALTER SEQUENCE fork_networks_id_seq OWNED BY fork_networks.id; -CREATE TABLE generated_ref_merge_request_commits ( +CREATE TABLE generated_ref_commits ( id bigint NOT NULL, commit_sha bytea, merge_request_id bigint NOT NULL, @@ -15142,14 +15142,14 @@ CREATE TABLE generated_ref_merge_request_commits ( project_id bigint NOT NULL ); -CREATE SEQUENCE generated_ref_merge_request_commits_id_seq +CREATE SEQUENCE generated_ref_commits_id_seq START WITH 1 INCREMENT BY 1 NO MINVALUE NO MAXVALUE CACHE 1; -ALTER SEQUENCE generated_ref_merge_request_commits_id_seq OWNED BY generated_ref_merge_request_commits.id; +ALTER SEQUENCE generated_ref_commits_id_seq OWNED BY generated_ref_commits.id; CREATE TABLE geo_cache_invalidation_events ( id bigint NOT NULL, @@ -28270,7 +28270,7 @@ ALTER TABLE ONLY fork_network_members ALTER COLUMN id SET DEFAULT nextval('fork_ ALTER TABLE ONLY fork_networks ALTER COLUMN id SET DEFAULT nextval('fork_networks_id_seq'::regclass); -ALTER TABLE ONLY generated_ref_merge_request_commits ALTER COLUMN id SET DEFAULT nextval('generated_ref_merge_request_commits_id_seq'::regclass); +ALTER TABLE ONLY generated_ref_commits ALTER COLUMN id SET DEFAULT nextval('generated_ref_commits_id_seq'::regclass); ALTER TABLE ONLY geo_cache_invalidation_events ALTER COLUMN id SET DEFAULT nextval('geo_cache_invalidation_events_id_seq'::regclass); @@ -30809,8 +30809,8 @@ ALTER TABLE ONLY fork_network_members ALTER TABLE ONLY fork_networks ADD CONSTRAINT fork_networks_pkey PRIMARY KEY (id); -ALTER TABLE ONLY generated_ref_merge_request_commits - ADD CONSTRAINT generated_ref_merge_request_commits_pkey PRIMARY KEY (id); +ALTER TABLE ONLY generated_ref_commits + ADD CONSTRAINT generated_ref_commits_pkey PRIMARY KEY (id); ALTER TABLE ONLY geo_cache_invalidation_events ADD CONSTRAINT geo_cache_invalidation_events_pkey PRIMARY KEY (id); @@ -36284,9 +36284,9 @@ CREATE INDEX index_fork_networks_on_organization_id ON fork_networks USING btree CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USING btree (root_project_id); -CREATE INDEX index_generated_ref_merge_request_commits_on_merge_request_id ON generated_ref_merge_request_commits USING btree (merge_request_id); +CREATE INDEX index_generated_ref_commits_on_merge_request_id ON generated_ref_commits USING btree (merge_request_id); -CREATE INDEX index_generated_ref_mr_commits_on_project_id_and_commit_sha ON generated_ref_merge_request_commits USING btree (project_id, commit_sha); +CREATE INDEX index_generated_ref_commits_on_project_id_and_commit_sha ON generated_ref_commits USING btree (project_id, commit_sha); CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); @@ -44712,7 +44712,7 @@ ALTER TABLE ONLY description_versions ALTER TABLE ONLY bulk_import_entities ADD CONSTRAINT fk_b69fa2b2df FOREIGN KEY (bulk_import_id) REFERENCES bulk_imports(id) ON DELETE CASCADE; -ALTER TABLE ONLY generated_ref_merge_request_commits +ALTER TABLE ONLY generated_ref_commits ADD CONSTRAINT fk_b6cc4921de FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; ALTER TABLE ONLY security_policy_requirements @@ -45384,8 +45384,8 @@ ALTER TABLE ONLY packages_conan_package_revisions ALTER TABLE ONLY issues ADD CONSTRAINT fk_ffed080f01 FOREIGN KEY (updated_by_id) REFERENCES users(id) ON DELETE SET NULL; -ALTER TABLE ONLY generated_ref_merge_request_commits - ADD CONSTRAINT fk_generated_ref_mr_commits_project_id FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; +ALTER TABLE ONLY generated_ref_commits + ADD CONSTRAINT fk_generated_ref_commits_project_id FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; ALTER TABLE ONLY geo_event_log ADD CONSTRAINT fk_geo_event_log_on_geo_event_id FOREIGN KEY (geo_event_id) REFERENCES geo_events(id) ON DELETE CASCADE; diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 2c15da5f5cd353..5dc8030c7927d4 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -205,7 +205,7 @@ def train def cleanup_ref(async: true) # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. - MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request: merge_request).delete_all unless merged? + MergeRequests::GeneratedRefCommit.where(merge_request: merge_request).delete_all unless merged? if async merge_request.schedule_cleanup_refs(only: :train) diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 4a85e0967b47bb..9ed07b53fa04f3 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -519,23 +519,23 @@ describe '#try_cleanup_ref' do let(:train_car) { create(:merge_train_car) } - let(:generated_ref_merge_request_commits) { class_double(MergeRequests::GeneratedRefMergeRequestCommit) } + let(:generated_ref_commits) { class_double(MergeRequests::GeneratedRefCommit) } context 'when a merge request is merged' do subject { train_car.try_cleanup_ref } before do - stub_const('MergeRequests::GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) - allow(generated_ref_merge_request_commits).to receive(:where) - .and_return(generated_ref_merge_request_commits) - allow(generated_ref_merge_request_commits).to receive(:delete_all) + stub_const('MergeRequests::GeneratedRefCommit', generated_ref_commits) + allow(generated_ref_commits).to receive(:where) + .and_return(generated_ref_commits) + allow(generated_ref_commits).to receive(:delete_all) end it 'does not delete created ref merge request commits' do train_car.refresh_pipeline! train_car.start_merge! train_car.finish_merge! - expect(generated_ref_merge_request_commits).not_to receive(:delete_all) + expect(generated_ref_commits).not_to receive(:delete_all) subject end end @@ -544,14 +544,14 @@ subject { train_car.try_cleanup_ref } before do - stub_const('MergeRequests::GeneratedRefMergeRequestCommit', generated_ref_merge_request_commits) - allow(generated_ref_merge_request_commits).to receive(:where) - .and_return(generated_ref_merge_request_commits) - allow(generated_ref_merge_request_commits).to receive(:delete_all) + stub_const('MergeRequests::GeneratedRefCommit', generated_ref_commits) + allow(generated_ref_commits).to receive(:where) + .and_return(generated_ref_commits) + allow(generated_ref_commits).to receive(:delete_all) end it 'does delete created ref merge request commits' do - expect(generated_ref_merge_request_commits).to receive(:delete_all) + expect(generated_ref_commits).to receive(:delete_all) subject end end diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index ad89479b150fd2..1813f06ba2c18f 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -136,9 +136,9 @@ it 'created generated ref merge request commits' do result - expect(MergeRequests::GeneratedRefMergeRequestCommit.where.not(merge_request_id: merge_request.id)) + expect(MergeRequests::GeneratedRefCommit.where.not(merge_request_id: merge_request.id)) .to be_empty - expect(MergeRequests::GeneratedRefMergeRequestCommit.where(merge_request_id: merge_request.id).count).to eq( + expect(MergeRequests::GeneratedRefCommit.where(merge_request_id: merge_request.id).count).to eq( commit_count ) end @@ -149,7 +149,7 @@ it 'created generated ref merge request commits' do result - expect(MergeRequests::GeneratedRefMergeRequestCommit.exists?).to be false + expect(MergeRequests::GeneratedRefCommit.exists?).to be false end end diff --git a/spec/factories/generated_ref_merge_request_commits.rb b/spec/factories/generated_ref_commits.rb similarity index 67% rename from spec/factories/generated_ref_merge_request_commits.rb rename to spec/factories/generated_ref_commits.rb index 75677182c72236..def491443be1e2 100644 --- a/spec/factories/generated_ref_merge_request_commits.rb +++ b/spec/factories/generated_ref_commits.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :generated_ref_merge_request_commit, class: 'MergeRequests::GeneratedRefMergeRequestCommit' do + factory :generated_ref_commit, class: 'MergeRequests::GeneratedRefCommit' do commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } # rubocop:disable Fips/SHA1 -- test data association :project association :merge_request diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 14aeefcdf91d0f..819fdb76dea407 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -288,7 +288,7 @@ merge_requests: - failed_scan_result_policy_violations - approval_metrics - approval_policy_merge_request_bypass_events -- generated_ref_merge_request_commits +- generated_ref_commits external_pull_requests: - project merge_request_diff: diff --git a/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb b/spec/models/merge_requests/generated_ref_commit_spec.rb similarity index 77% rename from spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb rename to spec/models/merge_requests/generated_ref_commit_spec.rb index c04ca6ac327518..cb76a51473305c 100644 --- a/spec/models/merge_requests/generated_ref_merge_request_commit_spec.rb +++ b/spec/models/merge_requests/generated_ref_commit_spec.rb @@ -2,12 +2,12 @@ require 'spec_helper' -RSpec.describe MergeRequests::GeneratedRefMergeRequestCommit, feature_category: :code_review_workflow do +RSpec.describe MergeRequests::GeneratedRefCommit, feature_category: :code_review_workflow do let_it_be(:project) { create(:project) } let_it_be(:merge_request) { create(:merge_request, source_project: project) } subject(:ref_commit) do - build(:generated_ref_merge_request_commit, project: project, merge_request: merge_request) + build(:generated_ref_commit, project: project, merge_request: merge_request) end describe 'associations' do -- GitLab From da6a00835a18295432c3e165e0d4d5036b371028 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 2 Jul 2025 23:49:13 +0200 Subject: [PATCH 32/74] Update db doc for generated ref commits --- db/docs/generated_ref_commits.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/db/docs/generated_ref_commits.yml b/db/docs/generated_ref_commits.yml index 134ea6ab67796a..10a6244fb2c2c8 100644 --- a/db/docs/generated_ref_commits.yml +++ b/db/docs/generated_ref_commits.yml @@ -1,11 +1,10 @@ --- table_name: generated_ref_commits classes: -- GeneratedRefCommit - MergeRequests::GeneratedRefCommit feature_categories: -- code_review_workflow -description: Stores the new commit shas for a merged merge request +- source_code_management +description: Tracks the relationship between commit SHAs and merge requests for gitlab generated references such as train references. introduced_by_url: milestone: '18.2' gitlab_schema: gitlab_main_cell -- GitLab From c137bddd4f180e940aa003f04b17840a1090d7e6 Mon Sep 17 00:00:00 2001 From: "Daniel P." Date: Thu, 3 Jul 2025 14:57:56 +0200 Subject: [PATCH 33/74] Apply 5 suggestion(s) to 4 file(s) Co-authored-by: GitLab Duo --- app/finders/merge_requests_finder.rb | 1 + app/models/merge_requests/generated_ref_commit.rb | 2 +- app/services/merge_requests/create_ref_service.rb | 6 +++--- ...d_index_for_merge_request_id_to_generated_ref_commits.rb | 1 - 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index ef4046c324624d..5970d329aa3bae 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -133,6 +133,7 @@ def sort(items) def by_generated_ref_commit(items) return items unless params[:project_id].present? && params[:commit_sha].present? + # Only perform generated ref commit lookup as fallback when no items found return items unless items.empty? ::MergeRequests::GeneratedRefCommit diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index a0d480bec95751..a28895571d1ff3 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -17,7 +17,7 @@ def self.merge_request_for_sha(project_id, sha) MergeRequest .joins(:generated_ref_commits) .where( - merge_requests: { project_id: project_id }, + merge_requests: { target_project_id: project_id }, generated_ref_commits: { project_id: project_id, commit_sha: sha diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 64dbd434745ac5..1cec29fb2e4f7a 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -76,9 +76,9 @@ def store_generated_ref_commits(final_commit_sha) # Use upsert to handle potential duplicates GeneratedRefCommit.upsert_all(records) rescue StandardError => e - ServiceResponse.error(message: "Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) - # Don't raise error to avoid breaking the main flow + # Log error but don't break the main flow + Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") end def collect_commits_data(final_commit_sha) @@ -96,8 +96,8 @@ def collect_commits_data(final_commit_sha) } end rescue StandardError => e - ServiceResponse.error(message: "Failed to collect commits data for MR #{merge_request.id}: #{e.message}") Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) + Gitlab::AppLogger.error("Failed to collect commits data for MR #{merge_request.id}: #{e.message}") [] end diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb index 3e24ab8648dcc5..fea5928cc2299b 100644 --- a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb +++ b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb @@ -7,7 +7,6 @@ class AddIndexForMergeRequestIdToGeneratedRefCommits < Gitlab::Database::Migrati disable_ddl_transaction! milestone '18.2' - disable_ddl_transaction! def up add_concurrent_index( -- GitLab From 3696eede152c515944059fdfc043a114125afff6 Mon Sep 17 00:00:00 2001 From: "Daniel P." Date: Thu, 3 Jul 2025 15:10:30 +0200 Subject: [PATCH 34/74] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: GitLab Duo --- app/models/merge_requests/generated_ref_commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index a28895571d1ff3..bddd9c43bda48b 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -12,7 +12,7 @@ class GeneratedRefCommit < ApplicationRecord # Since the MergeRequestsFinder expects us to provide a relation instead of a single object, # we are returning here a relation, but limited to 1 since we only care about the merge request - # with the given commit shat and project id, which will be unique. + # with the given commit sha and project id, which will be unique. def self.merge_request_for_sha(project_id, sha) MergeRequest .joins(:generated_ref_commits) -- GitLab From 38fa512ef9173c0a0476cb8995477d3ab2e9b5a9 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 3 Jul 2025 15:13:49 +0200 Subject: [PATCH 35/74] Remove empty line added by GitLabDuo --- ...04_add_index_for_merge_request_id_to_generated_ref_commits.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb index fea5928cc2299b..70daeda4d58c39 100644 --- a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb +++ b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb @@ -7,7 +7,6 @@ class AddIndexForMergeRequestIdToGeneratedRefCommits < Gitlab::Database::Migrati disable_ddl_transaction! milestone '18.2' - def up add_concurrent_index( :generated_ref_commits, -- GitLab From c7aae41efc54172b6e82ed5b6b670df674f2a34d Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 3 Jul 2025 18:16:58 +0200 Subject: [PATCH 36/74] Refactor generated ref commit migrations and presence of attributes --- .../merge_requests/generated_ref_commit.rb | 2 +- ...20250626095521_add_generated_ref_commit.rb | 24 +++++++++++++++++-- ...rge_request_id_to_generated_ref_commits.rb | 24 ------------------- ...s_foreign_key_to_generated_ref_commits.rb} | 9 +------ ...mposite_index_project_id_and_commit_sha.rb | 20 ---------------- db/schema_migrations/20250626165904 | 1 - db/schema_migrations/20250702141038 | 1 - db/structure.sql | 2 +- .../generated_ref_commit_spec.rb | 10 ++++++++ 9 files changed, 35 insertions(+), 58 deletions(-) delete mode 100644 db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb rename db/migrate/{20250627165139_add_project_id_to_generated_ref_commits.rb => 20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb} (58%) delete mode 100644 db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb delete mode 100644 db/schema_migrations/20250626165904 delete mode 100644 db/schema_migrations/20250702141038 diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index bddd9c43bda48b..9e83c8837f5c63 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -8,7 +8,7 @@ class GeneratedRefCommit < ApplicationRecord belongs_to :merge_request belongs_to :project - validates :commit_sha, presence: true + validates :commit_sha, :project, :merge_request, presence: true # Since the MergeRequestsFinder expects us to provide a relation instead of a single object, # we are returning here a relation, but limited to 1 since we only care about the merge request diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index 7f10e98a8de912..f2987b1d1c8d7b 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -7,13 +7,33 @@ class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] milestone '18.2' disable_ddl_transaction! - def change + INDEX_NAME_MERGE_REQUEST = 'index_generated_ref_commits_on_merge_request_id' + INDEX_NAME_COMMITS_AND_PROJECT_ID = 'index_generated_ref_commits_on_project_id_and_commit_sha' + + def up # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found create_table :generated_ref_commits do |t| - t.binary :commit_sha, limit: 20 + t.binary :commit_sha, null: false, limit: 20 t.bigint :merge_request_id, null: false + t.bigint :project_id, :bigint, null: false t.timestamps_with_timezone end # rubocop:enable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found + add_concurrent_index( + :generated_ref_commits, + :merge_request_id, + name: INDEX_NAME_MERGE_REQUEST + ) + add_concurrent_index :generated_ref_commits, [:project_id, :commit_sha], name: INDEX_NAME_COMMITS_AND_PROJECT_ID, + using: :btree + end + + def down + remove_concurrent_index_by_name( + :generated_ref_commits, + INDEX_NAME_MERGE_REQUEST + ) + remove_concurrent_index_by_name :generated_ref_commits, INDEX_NAME_COMMITS_AND_PROJECT_ID + drop_table :generated_ref_commits end end diff --git a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb b/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb deleted file mode 100644 index 70daeda4d58c39..00000000000000 --- a/db/migrate/20250626165904_add_index_for_merge_request_id_to_generated_ref_commits.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddIndexForMergeRequestIdToGeneratedRefCommits < Gitlab::Database::Migration[2.3] - disable_ddl_transaction! - milestone '18.2' - - def up - add_concurrent_index( - :generated_ref_commits, - :merge_request_id, - name: 'index_generated_ref_commits_on_merge_request_id' - ) - end - - def down - remove_concurrent_index_by_name( - :generated_ref_commits, - 'index_generated_ref_commits_on_merge_request_id' - ) - end -end diff --git a/db/migrate/20250627165139_add_project_id_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb similarity index 58% rename from db/migrate/20250627165139_add_project_id_to_generated_ref_commits.rb rename to db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb index 2d6dd70031a124..1bb27a92b19132 100644 --- a/db/migrate/20250627165139_add_project_id_to_generated_ref_commits.rb +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -3,18 +3,13 @@ # See https://docs.gitlab.com/ee/development/migration_style_guide.html # for more information on how to write migrations for GitLab. -class AddProjectIdToGeneratedRefCommits < Gitlab::Database::Migration[2.3] +class AddProjectIdAsForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' - INDEX_NAME = 'index_generated_ref_commits_on_project_id' FK_NAME = 'fk_generated_ref_commits_project_id' def up - # rubocop:disable Rails/NotNullColumn -- empty table - add_column :generated_ref_commits, :project_id, :bigint, null: false - # rubocop:enable Rails/NotNullColumn -- empty table - add_concurrent_foreign_key :generated_ref_commits, :projects, column: :project_id, @@ -26,7 +21,5 @@ def down remove_foreign_key_if_exists :generated_ref_commits, column: :project_id end - - remove_column :generated_ref_commits, :project_id end end diff --git a/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb b/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb deleted file mode 100644 index 566d4dfe1cccfd..00000000000000 --- a/db/migrate/20250702141038_add_composite_index_project_id_and_commit_sha.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class AddCompositeIndexProjectIdAndCommitSha < Gitlab::Database::Migration[2.3] - milestone '18.2' - disable_ddl_transaction! - - INDEX_NAME = 'index_generated_ref_commits_on_project_id_and_commit_sha' - - def up - add_concurrent_index :generated_ref_commits, [:project_id, :commit_sha], name: INDEX_NAME, - using: :btree - end - - def down - remove_concurrent_index_by_name :generated_ref_commits, INDEX_NAME - end -end diff --git a/db/schema_migrations/20250626165904 b/db/schema_migrations/20250626165904 deleted file mode 100644 index 53fe87eb1f3090..00000000000000 --- a/db/schema_migrations/20250626165904 +++ /dev/null @@ -1 +0,0 @@ -09917fc70b944a621a6abba4f29e9c45abe5d2e655aca343453f22e24eaf59b0 \ No newline at end of file diff --git a/db/schema_migrations/20250702141038 b/db/schema_migrations/20250702141038 deleted file mode 100644 index 1bc484479ca0fb..00000000000000 --- a/db/schema_migrations/20250702141038 +++ /dev/null @@ -1 +0,0 @@ -e31da45a2e6e83524a79a058c8d7b9bfe4e8166147b129e17e6d8b42d05faef9 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 11d084d23aaf73..21f2feb7dd3116 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -15135,7 +15135,7 @@ ALTER SEQUENCE fork_networks_id_seq OWNED BY fork_networks.id; CREATE TABLE generated_ref_commits ( id bigint NOT NULL, - commit_sha bytea, + commit_sha bytea NOT NULL, merge_request_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, diff --git a/spec/models/merge_requests/generated_ref_commit_spec.rb b/spec/models/merge_requests/generated_ref_commit_spec.rb index cb76a51473305c..9c600e2c824524 100644 --- a/spec/models/merge_requests/generated_ref_commit_spec.rb +++ b/spec/models/merge_requests/generated_ref_commit_spec.rb @@ -28,5 +28,15 @@ ref_commit.commit_sha = nil expect(ref_commit).not_to be_valid end + + it 'is not valid without a project_id' do + ref_commit.project_id = nil + expect(ref_commit).not_to be_valid + end + + it 'is not valid without a merge_request_id' do + ref_commit.merge_request_id = nil + expect(ref_commit).not_to be_valid + end end end -- GitLab From ca15a821a1b975958871500c92cf57a08d688455 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 3 Jul 2025 18:26:58 +0200 Subject: [PATCH 37/74] Rename method; add reference additions --- app/models/merge_request.rb | 2 +- app/models/merge_requests/generated_ref_commit.rb | 2 +- app/models/project.rb | 2 +- app/services/merge_requests/create_ref_service.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index d848540bb59773..92630c5496860b 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -72,7 +72,7 @@ class MergeRequest < ApplicationRecord -> { regular }, inverse_of: :merge_request has_many :merge_request_context_commits, inverse_of: :merge_request has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files - has_many :generated_ref_commits, class_name: 'MergeRequests::GeneratedRefCommit' + has_many :generated_ref_commits, class_name: 'MergeRequests::GeneratedRefCommit', inverse_of: :merge_request has_one :merge_request_diff, -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request has_one :merge_head_diff, diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index 9e83c8837f5c63..b9f4b9fe1dd42c 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -6,7 +6,7 @@ class GeneratedRefCommit < ApplicationRecord sha_attribute :commit_sha - belongs_to :merge_request + belongs_to :merge_request, inverse_of: :generated_ref_commits belongs_to :project validates :commit_sha, :project, :merge_request, presence: true diff --git a/app/models/project.rb b/app/models/project.rb index ea6f7cc5f20c46..8eeb6fb3ae2371 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -3001,7 +3001,7 @@ def merge_method end end - def generate_ref_commits? + def can_create_new_ref_commits? merge_method != :merge end diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 1cec29fb2e4f7a..b1793a82474234 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -60,7 +60,7 @@ def execute def store_generated_ref_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha) - return unless commit_shas.any? && target_project.generate_ref_commits? + return unless commit_shas.any? && target_project.can_create_new_ref_commits? # Prepare records for bulk insert records = commit_shas.map do |commit_sha| -- GitLab From 55b67b6c32503eb51f498483d1f13dff0f7a4ea8 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 3 Jul 2025 18:35:40 +0200 Subject: [PATCH 38/74] Remove limit 20 --- db/migrate/20250626095521_add_generated_ref_commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index f2987b1d1c8d7b..3180968a452d6e 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -13,7 +13,7 @@ class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] def up # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found create_table :generated_ref_commits do |t| - t.binary :commit_sha, null: false, limit: 20 + t.binary :commit_sha, null: false t.bigint :merge_request_id, null: false t.bigint :project_id, :bigint, null: false t.timestamps_with_timezone -- GitLab From ecb81c09051a71c1f853e9a6e0d931d4df5689e4 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 3 Jul 2025 18:43:14 +0200 Subject: [PATCH 39/74] Rescue more specific errors --- app/services/merge_requests/create_ref_service.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index b1793a82474234..bcdb7558b0fd16 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -75,7 +75,7 @@ def store_generated_ref_commits(final_commit_sha) # Use upsert to handle potential duplicates GeneratedRefCommit.upsert_all(records) - rescue StandardError => e + rescue ::PG::Error, ::ActiveRecord::ActiveRecordError, ::NoMethodError => e Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) # Log error but don't break the main flow Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") @@ -95,7 +95,8 @@ def collect_commits_data(final_commit_sha) committed_date: commit.committed_date } end - rescue StandardError => e + rescue ::Gitaly::ReferenceNotFoundError, ::Gitaly::FindCommitsError, ::Gitaly::AmbiguousReferenceError, + ::Gitaly::LimitError, ::NoMethodError => e Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) Gitlab::AppLogger.error("Failed to collect commits data for MR #{merge_request.id}: #{e.message}") [] -- GitLab From 02f8e2c2e400c24dd524ea793c733e2334bfaeb0 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 3 Jul 2025 18:59:19 +0200 Subject: [PATCH 40/74] Remove superfluous comment --- app/services/merge_requests/create_ref_service.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index bcdb7558b0fd16..fe2c15d85fc0e7 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -73,7 +73,6 @@ def store_generated_ref_commits(final_commit_sha) } end - # Use upsert to handle potential duplicates GeneratedRefCommit.upsert_all(records) rescue ::PG::Error, ::ActiveRecord::ActiveRecordError, ::NoMethodError => e Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) -- GitLab From 6ba9c35aa8163348680c1d798204d5407f930d88 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 08:58:04 +0200 Subject: [PATCH 41/74] Remove superfluous bigint --- db/migrate/20250626095521_add_generated_ref_commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index 3180968a452d6e..6a595b63cf624c 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -15,7 +15,7 @@ def up create_table :generated_ref_commits do |t| t.binary :commit_sha, null: false t.bigint :merge_request_id, null: false - t.bigint :project_id, :bigint, null: false + t.bigint :project_id, null: false t.timestamps_with_timezone end # rubocop:enable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found -- GitLab From f3ee6c715578bbfe553fe6e62e1bb923ec054881 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 09:05:03 +0200 Subject: [PATCH 42/74] Add fk name to down route --- ...9_add_project_id_as_foreign_key_to_generated_ref_commits.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb index 1bb27a92b19132..4ef1012448fb81 100644 --- a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -19,7 +19,8 @@ def up def down with_lock_retries do remove_foreign_key_if_exists :generated_ref_commits, - column: :project_id + column: :project_id, + name: FK_NAME end end end -- GitLab From 76d9647642809831f34d2f372cf6fae28a099a4c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 09:12:06 +0200 Subject: [PATCH 43/74] Add foreign key name --- ...d_merge_request_foreign_key_to_generated_ref_commits.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index 2367540cefb819..5de434fe94142c 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -7,15 +7,18 @@ class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migrati disable_ddl_transaction! milestone '18.2' + FK_NAME = 'fk_b6cc4921de' + def up add_concurrent_foreign_key :generated_ref_commits, :merge_requests, column: :merge_request_id, - on_delete: :cascade + on_delete: :cascade, + name: FK_NAME end def down with_lock_retries do - remove_foreign_key_if_exists(:generated_ref_commits, column: :merge_request_id) + remove_foreign_key_if_exists(:generated_ref_commits, column: :merge_request_id, name: FK_NAME) end end end -- GitLab From d9f2852b142855f8195c06d0b84420fdc9ba6235 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 9 Jul 2025 09:55:24 +0200 Subject: [PATCH 44/74] Rename generated ref commit factory --- db/migrate/20250626095521_add_generated_ref_commit.rb | 3 +-- ...d_ref_commits.rb => merge_request_generated_ref_commits.rb} | 2 +- spec/models/merge_requests/generated_ref_commit_spec.rb | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) rename spec/factories/{generated_ref_commits.rb => merge_request_generated_ref_commits.rb} (70%) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index 6a595b63cf624c..f708d496c94871 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -11,14 +11,13 @@ class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] INDEX_NAME_COMMITS_AND_PROJECT_ID = 'index_generated_ref_commits_on_project_id_and_commit_sha' def up - # rubocop:disable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found create_table :generated_ref_commits do |t| t.binary :commit_sha, null: false t.bigint :merge_request_id, null: false t.bigint :project_id, null: false t.timestamps_with_timezone end - # rubocop:enable Lint/RedundantCopDisableDirective, Migration/EnsureFactoryForTable -- Exists, but is not found + add_concurrent_index( :generated_ref_commits, :merge_request_id, diff --git a/spec/factories/generated_ref_commits.rb b/spec/factories/merge_request_generated_ref_commits.rb similarity index 70% rename from spec/factories/generated_ref_commits.rb rename to spec/factories/merge_request_generated_ref_commits.rb index def491443be1e2..586d6ec13da8bb 100644 --- a/spec/factories/generated_ref_commits.rb +++ b/spec/factories/merge_request_generated_ref_commits.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true FactoryBot.define do - factory :generated_ref_commit, class: 'MergeRequests::GeneratedRefCommit' do + factory :merge_request_generated_ref_commit, class: 'MergeRequests::GeneratedRefCommit' do commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) } # rubocop:disable Fips/SHA1 -- test data association :project association :merge_request diff --git a/spec/models/merge_requests/generated_ref_commit_spec.rb b/spec/models/merge_requests/generated_ref_commit_spec.rb index 9c600e2c824524..dce2a2a4d2626f 100644 --- a/spec/models/merge_requests/generated_ref_commit_spec.rb +++ b/spec/models/merge_requests/generated_ref_commit_spec.rb @@ -7,7 +7,7 @@ let_it_be(:merge_request) { create(:merge_request, source_project: project) } subject(:ref_commit) do - build(:generated_ref_commit, project: project, merge_request: merge_request) + build(:merge_request_generated_ref_commit, project: project, merge_request: merge_request) end describe 'associations' do -- GitLab From eb818bcfadc8c46d6a6e10f35892aa210b0ff48b Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 10 Jul 2025 13:01:21 +0200 Subject: [PATCH 45/74] Add missing tests; remove thrown exception --- .../merge_requests/create_ref_service.rb | 3 --- .../merge_trains/create_ref_service_spec.rb | 26 ++++++++++++++++++- .../merge_requests/create_ref_service_spec.rb | 24 +++++++++++++++-- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index fe2c15d85fc0e7..5b5c4c6a4b85f2 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -72,10 +72,8 @@ def store_generated_ref_commits(final_commit_sha) updated_at: Time.current } end - GeneratedRefCommit.upsert_all(records) rescue ::PG::Error, ::ActiveRecord::ActiveRecordError, ::NoMethodError => e - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) # Log error but don't break the main flow Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") end @@ -96,7 +94,6 @@ def collect_commits_data(final_commit_sha) end rescue ::Gitaly::ReferenceNotFoundError, ::Gitaly::FindCommitsError, ::Gitaly::AmbiguousReferenceError, ::Gitaly::LimitError, ::NoMethodError => e - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) Gitlab::AppLogger.error("Failed to collect commits data for MR #{merge_request.id}: #{e.message}") [] end diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index 1813f06ba2c18f..fccf81b590f99d 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -146,7 +146,7 @@ end shared_examples 'does not generate ref merge request commits' do - it 'created generated ref merge request commits' do + it 'does not create generated ref merge request commits' do result expect(MergeRequests::GeneratedRefCommit.exists?).to be false @@ -170,6 +170,30 @@ include_examples 'generate ref merge request commits', 3 end + context 'when an error occurs' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + before do + project.merge_method = :rebase_merge + project.save! + allow(project).to receive(:can_create_new_ref_commits?).and_return(true) + end + + context 'when pg error' do + before do + allow(MergeRequests::GeneratedRefCommit).to receive(:upsert_all).and_raise(PG::Error) + end + + it 'rescues and logs PG::Error' do + expect(Gitlab::AppLogger).to receive(:error).with( + a_string_including("Failed to store generated ref commits") + ) + + result + end + end + end + context 'when fast-forward merge strategy' do before do project.merge_method = :ff diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index 0810a423825f9d..045a9a10239b77 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -30,7 +30,7 @@ ) end - subject(:result) do + let(:service) do described_class.new( current_user: user, merge_request: merge_request, @@ -38,7 +38,11 @@ source_sha: source_sha, first_parent_ref: first_parent_ref, merge_params: merge_params - ).execute + ) + end + + subject(:result) do + service.execute end context 'when there is a user-caused gitaly error' do @@ -268,6 +272,22 @@ include_examples 'merge commits with squash' end + context 'when gitaly error' do + before do + allow(service).to receive(:commits_between_refs) + .with(any_args) + .and_raise(::NoMethodError) + end + + it 'rescues and logs gitaly error' do + expect(Gitlab::AppLogger).to receive(:error).with( + a_string_including("Failed to collect commits data") + ) + + result + end + end + context 'when semi-linear merge strategy' do before do project.merge_method = :rebase_merge -- GitLab From 606a1ea852459ef8a116f1700fdaa3d17228da9e Mon Sep 17 00:00:00 2001 From: "Daniel P." Date: Fri, 11 Jul 2025 12:41:36 +0200 Subject: [PATCH 46/74] Apply 3 suggestion(s) to 3 file(s) Co-authored-by: Marius Bobin --- db/migrate/20250626095521_add_generated_ref_commit.rb | 3 --- ..._add_merge_request_foreign_key_to_generated_ref_commits.rb | 3 --- ..._add_project_id_as_foreign_key_to_generated_ref_commits.rb | 4 ---- 3 files changed, 10 deletions(-) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index f708d496c94871..f756aa175f3758 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] milestone '18.2' disable_ddl_transaction! diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index 5de434fe94142c..d20c2c4bcf31da 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -1,8 +1,5 @@ # frozen_string_literal: true -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' diff --git a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb index 4ef1012448fb81..0b06e3c53ffb2e 100644 --- a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -1,8 +1,4 @@ # frozen_string_literal: true - -# See https://docs.gitlab.com/ee/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - class AddProjectIdAsForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' -- GitLab From 8a658cfee8b41948d8e76057dd7b958d858c2952 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 11 Jul 2025 12:46:14 +0200 Subject: [PATCH 47/74] Use if_not_exists and remove explicit index removal --- db/migrate/20250626095521_add_generated_ref_commit.rb | 7 +------ ...d_project_id_as_foreign_key_to_generated_ref_commits.rb | 1 + 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index f756aa175f3758..fdacae4afe9e98 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -8,7 +8,7 @@ class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] INDEX_NAME_COMMITS_AND_PROJECT_ID = 'index_generated_ref_commits_on_project_id_and_commit_sha' def up - create_table :generated_ref_commits do |t| + create_table :generated_ref_commits, if_not_exists: true do |t| t.binary :commit_sha, null: false t.bigint :merge_request_id, null: false t.bigint :project_id, null: false @@ -25,11 +25,6 @@ def up end def down - remove_concurrent_index_by_name( - :generated_ref_commits, - INDEX_NAME_MERGE_REQUEST - ) - remove_concurrent_index_by_name :generated_ref_commits, INDEX_NAME_COMMITS_AND_PROJECT_ID drop_table :generated_ref_commits end end diff --git a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb index 0b06e3c53ffb2e..19c421b7ff6237 100644 --- a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + class AddProjectIdAsForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! milestone '18.2' -- GitLab From e7a86f16a6e319f5cf2a2007b6f146f967e2bfda Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 21 Jul 2025 15:31:46 +0200 Subject: [PATCH 48/74] Apply 3 suggestion(s) to 3 file(s) Co-authored-by: Bala Kumar --- db/migrate/20250626095521_add_generated_ref_commit.rb | 2 +- ...46_add_merge_request_foreign_key_to_generated_ref_commits.rb | 2 +- ...39_add_project_id_as_foreign_key_to_generated_ref_commits.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index fdacae4afe9e98..5f7fc9cd28409e 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] - milestone '18.2' + milestone '18.3' disable_ddl_transaction! INDEX_NAME_MERGE_REQUEST = 'index_generated_ref_commits_on_merge_request_id' diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index d20c2c4bcf31da..15e584d9823376 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -2,7 +2,7 @@ class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! - milestone '18.2' + milestone '18.3' FK_NAME = 'fk_b6cc4921de' diff --git a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb index 19c421b7ff6237..e5b189ee14d7bd 100644 --- a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -2,7 +2,7 @@ class AddProjectIdAsForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] disable_ddl_transaction! - milestone '18.2' + milestone '18.3' FK_NAME = 'fk_generated_ref_commits_project_id' -- GitLab From b1def10fe71c59960ea80d88def856138697b337 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 28 Jul 2025 14:20:03 +0200 Subject: [PATCH 49/74] Refactor to use upsert in batches --- .../merge_requests/create_ref_service.rb | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 5b5c4c6a4b85f2..1438a5297a9094 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -62,17 +62,20 @@ def store_generated_ref_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha) return unless commit_shas.any? && target_project.can_create_new_ref_commits? - # Prepare records for bulk insert - records = commit_shas.map do |commit_sha| - { - merge_request_id: merge_request.id, - commit_sha: commit_sha, - project_id: merge_request.project_id, - created_at: Time.current, - updated_at: Time.current - } + commit_shas.each_slice(1000) do |commit_sha_batch| + # Prepare records for bulk insert + records = commit_sha_batch.map do |commit_sha| + { + merge_request_id: merge_request.id, + commit_sha: commit_sha, + project_id: merge_request.project_id, + created_at: Time.current, + updated_at: Time.current + } + end + GeneratedRefCommit.upsert_all(records) end - GeneratedRefCommit.upsert_all(records) + rescue ::PG::Error, ::ActiveRecord::ActiveRecordError, ::NoMethodError => e # Log error but don't break the main flow Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") -- GitLab From ae7e9f9225a075402984a986b26f4fef90e05c16 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 29 Jul 2025 10:36:04 +0200 Subject: [PATCH 50/74] Switch generated_ref_commits to partitioned table --- .../merge_requests/generated_ref_commit.rb | 7 +++ .../merge_requests/create_ref_service.rb | 2 +- config/initializers/postgres_partitioning.rb | 3 +- ...ommits.yml => p_generated_ref_commits.yml} | 2 +- ...20250626095521_add_generated_ref_commit.rb | 40 +++++++++--- ...st_foreign_key_to_generated_ref_commits.rb | 9 ++- ...as_foreign_key_to_generated_ref_commits.rb | 8 ++- db/structure.sql | 63 ++++++++++--------- 8 files changed, 84 insertions(+), 50 deletions(-) rename db/docs/{generated_ref_commits.yml => p_generated_ref_commits.yml} (90%) diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index b9f4b9fe1dd42c..31ead5bf944aa2 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -3,6 +3,13 @@ module MergeRequests class GeneratedRefCommit < ApplicationRecord include ShaAttribute + include PartitionedTable + + self.table_name = 'p_generated_ref_commits' + self.primary_key = :id + PARTITION_SIZE = 2_000_000 + + partitioned_by :project_id, strategy: :int_range, partition_size: PARTITION_SIZE sha_attribute :commit_sha diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 1438a5297a9094..918422c348a1b9 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -73,7 +73,7 @@ def store_generated_ref_commits(final_commit_sha) updated_at: Time.current } end - GeneratedRefCommit.upsert_all(records) + GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) end rescue ::PG::Error, ::ActiveRecord::ActiveRecordError, ::NoMethodError => e diff --git a/config/initializers/postgres_partitioning.rb b/config/initializers/postgres_partitioning.rb index 7783f6a7599d4f..abcfe8aae22ba1 100644 --- a/config/initializers/postgres_partitioning.rb +++ b/config/initializers/postgres_partitioning.rb @@ -39,7 +39,8 @@ Users::GroupVisit, Users::ProjectVisit, MergeRequest::CommitsMetadata, - WebHookLog + WebHookLog, + MergeRequests::GeneratedRefCommit ]) if Gitlab.ee? diff --git a/db/docs/generated_ref_commits.yml b/db/docs/p_generated_ref_commits.yml similarity index 90% rename from db/docs/generated_ref_commits.yml rename to db/docs/p_generated_ref_commits.yml index 10a6244fb2c2c8..11a3c8531d9d25 100644 --- a/db/docs/generated_ref_commits.yml +++ b/db/docs/p_generated_ref_commits.yml @@ -1,5 +1,5 @@ --- -table_name: generated_ref_commits +table_name: p_generated_ref_commits classes: - MergeRequests::GeneratedRefCommit feature_categories: diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index 5f7fc9cd28409e..c09ea0a92b877e 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -1,30 +1,50 @@ # frozen_string_literal: true class AddGeneratedRefCommit < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + milestone '18.3' disable_ddl_transaction! - INDEX_NAME_MERGE_REQUEST = 'index_generated_ref_commits_on_merge_request_id' - INDEX_NAME_COMMITS_AND_PROJECT_ID = 'index_generated_ref_commits_on_project_id_and_commit_sha' + INDEX_NAME_MERGE_REQUEST = 'p_index_generated_ref_commits_on_merge_request_id' + INDEX_NAME_COMMITS_AND_PROJECT_ID = 'p_index_generated_ref_commits_on_project_id_and_commit_sha' + TABLE_NAME = :p_generated_ref_commits + PARTITION_SIZE = 2_000_000 def up - create_table :generated_ref_commits, if_not_exists: true do |t| + create_table TABLE_NAME, + options: 'PARTITION BY RANGE (project_id)', + primary_key: [:id, :project_id], if_not_exists: true do |t| + t.bigserial :id, null: false t.binary :commit_sha, null: false t.bigint :merge_request_id, null: false t.bigint :project_id, null: false t.timestamps_with_timezone end - add_concurrent_index( - :generated_ref_commits, - :merge_request_id, - name: INDEX_NAME_MERGE_REQUEST - ) - add_concurrent_index :generated_ref_commits, [:project_id, :commit_sha], name: INDEX_NAME_COMMITS_AND_PROJECT_ID, + add_concurrent_partitioned_index TABLE_NAME, :merge_request_id, name: INDEX_NAME_MERGE_REQUEST + + add_concurrent_partitioned_index TABLE_NAME, [:project_id, :commit_sha], + name: INDEX_NAME_COMMITS_AND_PROJECT_ID, using: :btree + + create_partitions end def down - drop_table :generated_ref_commits + drop_table TABLE_NAME + end + + private + + def create_partitions + min_id = 1 + max_id = Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do + Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection.with_suppressed do + define_batchable_model('projects', connection: connection).maximum(:id) || min_id + end + end + + create_int_range_partitions(TABLE_NAME, PARTITION_SIZE, min_id, max_id) end end diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index 15e584d9823376..c0bf54568dc302 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -1,13 +1,16 @@ # frozen_string_literal: true class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + disable_ddl_transaction! milestone '18.3' - FK_NAME = 'fk_b6cc4921de' + FK_NAME = 'fk_generated_ref_commits_merge_request_id' + TABLE_NAME = :p_generated_ref_commits def up - add_concurrent_foreign_key :generated_ref_commits, :merge_requests, + add_concurrent_partitioned_foreign_key TABLE_NAME, :merge_requests, column: :merge_request_id, on_delete: :cascade, name: FK_NAME @@ -15,7 +18,7 @@ def up def down with_lock_retries do - remove_foreign_key_if_exists(:generated_ref_commits, column: :merge_request_id, name: FK_NAME) + remove_foreign_key_if_exists(TABLE_NAME, column: :merge_request_id, name: FK_NAME) end end end diff --git a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb index e5b189ee14d7bd..a92d14b222c446 100644 --- a/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250627165139_add_project_id_as_foreign_key_to_generated_ref_commits.rb @@ -1,13 +1,15 @@ # frozen_string_literal: true class AddProjectIdAsForeignKeyToGeneratedRefCommits < Gitlab::Database::Migration[2.3] + include Gitlab::Database::PartitioningMigrationHelpers + disable_ddl_transaction! milestone '18.3' FK_NAME = 'fk_generated_ref_commits_project_id' - + TABLE_NAME = :p_generated_ref_commits def up - add_concurrent_foreign_key :generated_ref_commits, + add_concurrent_partitioned_foreign_key TABLE_NAME, :projects, column: :project_id, name: FK_NAME @@ -15,7 +17,7 @@ def up def down with_lock_retries do - remove_foreign_key_if_exists :generated_ref_commits, + remove_foreign_key_if_exists TABLE_NAME, column: :project_id, name: FK_NAME end diff --git a/db/structure.sql b/db/structure.sql index 21f2feb7dd3116..29554fb358f62e 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5096,6 +5096,16 @@ CREATE TABLE p_duo_workflows_checkpoints ( ) PARTITION BY RANGE (created_at); +CREATE TABLE p_generated_ref_commits ( + id bigint NOT NULL, + commit_sha bytea NOT NULL, + merge_request_id bigint NOT NULL, + project_id bigint NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL +) +PARTITION BY RANGE (project_id); + CREATE TABLE p_knowledge_graph_enabled_namespaces ( id bigint NOT NULL, namespace_id bigint NOT NULL, @@ -15133,24 +15143,6 @@ CREATE SEQUENCE fork_networks_id_seq ALTER SEQUENCE fork_networks_id_seq OWNED BY fork_networks.id; -CREATE TABLE generated_ref_commits ( - id bigint NOT NULL, - commit_sha bytea NOT NULL, - merge_request_id bigint NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - project_id bigint NOT NULL -); - -CREATE SEQUENCE generated_ref_commits_id_seq - START WITH 1 - INCREMENT BY 1 - NO MINVALUE - NO MAXVALUE - CACHE 1; - -ALTER SEQUENCE generated_ref_commits_id_seq OWNED BY generated_ref_commits.id; - CREATE TABLE geo_cache_invalidation_events ( id bigint NOT NULL, key character varying NOT NULL @@ -19496,6 +19488,15 @@ CREATE SEQUENCE p_duo_workflows_checkpoints_id_seq ALTER SEQUENCE p_duo_workflows_checkpoints_id_seq OWNED BY p_duo_workflows_checkpoints.id; +CREATE SEQUENCE p_generated_ref_commits_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE p_generated_ref_commits_id_seq OWNED BY p_generated_ref_commits.id; + CREATE SEQUENCE p_knowledge_graph_enabled_namespaces_id_seq START WITH 1 INCREMENT BY 1 @@ -28270,8 +28271,6 @@ ALTER TABLE ONLY fork_network_members ALTER COLUMN id SET DEFAULT nextval('fork_ ALTER TABLE ONLY fork_networks ALTER COLUMN id SET DEFAULT nextval('fork_networks_id_seq'::regclass); -ALTER TABLE ONLY generated_ref_commits ALTER COLUMN id SET DEFAULT nextval('generated_ref_commits_id_seq'::regclass); - ALTER TABLE ONLY geo_cache_invalidation_events ALTER COLUMN id SET DEFAULT nextval('geo_cache_invalidation_events_id_seq'::regclass); ALTER TABLE ONLY geo_event_log ALTER COLUMN id SET DEFAULT nextval('geo_event_log_id_seq'::regclass); @@ -28620,6 +28619,8 @@ ALTER TABLE ONLY p_ci_job_inputs ALTER COLUMN id SET DEFAULT nextval('p_ci_job_i ALTER TABLE ONLY p_ci_workloads ALTER COLUMN id SET DEFAULT nextval('p_ci_workloads_id_seq'::regclass); +ALTER TABLE ONLY p_generated_ref_commits ALTER COLUMN id SET DEFAULT nextval('p_generated_ref_commits_id_seq'::regclass); + ALTER TABLE ONLY p_knowledge_graph_enabled_namespaces ALTER COLUMN id SET DEFAULT nextval('p_knowledge_graph_enabled_namespaces_id_seq'::regclass); ALTER TABLE ONLY p_knowledge_graph_replicas ALTER COLUMN id SET DEFAULT nextval('p_knowledge_graph_replicas_id_seq'::regclass); @@ -30809,9 +30810,6 @@ ALTER TABLE ONLY fork_network_members ALTER TABLE ONLY fork_networks ADD CONSTRAINT fork_networks_pkey PRIMARY KEY (id); -ALTER TABLE ONLY generated_ref_commits - ADD CONSTRAINT generated_ref_commits_pkey PRIMARY KEY (id); - ALTER TABLE ONLY geo_cache_invalidation_events ADD CONSTRAINT geo_cache_invalidation_events_pkey PRIMARY KEY (id); @@ -31487,6 +31485,9 @@ ALTER TABLE ONLY p_ci_workloads ALTER TABLE ONLY p_duo_workflows_checkpoints ADD CONSTRAINT p_duo_workflows_checkpoints_pkey PRIMARY KEY (id, created_at); +ALTER TABLE ONLY p_generated_ref_commits + ADD CONSTRAINT p_generated_ref_commits_pkey PRIMARY KEY (id, project_id); + ALTER TABLE ONLY p_knowledge_graph_enabled_namespaces ADD CONSTRAINT p_knowledge_graph_enabled_namespaces_pkey PRIMARY KEY (id, namespace_id); @@ -36284,10 +36285,6 @@ CREATE INDEX index_fork_networks_on_organization_id ON fork_networks USING btree CREATE UNIQUE INDEX index_fork_networks_on_root_project_id ON fork_networks USING btree (root_project_id); -CREATE INDEX index_generated_ref_commits_on_merge_request_id ON generated_ref_commits USING btree (merge_request_id); - -CREATE INDEX index_generated_ref_commits_on_project_id_and_commit_sha ON generated_ref_commits USING btree (project_id, commit_sha); - CREATE INDEX index_geo_event_log_on_cache_invalidation_event_id ON geo_event_log USING btree (cache_invalidation_event_id) WHERE (cache_invalidation_event_id IS NOT NULL); CREATE INDEX index_geo_event_log_on_geo_event_id ON geo_event_log USING btree (geo_event_id) WHERE (geo_event_id IS NOT NULL); @@ -39702,6 +39699,10 @@ CREATE INDEX p_ci_stages_project_id_idx ON ONLY p_ci_stages USING btree (project CREATE UNIQUE INDEX p_ci_workloads_pipeline_id_idx ON ONLY p_ci_workloads USING btree (pipeline_id, partition_id); +CREATE INDEX p_index_generated_ref_commits_on_merge_request_id ON ONLY p_generated_ref_commits USING btree (merge_request_id); + +CREATE INDEX p_index_generated_ref_commits_on_project_id_and_commit_sha ON ONLY p_generated_ref_commits USING btree (project_id, commit_sha); + CREATE UNIQUE INDEX p_knowledge_graph_replicas_namespace_id_and_zoekt_node_id ON ONLY p_knowledge_graph_replicas USING btree (knowledge_graph_enabled_namespace_id, zoekt_node_id, namespace_id); CREATE INDEX package_name_index ON packages_packages USING btree (name); @@ -44712,9 +44713,6 @@ ALTER TABLE ONLY description_versions ALTER TABLE ONLY bulk_import_entities ADD CONSTRAINT fk_b69fa2b2df FOREIGN KEY (bulk_import_id) REFERENCES bulk_imports(id) ON DELETE CASCADE; -ALTER TABLE ONLY generated_ref_commits - ADD CONSTRAINT fk_b6cc4921de FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; - ALTER TABLE ONLY security_policy_requirements ADD CONSTRAINT fk_b6e48e3428 FOREIGN KEY (compliance_framework_security_policy_id) REFERENCES compliance_framework_security_policies(id) ON DELETE CASCADE; @@ -45384,7 +45382,10 @@ ALTER TABLE ONLY packages_conan_package_revisions ALTER TABLE ONLY issues ADD CONSTRAINT fk_ffed080f01 FOREIGN KEY (updated_by_id) REFERENCES users(id) ON DELETE SET NULL; -ALTER TABLE ONLY generated_ref_commits +ALTER TABLE p_generated_ref_commits + ADD CONSTRAINT fk_generated_ref_commits_merge_request_id FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + +ALTER TABLE p_generated_ref_commits ADD CONSTRAINT fk_generated_ref_commits_project_id FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; ALTER TABLE ONLY geo_event_log -- GitLab From 310cc1d8bdece09991f41fed98b5d2a39adcfb37 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 30 Jul 2025 12:39:59 +0200 Subject: [PATCH 51/74] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Dylan Griffith --- db/migrate/20250626095521_add_generated_ref_commit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index c09ea0a92b877e..94610bd94d0a95 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -16,10 +16,10 @@ def up options: 'PARTITION BY RANGE (project_id)', primary_key: [:id, :project_id], if_not_exists: true do |t| t.bigserial :id, null: false - t.binary :commit_sha, null: false t.bigint :merge_request_id, null: false t.bigint :project_id, null: false t.timestamps_with_timezone + t.binary :commit_sha, null: false end add_concurrent_partitioned_index TABLE_NAME, :merge_request_id, name: INDEX_NAME_MERGE_REQUEST -- GitLab From f696863d468e465838840cfcf68616f01091725c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 30 Jul 2025 12:56:25 +0200 Subject: [PATCH 52/74] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Dylan Griffith --- ...46_add_merge_request_foreign_key_to_generated_ref_commits.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index c0bf54568dc302..5f669bf150d4fa 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -11,7 +11,7 @@ class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migrati def up add_concurrent_partitioned_foreign_key TABLE_NAME, :merge_requests, - column: :merge_request_id, + column: [:project_id, :merge_request_id], on_delete: :cascade, name: FK_NAME end -- GitLab From 441327190bbe4dde38a2717e4f00913d9c554c88 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 30 Jul 2025 13:47:14 +0200 Subject: [PATCH 53/74] Slim down errors --- .../merge_requests/create_ref_service.rb | 6 +----- ...quest_foreign_key_to_generated_ref_commits.rb | 5 +++-- .../merge_requests/create_ref_service_spec.rb | 16 ---------------- 3 files changed, 4 insertions(+), 23 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 918422c348a1b9..88255b67674965 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -76,7 +76,7 @@ def store_generated_ref_commits(final_commit_sha) GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) end - rescue ::PG::Error, ::ActiveRecord::ActiveRecordError, ::NoMethodError => e + rescue ::PG::Error => e # Log error but don't break the main flow Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") end @@ -95,10 +95,6 @@ def collect_commits_data(final_commit_sha) committed_date: commit.committed_date } end - rescue ::Gitaly::ReferenceNotFoundError, ::Gitaly::FindCommitsError, ::Gitaly::AmbiguousReferenceError, - ::Gitaly::LimitError, ::NoMethodError => e - Gitlab::AppLogger.error("Failed to collect commits data for MR #{merge_request.id}: #{e.message}") - [] end def commits_between_refs(final_commit_sha) diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index 5f669bf150d4fa..f50259ac793c25 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -11,14 +11,15 @@ class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migrati def up add_concurrent_partitioned_foreign_key TABLE_NAME, :merge_requests, - column: [:project_id, :merge_request_id], + column: :merge_request_id, on_delete: :cascade, name: FK_NAME end def down with_lock_retries do - remove_foreign_key_if_exists(TABLE_NAME, column: :merge_request_id, name: FK_NAME) + remove_foreign_key_if_exists(TABLE_NAME, column: :merge_request_id, + name: FK_NAME) end end end diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index 045a9a10239b77..6672f9f538724d 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -272,22 +272,6 @@ include_examples 'merge commits with squash' end - context 'when gitaly error' do - before do - allow(service).to receive(:commits_between_refs) - .with(any_args) - .and_raise(::NoMethodError) - end - - it 'rescues and logs gitaly error' do - expect(Gitlab::AppLogger).to receive(:error).with( - a_string_including("Failed to collect commits data") - ) - - result - end - end - context 'when semi-linear merge strategy' do before do project.merge_method = :rebase_merge -- GitLab From 88d019557a7b15836f95e5c03bb2aabd92308bdb Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Wed, 30 Jul 2025 13:54:57 +0200 Subject: [PATCH 54/74] Wrap generated ref commit upsert into a transaction --- .../merge_requests/create_ref_service.rb | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 88255b67674965..b131f0c26ba043 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -62,20 +62,21 @@ def store_generated_ref_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha) return unless commit_shas.any? && target_project.can_create_new_ref_commits? - commit_shas.each_slice(1000) do |commit_sha_batch| - # Prepare records for bulk insert - records = commit_sha_batch.map do |commit_sha| - { - merge_request_id: merge_request.id, - commit_sha: commit_sha, - project_id: merge_request.project_id, - created_at: Time.current, - updated_at: Time.current - } + GeneratedRefCommit.transaction do + commit_shas.each_slice(1000) do |commit_sha_batch| + # Prepare records for bulk insert + records = commit_sha_batch.map do |commit_sha| + { + merge_request_id: merge_request.id, + commit_sha: commit_sha, + project_id: merge_request.project_id, + created_at: Time.current, + updated_at: Time.current + } + end + GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) end - GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) end - rescue ::PG::Error => e # Log error but don't break the main flow Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") -- GitLab From fad26373d1e977defdf24af3e11f204c04c3ec00 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 31 Jul 2025 14:26:25 +0200 Subject: [PATCH 55/74] Add project_id to queries; remove commits_data --- .../merge_requests/create_ref_service.rb | 19 ------------------- ee/app/models/merge_trains/car.rb | 5 ++++- .../merge_trains/create_ref_service_spec.rb | 9 +++++---- 3 files changed, 9 insertions(+), 24 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index b131f0c26ba043..6f0f4bab77b1e7 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -42,9 +42,6 @@ def execute # Store commits in generated_ref_commits table using the final commit_sha store_generated_ref_commits(result[:commit_sha]) if merge_request.merge_train_car.present? - # Add commits data to result for external use - result[:commits_data] = collect_commits_data(result[:commit_sha]) - ServiceResponse.success(payload: result) rescue CreateRefError => error ServiceResponse.error(message: error.message) @@ -82,22 +79,6 @@ def store_generated_ref_commits(final_commit_sha) Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") end - def collect_commits_data(final_commit_sha) - return [] unless final_commit_sha && first_parent_sha - - commits_between_refs(final_commit_sha).map do |commit| - { - commit_sha: commit.sha, - merge_request_id: merge_request.id, - message: commit.message, - author_name: commit.author_name, - author_email: commit.author_email, - authored_date: commit.authored_date, - committed_date: commit.committed_date - } - end - end - def commits_between_refs(final_commit_sha) return [] unless final_commit_sha && first_parent_sha diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 5dc8030c7927d4..366110bfdb8f65 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -205,7 +205,10 @@ def train def cleanup_ref(async: true) # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. - MergeRequests::GeneratedRefCommit.where(merge_request: merge_request).delete_all unless merged? + unless merged? + MergeRequests::GeneratedRefCommit + .where(project: target_project, merge_request: merge_request).delete_all + end if async merge_request.schedule_cleanup_refs(only: :train) diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index fccf81b590f99d..914c89b04b61b7 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -136,11 +136,12 @@ it 'created generated ref merge request commits' do result - expect(MergeRequests::GeneratedRefCommit.where.not(merge_request_id: merge_request.id)) + expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_id: merge_request.id)) .to be_empty - expect(MergeRequests::GeneratedRefCommit.where(merge_request_id: merge_request.id).count).to eq( - commit_count - ) + expect(MergeRequests::GeneratedRefCommit.where(project: project, + merge_request_id: merge_request.id).count).to eq( + commit_count + ) end end end -- GitLab From e169d244c66fcc21413b520f7b9167a25352984b Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 31 Jul 2025 14:46:01 +0200 Subject: [PATCH 56/74] Refactor car spec --- ee/spec/models/merge_trains/car_spec.rb | 54 +++++++++++++------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 9ed07b53fa04f3..788c12976a7e9a 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -521,38 +521,42 @@ let(:train_car) { create(:merge_train_car) } let(:generated_ref_commits) { class_double(MergeRequests::GeneratedRefCommit) } - context 'when a merge request is merged' do - subject { train_car.try_cleanup_ref } - + context 'when we deal with generated ref commits' do before do - stub_const('MergeRequests::GeneratedRefCommit', generated_ref_commits) - allow(generated_ref_commits).to receive(:where) - .and_return(generated_ref_commits) - allow(generated_ref_commits).to receive(:delete_all) + create_list( + :merge_request_generated_ref_commit, + 2, + project: train_car.target_project, + merge_request: train_car.merge_request + ) end - it 'does not delete created ref merge request commits' do - train_car.refresh_pipeline! - train_car.start_merge! - train_car.finish_merge! - expect(generated_ref_commits).not_to receive(:delete_all) - subject - end - end + context 'when a merge request is merged' do + subject { train_car.try_cleanup_ref } - context 'when a merge request is not merged' do - subject { train_car.try_cleanup_ref } + it 'does not delete created ref merge request commits' do + train_car.refresh_pipeline! + train_car.start_merge! + train_car.finish_merge! + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.project, + merge_request: train_car.merge_request) - before do - stub_const('MergeRequests::GeneratedRefCommit', generated_ref_commits) - allow(generated_ref_commits).to receive(:where) - .and_return(generated_ref_commits) - allow(generated_ref_commits).to receive(:delete_all) + subject + expect(generated_ref_commits.count).to eq(2) + end end - it 'does delete created ref merge request commits' do - expect(generated_ref_commits).to receive(:delete_all) - subject + context 'when a merge request is not merged' do + subject { train_car.try_cleanup_ref(async: false) } + + it 'does delete created ref merge request commits' do + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.project, + merge_request: train_car.merge_request) + + expect(generated_ref_commits.count).to eq(2) + subject + expect(generated_ref_commits.count).to eq(0) + end end end -- GitLab From ec20bff887a3301a28dfd74b95098f81809fb134 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 31 Jul 2025 15:58:53 +0200 Subject: [PATCH 57/74] Hide generate ref commits behind ff --- app/services/merge_requests/create_ref_service.rb | 6 ++++-- .../gitlab_com_derisk/generate_ref_commits.yml | 10 ++++++++++ ee/app/models/merge_trains/car.rb | 2 +- ee/spec/models/merge_trains/car_spec.rb | 15 +++++++++++++++ .../merge_trains/create_ref_service_spec.rb | 8 ++++++++ 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 6f0f4bab77b1e7..1c5152c30e15cc 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,8 +39,10 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) - # Store commits in generated_ref_commits table using the final commit_sha - store_generated_ref_commits(result[:commit_sha]) if merge_request.merge_train_car.present? + if Feature.enabled?(:generate_ref_commits, merge_request.target_project) && merge_request.merge_train_car.present? + # Store commits in generated_ref_commits table using the final commit_sha + store_generated_ref_commits(result[:commit_sha]) + end ServiceResponse.success(payload: result) rescue CreateRefError => error diff --git a/config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml b/config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml new file mode 100644 index 00000000000000..c63af2c1d01938 --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/generate_ref_commits.yml @@ -0,0 +1,10 @@ +--- +name: generate_ref_commits +description: +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/436943 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/195831 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/558276 +milestone: '18.3' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index 366110bfdb8f65..efb56864f14ab6 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -205,7 +205,7 @@ def train def cleanup_ref(async: true) # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. - unless merged? + if Feature.enabled?(:generate_ref_commits, merge_request.target_project) && !merged? MergeRequests::GeneratedRefCommit .where(project: target_project, merge_request: merge_request).delete_all end diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 788c12976a7e9a..c7c07360b76a7a 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -557,6 +557,21 @@ subject expect(generated_ref_commits.count).to eq(0) end + + context 'when the ff for generate ref commits is disabled' do + before do + stub_feature_flags(generate_ref_commits: false) + end + + it 'does not delete created ref merge request commits' do + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.project, + merge_request: train_car.merge_request) + + expect(generated_ref_commits.count).to eq(2) + subject + expect(generated_ref_commits.count).to eq(2) + end + end end end diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index 914c89b04b61b7..beac970a5c8eb8 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -143,6 +143,14 @@ commit_count ) end + + context 'when ff for generate ref commits is disabled' do + before do + stub_feature_flags(generate_ref_commits: false) + end + + include_examples 'does not generate ref merge request commits' + end end end -- GitLab From c1525e6ae9d797779a6132a48aa486e10c2ac8c1 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 31 Jul 2025 17:46:20 +0200 Subject: [PATCH 58/74] Regenerate structure.sql layout --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index 29554fb358f62e..c01b2762b39295 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5098,11 +5098,11 @@ PARTITION BY RANGE (created_at); CREATE TABLE p_generated_ref_commits ( id bigint NOT NULL, - commit_sha bytea NOT NULL, merge_request_id bigint NOT NULL, project_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL + commit_sha bytea NOT NULL ) PARTITION BY RANGE (project_id); -- GitLab From 9d95af43d5e6e73d9d23f3f58ac6adfdae9fdba0 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 31 Jul 2025 17:48:31 +0200 Subject: [PATCH 59/74] Add missing comma --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index c01b2762b39295..4605386a086a0c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5101,7 +5101,7 @@ CREATE TABLE p_generated_ref_commits ( merge_request_id bigint NOT NULL, project_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL + updated_at timestamp with time zone NOT NULL, commit_sha bytea NOT NULL ) PARTITION BY RANGE (project_id); -- GitLab From 7502aee9185fc9dacb9bf5583f32ca854bb2694c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 1 Aug 2025 10:10:57 +0200 Subject: [PATCH 60/74] Add composite foreign key --- app/models/merge_requests/generated_ref_commit.rb | 2 +- app/services/merge_requests/create_ref_service.rb | 2 +- db/migrate/20250626095521_add_generated_ref_commit.rb | 4 ++-- ...dd_merge_request_foreign_key_to_generated_ref_commits.rb | 5 +++-- db/structure.sql | 6 +++--- ee/spec/services/merge_trains/create_ref_service_spec.rb | 4 ++-- 6 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index 31ead5bf944aa2..c52467812f01e9 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -13,7 +13,7 @@ class GeneratedRefCommit < ApplicationRecord sha_attribute :commit_sha - belongs_to :merge_request, inverse_of: :generated_ref_commits + belongs_to :merge_request, inverse_of: :generated_ref_commits, foreign_key: :iid belongs_to :project validates :commit_sha, :project, :merge_request, presence: true diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 1c5152c30e15cc..77885864e7e977 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -66,7 +66,7 @@ def store_generated_ref_commits(final_commit_sha) # Prepare records for bulk insert records = commit_sha_batch.map do |commit_sha| { - merge_request_id: merge_request.id, + merge_request_iid: merge_request.iid, commit_sha: commit_sha, project_id: merge_request.project_id, created_at: Time.current, diff --git a/db/migrate/20250626095521_add_generated_ref_commit.rb b/db/migrate/20250626095521_add_generated_ref_commit.rb index 94610bd94d0a95..8ab7f4213a6790 100644 --- a/db/migrate/20250626095521_add_generated_ref_commit.rb +++ b/db/migrate/20250626095521_add_generated_ref_commit.rb @@ -16,13 +16,13 @@ def up options: 'PARTITION BY RANGE (project_id)', primary_key: [:id, :project_id], if_not_exists: true do |t| t.bigserial :id, null: false - t.bigint :merge_request_id, null: false + t.bigint :merge_request_iid, null: false t.bigint :project_id, null: false t.timestamps_with_timezone t.binary :commit_sha, null: false end - add_concurrent_partitioned_index TABLE_NAME, :merge_request_id, name: INDEX_NAME_MERGE_REQUEST + add_concurrent_partitioned_index TABLE_NAME, [:project_id, :merge_request_iid], name: INDEX_NAME_MERGE_REQUEST add_concurrent_partitioned_index TABLE_NAME, [:project_id, :commit_sha], name: INDEX_NAME_COMMITS_AND_PROJECT_ID, diff --git a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb index f50259ac793c25..5caa72fc8e7cf4 100644 --- a/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb +++ b/db/migrate/20250626123746_add_merge_request_foreign_key_to_generated_ref_commits.rb @@ -11,14 +11,15 @@ class AddMergeRequestForeignKeyToGeneratedRefCommits < Gitlab::Database::Migrati def up add_concurrent_partitioned_foreign_key TABLE_NAME, :merge_requests, - column: :merge_request_id, + column: [:project_id, :merge_request_iid], + target_column: [:target_project_id, :iid], on_delete: :cascade, name: FK_NAME end def down with_lock_retries do - remove_foreign_key_if_exists(TABLE_NAME, column: :merge_request_id, + remove_foreign_key_if_exists(TABLE_NAME, column: :merge_request_iid, name: FK_NAME) end end diff --git a/db/structure.sql b/db/structure.sql index 4605386a086a0c..b52e5d2895fe4f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -5098,7 +5098,7 @@ PARTITION BY RANGE (created_at); CREATE TABLE p_generated_ref_commits ( id bigint NOT NULL, - merge_request_id bigint NOT NULL, + merge_request_iid bigint NOT NULL, project_id bigint NOT NULL, created_at timestamp with time zone NOT NULL, updated_at timestamp with time zone NOT NULL, @@ -39699,7 +39699,7 @@ CREATE INDEX p_ci_stages_project_id_idx ON ONLY p_ci_stages USING btree (project CREATE UNIQUE INDEX p_ci_workloads_pipeline_id_idx ON ONLY p_ci_workloads USING btree (pipeline_id, partition_id); -CREATE INDEX p_index_generated_ref_commits_on_merge_request_id ON ONLY p_generated_ref_commits USING btree (merge_request_id); +CREATE INDEX p_index_generated_ref_commits_on_merge_request_id ON ONLY p_generated_ref_commits USING btree (project_id, merge_request_iid); CREATE INDEX p_index_generated_ref_commits_on_project_id_and_commit_sha ON ONLY p_generated_ref_commits USING btree (project_id, commit_sha); @@ -45383,7 +45383,7 @@ ALTER TABLE ONLY issues ADD CONSTRAINT fk_ffed080f01 FOREIGN KEY (updated_by_id) REFERENCES users(id) ON DELETE SET NULL; ALTER TABLE p_generated_ref_commits - ADD CONSTRAINT fk_generated_ref_commits_merge_request_id FOREIGN KEY (merge_request_id) REFERENCES merge_requests(id) ON DELETE CASCADE; + ADD CONSTRAINT fk_generated_ref_commits_merge_request_id FOREIGN KEY (project_id, merge_request_iid) REFERENCES merge_requests(target_project_id, iid) ON DELETE CASCADE; ALTER TABLE p_generated_ref_commits ADD CONSTRAINT fk_generated_ref_commits_project_id FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index beac970a5c8eb8..34d1cdf9ed10c5 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -136,10 +136,10 @@ it 'created generated ref merge request commits' do result - expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_id: merge_request.id)) + expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_iid: merge_request.iid)) .to be_empty expect(MergeRequests::GeneratedRefCommit.where(project: project, - merge_request_id: merge_request.id).count).to eq( + merge_request_iid: merge_request.iid).count).to eq( commit_count ) end -- GitLab From 0244e53ec003cf8395d8d008770ba1dee7154349 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 1 Aug 2025 10:42:44 +0200 Subject: [PATCH 61/74] Refactor create_ref_service spec --- .../merge_trains/create_ref_service_spec.rb | 60 ------------------- .../merge_requests/create_ref_service_spec.rb | 60 +++++++++++++++++++ 2 files changed, 60 insertions(+), 60 deletions(-) diff --git a/ee/spec/services/merge_trains/create_ref_service_spec.rb b/ee/spec/services/merge_trains/create_ref_service_spec.rb index 34d1cdf9ed10c5..532935b9cfb651 100644 --- a/ee/spec/services/merge_trains/create_ref_service_spec.rb +++ b/ee/spec/services/merge_trains/create_ref_service_spec.rb @@ -129,43 +129,9 @@ end end - shared_examples 'generate ref merge request commits' do |commit_count| - context 'when coming from a merge train' do - let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } - - it 'created generated ref merge request commits' do - result - - expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_iid: merge_request.iid)) - .to be_empty - expect(MergeRequests::GeneratedRefCommit.where(project: project, - merge_request_iid: merge_request.iid).count).to eq( - commit_count - ) - end - - context 'when ff for generate ref commits is disabled' do - before do - stub_feature_flags(generate_ref_commits: false) - end - - include_examples 'does not generate ref merge request commits' - end - end - end - - shared_examples 'does not generate ref merge request commits' do - it 'does not create generated ref merge request commits' do - result - - expect(MergeRequests::GeneratedRefCommit.exists?).to be false - end - end - context 'when merged commit strategy' do include_examples 'merge commits without squash' include_examples 'merge commits with squash' - include_examples 'does not generate ref merge request commits' end context 'when semi-linear merge strategy' do @@ -176,31 +142,6 @@ include_examples 'merge commits without squash' include_examples 'merge commits with squash' - include_examples 'generate ref merge request commits', 3 - end - - context 'when an error occurs' do - let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } - - before do - project.merge_method = :rebase_merge - project.save! - allow(project).to receive(:can_create_new_ref_commits?).and_return(true) - end - - context 'when pg error' do - before do - allow(MergeRequests::GeneratedRefCommit).to receive(:upsert_all).and_raise(PG::Error) - end - - it 'rescues and logs PG::Error' do - expect(Gitlab::AppLogger).to receive(:error).with( - a_string_including("Failed to store generated ref commits") - ) - - result - end - end end context 'when fast-forward merge strategy' do @@ -210,7 +151,6 @@ end it_behaves_like 'writing without a merge commit' - it_behaves_like 'generate ref merge request commits', 2 context 'when squash set' do let(:squash) { true } diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index 6672f9f538724d..2f14f2f05bda40 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -91,6 +91,39 @@ branch_name: first_parent_ref ) end + shared_examples 'generate ref merge request commits' do |commit_count| + context 'when coming from a merge train' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + it 'created generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_iid: merge_request.iid)) + .to be_empty + + expect(MergeRequests::GeneratedRefCommit.where(project: project, + merge_request_iid: merge_request.iid).count).to eq( + commit_count + ) + end + + context 'when ff for generate ref commits is disabled' do + before do + stub_feature_flags(generate_ref_commits: false) + end + + include_examples 'does not generate ref merge request commits' + end + end + end + + shared_examples 'does not generate ref merge request commits' do + it 'does not create generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.exists?).to be false + end + end shared_examples_for 'writing with a merge commit' do it 'merges with a merge commit', :aggregate_failures do @@ -226,6 +259,30 @@ end end + context 'when a database error occurs' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + before do + project.merge_method = :rebase_merge + project.save! + allow(project).to receive(:can_create_new_ref_commits?).and_return(true) + end + + context 'when pg error' do + before do + allow(MergeRequests::GeneratedRefCommit).to receive(:upsert_all).and_raise(PG::Error) + end + + it 'rescues and logs PG::Error' do + expect(Gitlab::AppLogger).to receive(:error).with( + a_string_including("Failed to store generated ref commits") + ) + + result + end + end + end + context 'when the merge commit message is provided at time of merge' do let(:expected_merge_commit) { 'something custom' } let(:extra_mr_merge_params) { {} } @@ -270,6 +327,7 @@ context 'when merged commit strategy' do include_examples 'merge commits without squash' include_examples 'merge commits with squash' + include_examples 'does not generate ref merge request commits' end context 'when semi-linear merge strategy' do @@ -280,6 +338,7 @@ include_examples 'merge commits without squash' include_examples 'merge commits with squash' + include_examples 'generate ref merge request commits', 3 context 'when the target ref changes between rebase and merge' do # this tests internal handling of expected_old_oid @@ -311,6 +370,7 @@ end it_behaves_like 'writing without a merge commit' + it_behaves_like 'generate ref merge request commits', 2 context 'when squash set' do let(:squash) { true } -- GitLab From d6717f84e145238627c834bedde5e4fadaf9ff8c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 1 Aug 2025 10:47:07 +0200 Subject: [PATCH 62/74] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Dylan Griffith --- app/finders/merge_requests_finder.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/finders/merge_requests_finder.rb b/app/finders/merge_requests_finder.rb index 5970d329aa3bae..7f54ffab220fb8 100644 --- a/app/finders/merge_requests_finder.rb +++ b/app/finders/merge_requests_finder.rb @@ -129,7 +129,6 @@ def sort(items) # and MergeRequests::MergeStrategies::FromTrainRef. # Lookup of Refs generated via `MergeRequests::MergeToRefService` is not currently supported # See: https://gitlab.com/gitlab-org/gitlab/-/issues/421025 - def by_generated_ref_commit(items) return items unless params[:project_id].present? && params[:commit_sha].present? -- GitLab From eb4864a72ca424e40db287677b095a2f10acefe0 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 1 Aug 2025 15:40:02 +0200 Subject: [PATCH 63/74] Fix references for merge_requests --- app/models/merge_request.rb | 6 +++++- app/models/merge_requests/generated_ref_commit.rb | 5 ++++- ee/app/models/merge_trains/car.rb | 4 ++-- ee/spec/models/merge_trains/car_spec.rb | 11 +++++------ .../merge_requests/generated_ref_commit_spec.rb | 2 +- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 92630c5496860b..1e8cec9a5de81c 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -72,7 +72,11 @@ class MergeRequest < ApplicationRecord -> { regular }, inverse_of: :merge_request has_many :merge_request_context_commits, inverse_of: :merge_request has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files - has_many :generated_ref_commits, class_name: 'MergeRequests::GeneratedRefCommit', inverse_of: :merge_request + has_many :generated_ref_commits, + primary_key: :iid, + class_name: 'MergeRequests::GeneratedRefCommit', + foreign_key: :merge_request_iid, + inverse_of: :merge_request has_one :merge_request_diff, -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request has_one :merge_head_diff, diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index c52467812f01e9..1c5d2f0dc61055 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -13,7 +13,10 @@ class GeneratedRefCommit < ApplicationRecord sha_attribute :commit_sha - belongs_to :merge_request, inverse_of: :generated_ref_commits, foreign_key: :iid + belongs_to :merge_request, + primary_key: :iid, + foreign_key: :merge_request_iid, + inverse_of: :generated_ref_commits belongs_to :project validates :commit_sha, :project, :merge_request, presence: true diff --git a/ee/app/models/merge_trains/car.rb b/ee/app/models/merge_trains/car.rb index efb56864f14ab6..e3968f529ea2e6 100644 --- a/ee/app/models/merge_trains/car.rb +++ b/ee/app/models/merge_trains/car.rb @@ -205,9 +205,9 @@ def train def cleanup_ref(async: true) # Since we don't have any hooks for GeneratedrefMergeRequestCommit, we can use delete_all here. - if Feature.enabled?(:generate_ref_commits, merge_request.target_project) && !merged? + if Feature.enabled?(:generate_ref_commits, merge_request.project) && !merged? MergeRequests::GeneratedRefCommit - .where(project: target_project, merge_request: merge_request).delete_all + .where(project: merge_request.project, merge_request: merge_request).delete_all end if async diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index c7c07360b76a7a..1198c2f8b76824 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -526,8 +526,8 @@ create_list( :merge_request_generated_ref_commit, 2, - project: train_car.target_project, - merge_request: train_car.merge_request + project: train_car.merge_request.project, + merge_request_iid: train_car.merge_request.iid ) end @@ -538,9 +538,8 @@ train_car.refresh_pipeline! train_car.start_merge! train_car.finish_merge! - generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.project, + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.merge_request.project, merge_request: train_car.merge_request) - subject expect(generated_ref_commits.count).to eq(2) end @@ -550,7 +549,7 @@ subject { train_car.try_cleanup_ref(async: false) } it 'does delete created ref merge request commits' do - generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.project, + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.merge_request.project, merge_request: train_car.merge_request) expect(generated_ref_commits.count).to eq(2) @@ -564,7 +563,7 @@ end it 'does not delete created ref merge request commits' do - generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.project, + generated_ref_commits = MergeRequests::GeneratedRefCommit.where(project: train_car.merge_request.project, merge_request: train_car.merge_request) expect(generated_ref_commits.count).to eq(2) diff --git a/spec/models/merge_requests/generated_ref_commit_spec.rb b/spec/models/merge_requests/generated_ref_commit_spec.rb index dce2a2a4d2626f..aef661aa37c110 100644 --- a/spec/models/merge_requests/generated_ref_commit_spec.rb +++ b/spec/models/merge_requests/generated_ref_commit_spec.rb @@ -35,7 +35,7 @@ end it 'is not valid without a merge_request_id' do - ref_commit.merge_request_id = nil + ref_commit.merge_request = nil expect(ref_commit).not_to be_valid end end -- GitLab From 89aa916de471b25344d797055772f34f6181e930 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 1 Aug 2025 16:59:30 +0200 Subject: [PATCH 64/74] Fix scope for generated_ref_commits due to primary key change --- app/models/merge_request.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1e8cec9a5de81c..71d168db376f61 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -73,10 +73,11 @@ class MergeRequest < ApplicationRecord has_many :merge_request_context_commits, inverse_of: :merge_request has_many :merge_request_context_commit_diff_files, through: :merge_request_context_commits, source: :diff_files has_many :generated_ref_commits, - primary_key: :iid, + primary_key: [:iid, :target_project_id], class_name: 'MergeRequests::GeneratedRefCommit', foreign_key: :merge_request_iid, - inverse_of: :merge_request + inverse_of: :merge_request, + query_constraints: [:merge_request_iid, :project_id] has_one :merge_request_diff, -> { regular.order('merge_request_diffs.id DESC') }, inverse_of: :merge_request has_one :merge_head_diff, -- GitLab From 6292511c6b4b2de4015e711bd25682b9adc31b2c Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 1 Aug 2025 17:58:57 +0200 Subject: [PATCH 65/74] Fix merge_request scope --- app/models/merge_requests/generated_ref_commit.rb | 5 +++-- ee/spec/models/merge_trains/car_spec.rb | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/models/merge_requests/generated_ref_commit.rb b/app/models/merge_requests/generated_ref_commit.rb index 1c5d2f0dc61055..def0ba79a38b71 100644 --- a/app/models/merge_requests/generated_ref_commit.rb +++ b/app/models/merge_requests/generated_ref_commit.rb @@ -14,9 +14,10 @@ class GeneratedRefCommit < ApplicationRecord sha_attribute :commit_sha belongs_to :merge_request, - primary_key: :iid, + primary_key: [:iid, :target_project_id], foreign_key: :merge_request_iid, - inverse_of: :generated_ref_commits + inverse_of: :generated_ref_commits, + query_constraints: [:merge_request_iid, :project_id] belongs_to :project validates :commit_sha, :project, :merge_request, presence: true diff --git a/ee/spec/models/merge_trains/car_spec.rb b/ee/spec/models/merge_trains/car_spec.rb index 1198c2f8b76824..f25a08380b5829 100644 --- a/ee/spec/models/merge_trains/car_spec.rb +++ b/ee/spec/models/merge_trains/car_spec.rb @@ -518,7 +518,7 @@ end describe '#try_cleanup_ref' do - let(:train_car) { create(:merge_train_car) } + let(:train_car) { create(:merge_train_car, target_project: project) } let(:generated_ref_commits) { class_double(MergeRequests::GeneratedRefCommit) } context 'when we deal with generated ref commits' do @@ -527,7 +527,7 @@ :merge_request_generated_ref_commit, 2, project: train_car.merge_request.project, - merge_request_iid: train_car.merge_request.iid + merge_request: train_car.merge_request ) end -- GitLab From f8497b5edc85e447aba2c944383331aa4bb09bcc Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Mon, 4 Aug 2025 09:51:41 +0200 Subject: [PATCH 66/74] Add limit of 500 for generated ref commits --- .../merge_requests/create_ref_service.rb | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 77885864e7e977..6a0eb0a1149ee9 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -58,37 +58,35 @@ def execute delegate :repository, to: :target_project def store_generated_ref_commits(final_commit_sha) - commit_shas = commit_shas_between_refs(final_commit_sha) + commit_shas = commit_shas_between_refs(final_commit_sha, limit: 500) return unless commit_shas.any? && target_project.can_create_new_ref_commits? GeneratedRefCommit.transaction do - commit_shas.each_slice(1000) do |commit_sha_batch| - # Prepare records for bulk insert - records = commit_sha_batch.map do |commit_sha| - { - merge_request_iid: merge_request.iid, - commit_sha: commit_sha, - project_id: merge_request.project_id, - created_at: Time.current, - updated_at: Time.current - } - end - GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) + # Prepare records for bulk insert + records = commit_shas.map do |commit_sha| + { + merge_request_iid: merge_request.iid, + commit_sha: commit_sha, + project_id: merge_request.project_id, + created_at: Time.current, + updated_at: Time.current + } end + GeneratedRefCommit.upsert_all(records, unique_by: [:id, :project_id]) end rescue ::PG::Error => e # Log error but don't break the main flow Gitlab::AppLogger.error("Failed to store generated ref commits for MR #{merge_request.id}: #{e.message}") end - def commits_between_refs(final_commit_sha) + def commits_between_refs(final_commit_sha, limit: nil) return [] unless final_commit_sha && first_parent_sha - repository.commits_between(first_parent_sha, final_commit_sha) + repository.commits_between(first_parent_sha, final_commit_sha, limit: limit) end - def commit_shas_between_refs(final_commit_sha) - commits_between_refs(final_commit_sha).map(&:sha) + def commit_shas_between_refs(final_commit_sha, limit: nil) + commits_between_refs(final_commit_sha, limit: limit).map(&:sha) end def maybe_squash!(commit_sha:, **rest) -- GitLab From a1fd0462e1fac14847dadf539df8291084eab7e2 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 5 Aug 2025 12:51:10 +0200 Subject: [PATCH 67/74] Wrap gitaly operation in safe_gitaly_operation --- app/services/merge_requests/create_ref_service.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 6a0eb0a1149ee9..b4d57fb1012bd5 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -82,7 +82,9 @@ def store_generated_ref_commits(final_commit_sha) def commits_between_refs(final_commit_sha, limit: nil) return [] unless final_commit_sha && first_parent_sha - repository.commits_between(first_parent_sha, final_commit_sha, limit: limit) + safe_gitaly_operation do + repository.commits_between(first_parent_sha, final_commit_sha, limit: limit) + end end def commit_shas_between_refs(final_commit_sha, limit: nil) -- GitLab From 6f1edf3d1062409b1a24c4cf2cb1b5e7c9620e33 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Tue, 5 Aug 2025 13:31:40 +0200 Subject: [PATCH 68/74] Fix structure.sql default value --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index b52e5d2895fe4f..4bb0581f900227 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -25303,7 +25303,7 @@ CREATE TABLE user_preferences ( organization_groups_projects_display smallint DEFAULT 1 NOT NULL, dpop_enabled boolean DEFAULT false NOT NULL, use_work_items_view boolean DEFAULT false NOT NULL, - text_editor_type smallint DEFAULT 0 NOT NULL, + text_editor_type smallint DEFAULT 2 NOT NULL, merge_request_dashboard_list_type smallint DEFAULT 0 NOT NULL, extensions_marketplace_opt_in_url text, dark_color_scheme_id smallint, -- GitLab From 5357951b820863a7ff0d3fe1c130b7e74d7e1065 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 7 Aug 2025 11:31:11 +0200 Subject: [PATCH 69/74] Fix specs for non ee pipeline --- app/services/merge_requests/create_ref_service.rb | 5 ++++- spec/services/merge_requests/create_ref_service_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index b4d57fb1012bd5..9a213dbad0c99a 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,7 +39,10 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) - if Feature.enabled?(:generate_ref_commits, merge_request.target_project) && merge_request.merge_train_car.present? + # Since we are only fixing this for now for merge trains and since merge trains are an ee feature, + # we need to check, if we are on ee here. + if Gitlab.ee? && (Feature.enabled?(:generate_ref_commits, + merge_request.target_project) && merge_request.merge_train_car.present?) # Store commits in generated_ref_commits table using the final commit_sha store_generated_ref_commits(result[:commit_sha]) end diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index 2f14f2f05bda40..29f9d36ff7b781 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -92,7 +92,7 @@ ) end shared_examples 'generate ref merge request commits' do |commit_count| - context 'when coming from a merge train' do + context 'when coming from a merge train', if: Gitlab.ee? do let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } it 'created generated ref merge request commits' do @@ -259,7 +259,7 @@ end end - context 'when a database error occurs' do + context 'when a database error occurs', if: Gitlab.ee? do let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } before do -- GitLab From 4ecf45c23aae0b104c85678f41c96c8fa6d7dca8 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 7 Aug 2025 18:04:54 +0200 Subject: [PATCH 70/74] Add ee module for create_ref_service --- .../merge_requests/create_ref_service.rb | 22 ++- .../ee/merge_requests/create_ref_service.rb | 16 ++ .../merge_requests/create_ref_service_spec.rb | 167 ++++++++++++++++++ .../merge_requests/create_ref_service_spec.rb | 52 ------ 4 files changed, 197 insertions(+), 60 deletions(-) create mode 100644 ee/app/services/ee/merge_requests/create_ref_service.rb create mode 100644 ee/spec/services/ee/merge_requests/create_ref_service_spec.rb diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 9a213dbad0c99a..18d76348cc6a34 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -39,13 +39,8 @@ def execute result = maybe_rebase!(**result) result = maybe_merge!(**result) - # Since we are only fixing this for now for merge trains and since merge trains are an ee feature, - # we need to check, if we are on ee here. - if Gitlab.ee? && (Feature.enabled?(:generate_ref_commits, - merge_request.target_project) && merge_request.merge_train_car.present?) - # Store commits in generated_ref_commits table using the final commit_sha - store_generated_ref_commits(result[:commit_sha]) - end + # Store generated ref commits if conditions are met + store_generated_ref_commits_if_needed(result[:commit_sha]) ServiceResponse.success(payload: result) rescue CreateRefError => error @@ -54,6 +49,17 @@ def execute private + def store_generated_ref_commits_if_needed(final_commit_sha) + return unless should_store_generated_ref_commits? + + store_generated_ref_commits(final_commit_sha) + end + + # Default CE implementation - can be overridden in EE + def should_store_generated_ref_commits? + false # only available in ee for merge trains for now + end + attr_reader :current_user, :merge_request, :target_ref, :first_parent_ref, :first_parent_sha, :source_sha, :merge_params @@ -62,7 +68,7 @@ def execute def store_generated_ref_commits(final_commit_sha) commit_shas = commit_shas_between_refs(final_commit_sha, limit: 500) - return unless commit_shas.any? && target_project.can_create_new_ref_commits? + return unless commit_shas.any? GeneratedRefCommit.transaction do # Prepare records for bulk insert diff --git a/ee/app/services/ee/merge_requests/create_ref_service.rb b/ee/app/services/ee/merge_requests/create_ref_service.rb new file mode 100644 index 00000000000000..981ae678686c26 --- /dev/null +++ b/ee/app/services/ee/merge_requests/create_ref_service.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module EE + module MergeRequests + module CreateRefService + extend ::Gitlab::Utils::Override + + override :should_store_generated_ref_commits? + + def should_store_generated_ref_commits? + ::Feature.enabled?(:generate_ref_commits, merge_request.target_project) && + target_project.can_create_new_ref_commits? && merge_request.merge_train_car.present? + end + end + end +end diff --git a/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb b/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb new file mode 100644 index 00000000000000..f884efd4c03351 --- /dev/null +++ b/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb @@ -0,0 +1,167 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::CreateRefService, feature_category: :merge_trains do + using RSpec::Parameterized::TableSyntax + + describe '#execute' do + let_it_be_with_reload(:project) { create(:project, :empty_repo) } + let_it_be(:user) { project.creator } + let_it_be(:first_parent_ref) { project.default_branch_or_main } + let_it_be(:source_branch) { 'branch' } + let(:target_ref) { "refs/merge-requests/#{merge_request.iid}/train" } + let(:source_sha) { project.commit(source_branch).sha } + let(:squash) { false } + let(:default_commit_message) { merge_request.default_merge_commit_message(user: user) } + let(:expected_commit_message) { "#{merge_request.title}\n" } + let(:merge_params) { {} } + + let(:merge_request) do + create( + :merge_request, + title: 'Merge request ref test', + author: user, + source_project: project, + target_project: project, + source_branch: source_branch, + target_branch: first_parent_ref, + squash: squash + ) + end + + let(:service) do + described_class.new( + current_user: user, + merge_request: merge_request, + target_ref: target_ref, + source_sha: source_sha, + first_parent_ref: first_parent_ref, + merge_params: merge_params + ) + end + + subject(:result) do + service.execute + end + + context 'with valid inputs' do + before_all do + # ensure first_parent_ref is created before source_sha + project.repository.create_file( + user, + 'README.md', + '', + message: 'Base parent commit 1', + branch_name: first_parent_ref + ) + project.repository.create_branch(source_branch, first_parent_ref) + + # create two commits source_branch to test squashing + project.repository.create_file( + user, + '.gitlab-ci.yml', + '', + message: 'Feature branch commit 1', + branch_name: source_branch + ) + + project.repository.create_file( + user, + '.gitignore', + '', + message: 'Feature branch commit 2', + branch_name: source_branch + ) + + # create an extra commit not present on source_branch + project.repository.create_file( + user, + 'EXTRA', + '', + message: 'Base parent commit 2', + branch_name: first_parent_ref + ) + end + shared_examples 'generate ref merge request commits' do |commit_count| + context 'when coming from a merge train', if: Gitlab.ee? do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + it 'created generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_iid: merge_request.iid)) + .to be_empty + + expect(MergeRequests::GeneratedRefCommit.where(project: project, + merge_request_iid: merge_request.iid).count).to eq( + commit_count + ) + end + + context 'when ff for generate ref commits is disabled' do + before do + stub_feature_flags(generate_ref_commits: false) + end + + include_examples 'does not generate ref merge request commits' + end + end + end + + shared_examples 'does not generate ref merge request commits' do + it 'does not create generated ref merge request commits' do + result + + expect(MergeRequests::GeneratedRefCommit.exists?).to be false + end + end + + context 'when a database error occurs' do + let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } + + before do + project.merge_method = :rebase_merge + project.save! + allow(project).to receive(:can_create_new_ref_commits?).and_return(true) + end + + context 'when pg error' do + before do + allow(MergeRequests::GeneratedRefCommit).to receive(:upsert_all).and_raise(PG::Error) + end + + it 'rescues and logs PG::Error' do + expect(Gitlab::AppLogger).to receive(:error).with( + a_string_including("Failed to store generated ref commits") + ) + + result + end + end + end + + context 'when merged commit strategy' do + include_examples 'does not generate ref merge request commits' + end + + context 'when semi-linear merge strategy' do + before do + project.merge_method = :rebase_merge + project.save! + end + + include_examples 'generate ref merge request commits', 3 + end + + context 'when fast-forward merge strategy' do + before do + project.merge_method = :ff + project.save! + end + + it_behaves_like 'generate ref merge request commits', 2 + end + end + end +end diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index 29f9d36ff7b781..e0c665688f15e7 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -91,31 +91,6 @@ branch_name: first_parent_ref ) end - shared_examples 'generate ref merge request commits' do |commit_count| - context 'when coming from a merge train', if: Gitlab.ee? do - let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } - - it 'created generated ref merge request commits' do - result - - expect(MergeRequests::GeneratedRefCommit.where.not(project: project, merge_request_iid: merge_request.iid)) - .to be_empty - - expect(MergeRequests::GeneratedRefCommit.where(project: project, - merge_request_iid: merge_request.iid).count).to eq( - commit_count - ) - end - - context 'when ff for generate ref commits is disabled' do - before do - stub_feature_flags(generate_ref_commits: false) - end - - include_examples 'does not generate ref merge request commits' - end - end - end shared_examples 'does not generate ref merge request commits' do it 'does not create generated ref merge request commits' do @@ -259,30 +234,6 @@ end end - context 'when a database error occurs', if: Gitlab.ee? do - let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } - - before do - project.merge_method = :rebase_merge - project.save! - allow(project).to receive(:can_create_new_ref_commits?).and_return(true) - end - - context 'when pg error' do - before do - allow(MergeRequests::GeneratedRefCommit).to receive(:upsert_all).and_raise(PG::Error) - end - - it 'rescues and logs PG::Error' do - expect(Gitlab::AppLogger).to receive(:error).with( - a_string_including("Failed to store generated ref commits") - ) - - result - end - end - end - context 'when the merge commit message is provided at time of merge' do let(:expected_merge_commit) { 'something custom' } let(:extra_mr_merge_params) { {} } @@ -327,7 +278,6 @@ context 'when merged commit strategy' do include_examples 'merge commits without squash' include_examples 'merge commits with squash' - include_examples 'does not generate ref merge request commits' end context 'when semi-linear merge strategy' do @@ -338,7 +288,6 @@ include_examples 'merge commits without squash' include_examples 'merge commits with squash' - include_examples 'generate ref merge request commits', 3 context 'when the target ref changes between rebase and merge' do # this tests internal handling of expected_old_oid @@ -370,7 +319,6 @@ end it_behaves_like 'writing without a merge commit' - it_behaves_like 'generate ref merge request commits', 2 context 'when squash set' do let(:squash) { true } -- GitLab From 36d140710aa50c14685eb48ff799defea3cd5c17 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 7 Aug 2025 18:31:23 +0200 Subject: [PATCH 71/74] Remove superfluous Gitlab.ee? --- ee/spec/services/ee/merge_requests/create_ref_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb b/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb index f884efd4c03351..e53efbb0995048 100644 --- a/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/create_ref_service_spec.rb @@ -84,7 +84,7 @@ ) end shared_examples 'generate ref merge request commits' do |commit_count| - context 'when coming from a merge train', if: Gitlab.ee? do + context 'when coming from a merge train' do let!(:train_car) { create(:merge_train_car, merge_request: merge_request) } it 'created generated ref merge request commits' do -- GitLab From 9e3fcf234380be477eb0dcb9c5890653db5b5ba2 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 7 Aug 2025 20:40:36 +0200 Subject: [PATCH 72/74] Add missing spec --- spec/services/merge_requests/create_ref_service_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/services/merge_requests/create_ref_service_spec.rb b/spec/services/merge_requests/create_ref_service_spec.rb index e0c665688f15e7..05a1410bf7fc59 100644 --- a/spec/services/merge_requests/create_ref_service_spec.rb +++ b/spec/services/merge_requests/create_ref_service_spec.rb @@ -341,6 +341,10 @@ end end end + + context 'when we are not on ee' do + include_examples 'does not generate ref merge request commits' + end end end end -- GitLab From e2e35d0dc3ad5b222a681ab5ecd9b933c354892e Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Thu, 7 Aug 2025 22:23:56 +0200 Subject: [PATCH 73/74] Exclude false negative from undercoverage --- app/services/merge_requests/create_ref_service.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 18d76348cc6a34..87a8bb55d37877 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -56,9 +56,13 @@ def store_generated_ref_commits_if_needed(final_commit_sha) end # Default CE implementation - can be overridden in EE + # rubocop:disable Gitlab/NoCodeCoverageComment -- Fully tested in Foss, overridden in EE + # :nocov: def should_store_generated_ref_commits? false # only available in ee for merge trains for now end + # :nocov: + # rubocop:enable Gitlab/NoCodeCoverageComment attr_reader :current_user, :merge_request, :target_ref, :first_parent_ref, :first_parent_sha, :source_sha, :merge_params -- GitLab From b48c90366dd29e098f33a316e1c0e487d53bef70 Mon Sep 17 00:00:00 2001 From: Daniel Prause Date: Fri, 8 Aug 2025 00:48:58 +0200 Subject: [PATCH 74/74] Attempt to fix undercoverage --- app/services/merge_requests/create_ref_service.rb | 4 ---- ee/app/services/ee/merge_requests/create_ref_service.rb | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/app/services/merge_requests/create_ref_service.rb b/app/services/merge_requests/create_ref_service.rb index 87a8bb55d37877..18d76348cc6a34 100644 --- a/app/services/merge_requests/create_ref_service.rb +++ b/app/services/merge_requests/create_ref_service.rb @@ -56,13 +56,9 @@ def store_generated_ref_commits_if_needed(final_commit_sha) end # Default CE implementation - can be overridden in EE - # rubocop:disable Gitlab/NoCodeCoverageComment -- Fully tested in Foss, overridden in EE - # :nocov: def should_store_generated_ref_commits? false # only available in ee for merge trains for now end - # :nocov: - # rubocop:enable Gitlab/NoCodeCoverageComment attr_reader :current_user, :merge_request, :target_ref, :first_parent_ref, :first_parent_sha, :source_sha, :merge_params diff --git a/ee/app/services/ee/merge_requests/create_ref_service.rb b/ee/app/services/ee/merge_requests/create_ref_service.rb index 981ae678686c26..4977cf5ab4370b 100644 --- a/ee/app/services/ee/merge_requests/create_ref_service.rb +++ b/ee/app/services/ee/merge_requests/create_ref_service.rb @@ -8,8 +8,8 @@ module CreateRefService override :should_store_generated_ref_commits? def should_store_generated_ref_commits? - ::Feature.enabled?(:generate_ref_commits, merge_request.target_project) && - target_project.can_create_new_ref_commits? && merge_request.merge_train_car.present? + super || (::Feature.enabled?(:generate_ref_commits, merge_request.target_project) && + target_project.can_create_new_ref_commits? && merge_request.merge_train_car.present?) end end end -- GitLab