disable_statement_timeout can leak to other migrations
Summary
In the Migrations Helpers, we have a method to disable statement timeouts .disable_statement_timeout
https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/database/migration_helpers.rb#L227-229
As discussed here: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20413#note_86515707 it works for the whole connection, not only for the specific migration.
So if any migration disables the statement, it will reflect on the following migrations next to it. This is usually mitigated by the fact that you are only executing a fraction of the total migrations in a live system, and after the execution finishes, the connection is dropped and you get the statement timeout to reset again, so it is very short lived.
This is more problematic in the post migrations, as from my understanding they are executed inside sidekiq, and so the .disable_statement_timeout
will leak to the next job executed in sidekiq that previously had the migration on it, as the connection is shared and longlived.
This can possibly leak as well to the pgbouncer
, as the connection gets returned back to the pool, the state is probably preserved.
Possible fixes
Change statement to be SET LOCAL statement_timeout = 0;
instead or make it optional by transaction_only: true
:
def disable_statement_timeout(transaction_only: false)
if transaction_only
execute('SET LOCAL statement_timeout TO 0') if Database.postgresql?
else
execute('SET statement_timeout TO 0') if Database.postgresql?
end
end
Caveat is that if in a few migrations we use this: disable_ddl_transaction! to remove the enclosing transaction, which makes it behave as today again.
Related Issues / MRs
disable_statement_timeout
was introduced by: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5263