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')yieldconnection.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
withreads the current value withSHOW, applies the new one withSET, and writes the old value back withSETin anensure. It never usesRESET.withruns the block underuse_primaryfor the connection's load balancer. TheSETlands 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.withis the only public method. A bareSETwithout the restore would stay on the pooled connection.withraisesTransactionOpenErrorinside a transaction. A session-levelSETwould survive theCOMMIT, and aSET LOCALvalue read back bySHOWwould be persisted on the restore. Inside a transaction, useTransaction::Settings.
New helper Session::Settings::StatementTimeout.with(connection, timeout)
- Sets
statement_timeoutto the given seconds for the block and restores it afterwards. 0 disables it. - It changes only
statement_timeout. The sequence sync keeps liftingtransaction_timeoutitself throughGitlab::Database::TransactionTimeout, as it does onmaster.
Other changes
lib/gitlab/database/sync_sequences_with_table_data.rbnow uses the helper.- The sequence sync spec checks the value is restored after success and after an exception.
References
- Work item: #630538 (closed)
- Earlier inline fix: !257270 (merged)
- Replaced draft: !258188 (closed)
How to set up and validate locally
- Open a Rails console with
bin/rails console. - Run
ApplicationRecord.connection.select_value('SHOW statement_timeout'). It shows the value fromconfig/database.yml, for example2min. - Run
Gitlab::Database::Session::Settings::StatementTimeout.with(ApplicationRecord.connection, 0) { puts ApplicationRecord.connection.select_value('SHOW statement_timeout') }. It prints0. - Run the
SHOW statement_timeoutquery again. It shows the original value, not0.
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.