Skip to content

ActiveRecord::RecordInvalid: Validation failed: Name has already been taken

Sentry Issue: GITLABCOM-MQ5

ActiveRecord::RecordInvalid: Validation failed: Name has already been taken
  lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `public_send'
    connection.public_send(...)
  lib/gitlab/database/load_balancing/connection_proxy.rb:127:in `block in write_using_load_balancer'
    connection.public_send(...)
  lib/gitlab/database/load_balancing/load_balancer.rb:127:in `block in read_write'
    yield connection
  lib/gitlab/database/load_balancing/load_balancer.rb:205:in `retry_with_backoff'
    return yield attempt # Yield the current attempt count
  lib/gitlab/database/load_balancing/load_balancer.rb:116:in `read_write'
    retry_with_backoff(attempts: attempts) do |attempt|
...
(156 additional frame(s) were not displayed)

Root cause

Race condition between process_scan_result_policy_worker and sync_opened_merge_request_worker running in parallel when the feature flag sync_mr_approval_rules_security_policies is enabled.

The error was described and reproduced here(internal link). The error results in an invalid record being created.

Implementation Plan

Short term:

Update ApprovalProjectRule#apply_report_approver_rules_to to validate if approval_project_rules instance is still valid when a approval_merge_request_rules is created.

POC MR: !123691 (closed) (diffs).

This implementation plan is related to a short-term solution to fix the bug.

A long-term solution will be implemented in a follow-up typemaintenance issue.

Long term: as suggested here

Stop using approval_merge_request_rule_sources and move approval_project_rule_id column to approval_merge_request_rules table with a foreign key relation for approval_project_rules table. Having this foreign_key will catch the invalid record being created in approval_merge_request_rules. Can create a follow up issue of type

  1. Add approval_project_rule_id column to approval_merge_request_rules table
class AddApprovalProjectRuleIdToApprovalMergeRequestRules
  def up
    add_column :approval_merge_request_rules, :approval_project_rule_id, :bigint
  end

  def down
    remove_column :approval_merge_request_rules, :approval_project_rule_id, :bigint
  end
end

Add index and foreign_key

class AddApprovalProjectRuleIdIndexesForeignKeyToApprovalMergeRequestRules < Gitlab::Database::Migration[2.0]
  disable_ddl_transaction!

  INDEX_NAME = 'index_approval_merge_request_rules_approval_project_rule_id'

  def up
    add_concurrent_index :approval_merge_request_rules, :approval_project_rule_id, name: INDEX_NAME
    add_concurrent_foreign_key :approval_merge_request_rules, :approval_project_rules, column: :approval_project_rule_id
  end

  def down
    with_lock_retries do
      remove_foreign_key :approval_merge_request_rules, column: :approval_project_rule_id
    end
    remove_concurrent_index :approval_merge_request_rules, :approval_project_rule_id, name: INDEX_NAME
  end
end
  1. Move the data in approval_project_rule_id from approval_merge_request_rule_sources to the new column
UPDATE
    approval_merge_request_rules
SET
    approval_project_rule_id = (
        SELECT
            approval_project_rule_id
        FROM
            approval_merge_request_rule_sources
        WHERE
            approval_merge_request_rule_sources.approval_merge_request_rule_id = approval_merge_request_rules.id)
  1. Drop approval_merge_request_rule_sources

This step should be done after the application no longer uses the table.

This table has foreign keys. We should the Table has foreign keys: steps as described in our handbook.

  • Remove the application code related to the table, such as models, controllers, and services.

  • Create a post-deployment migration to drop the table

# first migration file
class RemovingForeignKeysApprovalMergeRequestRuleSources < Gitlab::Database::Migration[2.1]
  disable_ddl_transaction!

  def up
    with_lock_retries do
      remove_foreign_key :approval_merge_request_rule_sources, :approval_project_rules
      remove_foreign_key :approval_merge_request_rule_sources, :approval_merge_request_rules
    end
  end

  def down
    add_concurrent_foreign_key :approval_merge_request_rule_sources, :approval_project_rules, column: approval_project_rule_id
    add_concurrent_foreign_key :approval_merge_request_rule_sources, :approval_merge_request_rules, column: approval_merge_request_rule_id
  end
end
# second migration file
class DroppingTableApprovalMergeRequestRuleSources < Gitlab::Database::Migration[2.1]
  def up
    drop_table :approval_merge_request_rule_sources
  end

  def down
    # create_table with the same schema but without the removed foreign key ...
  end
end
  • Add the approval_merge_request_rule_sources table to db/docs/deleted_tables as described here.

Feature flag

report_approver_rules

Verification steps

Check for ActiveRecord::RecordInvalid in SyncOpenedMergeRequestsWorker jobs.

Log query(Internal only):

https://log.gprd.gitlab.net/goto/93f919e0-1d02-11ee-a017-0d32180b1390

Edited by Marcos Rocha