Restore session timeouts after disable_statement_timeout (!263330) · Merge requests · GitLab.org / GitLab

What does this MR do and why?

The block form of disable_statement_timeout ends with RESET statement_timeout and RESET transaction_timeout (timeout_helpers.rb:34, :65). A reset returns the session to the server default, so the statement_timeout that database.yml sets (database.yml.postgresql:48-49) and any session transaction_timeout are gone after the first block.

This builds the nested shape @rutgerwessels sketched on the work item. The helper wraps Session::Settings::StatementTimeout.with around a new Session::Settings::TransactionTimeout.with, so each block puts back the value it read, including when the block raises.

Session::Settings::TransactionTimeout mirrors statement_timeout.rb. On PostgreSQL 16 it yields untouched, because transaction_timeout does not exist there (transaction_timeout.rb:9-14).
transaction_timeout joins Settings::ALLOWED_CONFIGS (settings.rb:9-11).
SyncSequencesWithTableData uses the same pair in place of its hand-rolled save and restore (sync_sequences_with_table_data.rb:201-210).
The blockless form is unchanged. with_transaction_timeout_disabled, statement_timeout_disabled? and transaction_timeout_disabled? go, because the nested with calls restore their own values.

With a transaction open, the block form uses SET LOCAL

Settings.with raises TransactionOpenError when a transaction is open, a deliberate choice by @rutgerwessels to make the choice between SET and SET LOCAL explicit. @dgruzd described the failure it prevents: a session-level restore inside a transaction can leave a value behind after COMMIT, and after a database error it replaces the original exception with PG::InFailedSqlTransaction. The same note proposed SET LOCAL when connection.transaction_open?, which is the branch this MR adds in the helper.

The block form cannot raise there, though. validate_foreign_key and the other validate_* helpers call the block with no transaction guard, and 17 post-deploy migrations call them without disable_ddl_transaction!, such as ValidateGroupWikiRepositoriesGroupIdFk. Their block runs inside the migration transaction today, so a raise would break an upgrade that replays them.

So the helper asks the same question Settings.with asks, connection.transaction_open? (timeout_helpers.rb:21-31). Inside a transaction it sets both timeouts with SET LOCAL, as the blockless form does. They stay at 0 after the block until COMMIT or ROLLBACK, and nothing has to restore inside an aborted transaction. In all 17 post-deploy migrations the validate call is the last statement, so nothing later relies on the timeout coming back.

The 17 post-deploy migrations (db/post_migrate)

20260414185714_validate_clusters_kubernetes_namespaces_sharding_key_constraint
20260421035856_validate_spam_logs_organization_id_not_null
20260423132622_validate_cluster_platforms_kubernetes_sharding_key_constraint_fixed
20260423132623_validate_cluster_providers_gcp_sharding_key_constraint_fixed
20260423132624_validate_cluster_providers_aws_sharding_key_constraint_fixed
20260509012714_validate_group_wiki_repositories_group_id_fk
20260509012715_validate_group_wiki_repository_states_group_id_fk
20260509012716_validate_group_wiki_repository_states_group_wiki_repository_id_fk
20260601123629_validate_subscription_user_add_on_assignment_versions_org_fk_cascade
20260616221400_validate_composite_fk_on_vulnerability_occurrence_identifiers
20260623153558_validate_member_role_fk_on_protected_branch_merge_access_levels
20260623153630_validate_member_role_fk_on_protected_branch_push_access_levels
20260623153649_validate_member_role_fk_on_protected_branch_unprotect_access_levels
20260727231048_validate_enterprise_user_settings_is_hash
20260730144814_validate_composite_fk_on_vulnerability_occurrences
20260730150835_fk_validation_for_deployments_phase_two
20260821095513_validate_tag_gpg_signatures_project_id_fk

Block form path

