Skip to content

Remove PG warning with with_lock_retries

Adam Hegyi requested to merge remove-with-lock_retries-warning into master

What does this MR do?

Remove PG warning with with_lock_retries.

This change fixes the warning message when running a migration with disable_ddl_transaction!. This MR doesn't alter the behavior of with_lock_retries, it just makes the warning go away.

WARNING: SET LOCAL can only be used in transaction blocks

More context:

When using with_lock_retries, the given block might be executed several times with sleep between the attempts (waiting for other processes to release locks).

We are using ruby's sleep method for delaying the execution. This was a problem when running the code in production because the idle transaction timeout value is configured which caused aborted transactions (failed migration). (already fixed)

The solution was to temporarily disable idle transaction timeout and then re-enable it after the nested transaction block.

There is an edge case: when the migration is executed with disable_ddl_transaction!, then the migration is not wrapped with a transaction block, thus disabling the idle transaction timeout is not necessary (we're still sleep-ing, but not in a transaction -> cause of the warning message).

How to reproduce it:

On master branch:

  1. Open a PSQL session.
  2. Create a migration file with disable_ddl_transaction! + with_lock_retries.
  3. Run the migration.
  4. When it says: "LOCK NOW", go to the PSQL session and execute:
  begin;
  LOCK TABLE projects IN ACCESS EXCLUSIVE MODE;
  # leave it
  1. After a few seconds, you should see warning messages in your migration.

Doing the same thing on this branch should not print any warning.

Example migration file:

class TestMigration < ActiveRecord::Migration[6.0]
  include Gitlab::Database::MigrationHelpers

  DOWNTIME = false

  disable_ddl_transaction!

  def up
    puts "LOCK NOW"
    sleep 20

    with_lock_retries do
      Project.first.touch
    end
  end

  def down
  end
end

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Adam Hegyi

Merge request reports