`retry_lock` should not change `current_scope` for the subject's ActiveRecord class

Summary

From this failing EE spec https://gitlab.com/gitlab-org/gitlab/-/jobs/3671363906, a bug is found which has changed the scope for Ci::Build.all to:

(byebug) ::Ci::Build.all.to_sql
"SELECT \"ci_builds\".* FROM \"ci_builds\" WHERE \"ci_builds\".\"type\" = 'Ci::Build' AND \"ci_builds\".\"commit_id\" = 284 AND \"ci_builds\".\"status\" IN ('waiting_for_resource', 'preparing', 'pending', 'running', 'created', 'scheduled')"

What needs to be done:

• A Merge Request to fix the described bug by using the potential solution from below Possible fixes
• A follow-up Merge Request to remove the .unscoped (temporary fix for this bug) from file lib/gitlab/ci/pipeline/duration.rb

Steps to reproduce

Here is the code to run in Rails console to reproduce this bug:

class Demo
  include Gitlab::OptimisticLocking

  def reproduce
    pipeline = Ci::Pipeline.first
    relation = pipeline.cancelable_statuses

    puts "-> before: CommitStatus.current_scope:", CommitStatus.current_scope&.to_sql.inspect
    puts "-> before: Ci::Build.current_scope:", Ci::Build.current_scope&.to_sql.inspect

    retry_lock(relation, name: :bug) do
      puts "=> locking: CommitStatus.current_scope:", CommitStatus.current_scope&.to_sql.inspect
      puts "=> locking: Ci::Build.current_scope:", Ci::Build.current_scope&.to_sql.inspect
    end

    puts "-> after: CommitStatus.current_scope:", CommitStatus.current_scope&.to_sql.inspect
    puts "-> after: Ci::Build.current_scope:", Ci::Build.current_scope&.to_sql.inspect
  end
end

Demo.new.reproduce

# -> before: CommitStatus.current_scope:
# nil
# -> before: Ci::Build.current_scope:
# nil
# => locking: CommitStatus.current_scope:
# "SELECT \"ci_builds\".* FROM \"ci_builds\" WHERE \"ci_builds\".\"commit_id\" = 1 AND \"ci_builds\".\"status\" IN ('waiting_for_resource', 'preparing', 'pending', 'running', 'created', 'scheduled')"
# => locking: Ci::Build.current_scope:
# "SELECT \"ci_builds\".* FROM \"ci_builds\" WHERE \"ci_builds\".\"commit_id\" = 1 AND \"ci_builds\".\"status\" IN ('waiting_for_resource', 'preparing', 'pending', 'running', 'created', 'scheduled')"
# -> after: CommitStatus.current_scope:
# nil
# -> after: Ci::Build.current_scope:
# nil

Example Project

What is the current bug behavior?

As the demo from above, the ActiveRecord relation (e.g. pipeline.cancelable_statuses) passed to Gitlab::OptimisticLocking#retry_lock modifies the current_scope for the relation's ActiveRecord class (e.g. CommitStatus).

This is what happens:

• Gitlab::OptimisticLocking#retry_lock calls transaction on the subject it receives
• if the subject is an ActiveRecord relation instance, it calls scoping in the generated transaction method from this line
• the scoping method will then temporarily swap the current_scope for the transaction block from the internal method _scoping at this line for the current ActiveRecord class (in our case, CommitStatus)
• because Ci::Build inherits from CommitStatus, its current_scope is also changed.

What is the expected correct behavior?

When an ActiveRecord relation is passed to Gitlab::OptimisticLocking#retry_lock, retry_lock should not change the current_scope for the relation's ActiveRecord class.

Relevant logs and/or screenshots

Output of checks

Results of GitLab environment info

Expand for output related to GitLab environment info

(For installations with omnibus-gitlab package run and paste the output of:
`sudo gitlab-rake gitlab:env:info`)

(For installations from source run and paste the output of:
`sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)

Results of GitLab application Check

Expand for output related to the GitLab application check

(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true)
(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true)
(we will only investigate if the tests are passing)

Possible fixes

Instead of calling transaction on subject, retry_lock method should call transaction method on the subject's ActiveRecord class:

• for an ActiveRecord relation instance, the class should be subject.klass
• for an ActiveRecord instance, the class should be subject.class