flowchart TD
    A["Block form of the timeout helper"] --> B{"Transaction already open?"}
    B -->|"No"| C["Read both timeouts<br/>Set both to 0<br/>Run the block"]
    C --> D["Put back the values that were read<br/>Also when the block raises"]
    B -->|"Yes"| E["SET LOCAL both to 0<br/>Run the block"]
    E --> F["PostgreSQL ends both settings<br/>at COMMIT or ROLLBACK"]

Follow-ups

These sites still use RESET or their own SET and are candidates for follow-up merge requests: async_indexes/index_creator.rb:31, reindexing/reindex_concurrently.rb:118, async_constraints/validators/base.rb:63-64, prepared_async_dml_operations_testing/index_creator.rb:34, transaction_timeout_settings.rb:18-19, orphaned_discoverer.rb:145, seeder.rb:158 and dev.rake:19.

Gitlab::Database::TransactionTimeout.current and .restore lose their last lib caller here. They stay for now; removing them, here or later, is the maintainers' call.

Size

Danger's size warning comes from spec lines. Most of the added lines are new coverage: a real-connection rewrite of timeout_helpers_spec.rb and a new session/settings/transaction_timeout_spec.rb. Most of the rest replace the SET and RESET string expectations in the helper specs, which cannot stay once statement_timeout_disabled? is gone.

References

Relates to #630538 (closed)
The session-level class this builds on: !261853 (merged)
Overlaps !262569 (merged) in timeout_helpers_spec.rb, whose cleanup this spec already restores with SET. Whichever merges second rebases.

Screenshots or screen recordings

The session starts with statement_timeout at 20s and transaction_timeout at 1h, standing in for the connection's configured values. The table shows both after the block, on PostgreSQL 17.11, and the error a failing statement surfaces. The test VM's server default is 0 for both settings, so master's RESET shows as 0. Inside the block both are 0 on master and on this branch.

Case	Before (master)	After (this MR)
Outside a transaction	0 / 0	20s / 1h
The block raises	0 / 0	20s / 1h
Nested blocks, after the outer one	0 / 0	20s / 1h
In a transaction, after `COMMIT`	0 / 0	20s / 1h
In a transaction, a statement in the block fails	`PG::InFailedSqlTransaction`	the original error

After ROLLBACK on this branch the session is also back at 20s / 1h.

How to set up and validate locally

Run the specs for the changed surface:

bundle exec rspec spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb spec/lib/gitlab/database/session spec/lib/gitlab/database/sync_sequences_with_table_data_spec.rb

In a Rails console on PostgreSQL 17, set the session values and read them after a block:

connection = ApplicationRecord.connection
connection.execute("SET statement_timeout TO '20s'")
connection.execute("SET transaction_timeout TO '1h'")
migration = ActiveRecord::Migration.new.extend(Gitlab::Database::Migrations::TimeoutHelpers)
migration.disable_statement_timeout { nil }
connection.uncached { [connection.select_value('SHOW statement_timeout'), connection.select_value('SHOW transaction_timeout')] }

The last line returns ["20s", "1h"] on this branch. On master both values read "0".

Specs: 564 examples, 0 failures over the 16 spec targets below on PostgreSQL 17.11. Master over the same targets before the change: 549 examples, 0 failures. RuboCop reports no offenses on the changed files at 4556b7bd.

The 16 spec targets (under spec/lib/gitlab/database/)

migrations/{timeout_helpers,foreign_key_helpers,constraints_helpers,index_helpers,sidekiq_helpers}_spec.rb
migration_helpers/{convert_to_bigint,require_foreign_key_indexes}_spec.rb
partitioning_migration_helpers/{table_management_helpers,foreign_key_helpers}_spec.rb
repair_index_spec.rb, collation_checker_spec.rb, partitioning/list/convert_table_spec.rb, migration_helpers_spec.rb
session/ (the directory), transaction_timeout_spec.rb, sync_sequences_with_table_data_spec.rb

Database review

The change adds no migration, no table, no column and no query against application tables. The only SQL is the session statements SET, SET LOCAL and SHOW, which have no query plan to attach.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

An AI coding assistant wrote this change and ran the checks above on test VMs.

Restore session timeouts after disable_statement_timeout