Stop timeout_helpers_spec leaking a reset statement_timeout
What does this MR do and why?
timeout_helpers_spec.rb ends its real-database examples with RESET statement_timeout. The RESET returns main to the cluster default, 0 in CI, not the 15s database.yml sets. The timeouts check that diagnostics_rake_spec.rb runs then reports an error, and the rake task exits. The SystemExit escapes the rspec-rails fixture around hook, which has no ensure. The hook's rollback never runs, so main stays in an open transaction. In single-db-ci-connection jobs the cleaner on the ci connection waits on that transaction's locks, and later examples fail.
flowchart TD
A1["timeout_helpers_spec<br/>RESET"] --> B["main: statement_timeout 0"]
A2["lock retries spec, :delete<br/>SET 100ms, then RESET"] --> B
B --> C["diagnostics_rake_spec<br/>timeouts check errors"]
C --> D["rake task exits<br/>with SystemExit"]
D --> E["rollback skipped<br/>main idle in transaction"]
E -->|main and ci on one database| F["ci cleaner waits<br/>later examples fail"]
X1["Fix 1: SHOW, then SET<br/>restores 15s"] -.-> A1
X2["Fix 2: SHOW, then SET<br/>if previous_timeout"] -.-> A2
X3["Fix 3: SET LOCAL<br/>at the check maximum"] -.-> C
X4["Fix 4: not_to raise_error<br/>rollback runs"] -.-> DThe hooks now capture the timeout with SHOW and restore it with SET, as !257270 (merged) did. The diagnostics example runs inside not_to raise_error, so a failing check later fails one example, not the rest of the run. The statement timeout example in with_lock_retries_outside_transaction_spec.rb ends with the same RESET. That carried an unlimited timeout into the diagnostics example in https://gitlab.com/gitlab-org/gitlab/-/jobs/17297954129. It now restores the captured value, and the diagnostics example sets its own timeout with SET LOCAL.
In the helper itself, the block form of disable_statement_timeout ends with the same RESET. That site is part of #630538, so this MR changes only the specs. The closed draft !258188 (closed) restored these hooks with SHOW and SET too.
References
Closes #633892
Screenshots or screen recordings
Not applicable, spec-only change.
How to set up and validate locally
Use a config/database.yml whose test: section matches config/database.yml.postgresql: main and ci both on gitlabhq_test, each with statement_timeout: 15s. That is what setup_database_yml in scripts/utils.sh writes for single-db-ci-connection jobs. Run the specs in the order that job runs them:
bin/rspec --order defined spec/lib/gitlab/database/migrations/timeout_helpers_spec.rb spec/tasks/gitlab/db/diagnostics_rake_spec.rb spec/workers/ci/pipeline_creation_metrics_worker_spec.rb spec/services/import/github/cancel_project_import_service_spec.rb| Checkout | Result |
|---|---|
| master | 42 examples, 22 failures, 1 error occurred outside of examples |
| this branch | 43 examples with the probe below, 0 failures |
this branch, timeout_helpers_spec.rb change reverted |
43 examples with the probe below, 0 failures; the probe reads 0 instead of 15s |
the 119 files of shard 34/35 in job order, then diagnostics_rake_spec.rb, with only main in database.yml, ci disabled, and level:background_migration, click_house, duo_workflow_service and partition_tz excluded (5083 examples) |
the diagnostics example fails at the first commit and passes on this branch |
To see the leak itself, list a throwaway spec file after timeout_helpers_spec.rb in the command above. Its one example runs puts ApplicationRecord.connection.select_value('SHOW statement_timeout'). It prints 15s on this branch and 0 with the timeout_helpers_spec.rb change reverted.
With only main in database.yml, --order defined ran with_lock_retries_outside_transaction_spec.rb:201, a line-filtered probe like the one above, then diagnostics_rake_spec.rb:100. The probe read 0, and the diagnostics example failed at the first commit.
The cascade needs main and ci on one database. With a decomposed database.yml, master still fails the diagnostics example when timeout_helpers_spec.rb runs first.
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.
An AI coding assistant wrote this change and ran the specs above and RuboCop on a test VM.