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"] -.-> D

The 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.

Edited by Andrew Dunn

Merge request reports

Loading
Loading