Restore statement_timeout through Session::Settings instead of RESET

What does this MR do and why?

In !257270 (merged), a fix was merged. The code in lib/gitlab/database/sync_sequences_with_table_data.rb was doing this:

  • connection.execute('SET statement_timeout TO 0')
  • yield
  • connection.execute('RESET statement_timeout')

PostgreSQL RESET resets a configuration value to the default, which is what is configured in the cluster level configuration (postgresql.conf).

However, it should reset to the value that is configured on the application level (config/database.yaml)

This logic be:

  • Remember current statement_timeout
  • connection.execute('SET statement_timeout TO 0')
  • yield
  • Restore remembered statement_timeout

This was addressed for lib/gitlab/database/sync_sequences_with_table_data.rb but since this pattern is used in more places, we should have a class that can handle this.

This MR adds Gitlab::Database::Session::Settings, a sibling of the existing Gitlab::Database::Transaction::Settings, so we can re-use this pattern in other places.

Implementation

Two classes, one per scope:

Class Statement Scope Allowlist
Gitlab::Database::Transaction::Settings (unchanged) SET LOCAL current transaction LOCK_TIMEOUT
Gitlab::Database::Session::Settings (new) SET whole session statement_timeout

To change another setting, add it to the allowlist of the class with the scope you need.

Gitlab::Database::Session::Settings

  • with reads the current value with SHOW, applies the new one with SET, and writes the old value back with SET in an ensure. It never uses RESET.
  • with runs the block under use_primary for the connection's load balancer. The SET lands on the primary, so without this a relation read inside the block could run on a replica that never saw the new value. A raw adapter connection has no load balancer and is used as is.
  • with is the only public method. A bare SET without the restore would stay on the pooled connection.
  • with raises TransactionOpenError inside a transaction. A session-level SET would survive the COMMIT, and a SET LOCAL value read back by SHOW would be persisted on the restore. Inside a transaction, use Transaction::Settings.

New helper Session::Settings::StatementTimeout.with(connection, timeout)

  • Sets statement_timeout to the given seconds for the block and restores it afterwards. 0 disables it.
  • It changes only statement_timeout. The sequence sync keeps lifting transaction_timeout itself through Gitlab::Database::TransactionTimeout, as it does on master.

Other changes

  • lib/gitlab/database/sync_sequences_with_table_data.rb now uses the helper.
  • The sequence sync spec checks the value is restored after success and after an exception.

References

How to set up and validate locally

  1. Open a Rails console with bin/rails console.
  2. Run ApplicationRecord.connection.select_value('SHOW statement_timeout'). It shows the value from config/database.yml, for example 2min.
  3. Run Gitlab::Database::Session::Settings::StatementTimeout.with(ApplicationRecord.connection, 0) { puts ApplicationRecord.connection.select_value('SHOW statement_timeout') }. It prints 0.
  4. Run the SHOW statement_timeout query again. It shows the original value, not 0.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Rutger Wessels

Merge request reports

Loading
Loading