Resolve "Remove double locking as the underlying codebase has changed"

What does this MR do and why?

This MR introduces a new feature flag that handles the double locking regarding the skipping of transactional checks.

Problem

The overall problem is that the service is trying to acquire ExclusiveLease locks while running within the context of a transaction.

This problem appears in 2 places in the code:

  1. In app/models/ci/build.rb, the service is called during the state transition, which always runs in the context of a transaction
  2. In app/models/environment.rb, Gitlab::OptimisticLocking.retry_lock wraps the service call within a transaction, so we needed to remove the call from this wrapper. We received approval from the code-owners and added a feature flag to ensure a safe transition.

Solution

We put the problematic code block inside a run_after_commit hook and let it run asynchronously.

In addition, we remove the skipping_transaction_check method since our solution makes it superfluous.

Therefore, we changed environment.rb and hid the removal of the "old" way of acquiring an optimistic lock behind a new feature flag called no_locking_for_stop_actions since we execute the stopping/retrying of a job inside a newly introduced run_after_commit hook.

See: !166146 (closed)

Related to #492409 (closed)

Edited by Daniel Prause

Merge request reports

Loading