Skip to content

test: fix StartCmd keep-alive race, shared DB table/query collisions, and sample 503 retries - #4419

Closed
DylanVolz wants to merge 3 commits into
mainfrom
fix/deflake-ci-tests
Closed

DylanVolz wants to merge 3 commits into
mainfrom
fix/deflake-ci-tests

Conversation

@DylanVolz

@DylanVolz DylanVolz commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes five classes of CI test flakes across PR and post-merge Cloud Build runs:

1. **Shared `tests.StartCmd` shutdown & Keep-Alive race (all 121 call sites):** 
`StartCmd`'s returned `cleanup()` previously only deleted the temporary config file without stopping the in-process Toolbox server, waiting for `:5000` to close, or flushing `http.DefaultClient` Keep-Alive connections. Sequential `Test*` functions in multi-server packages (e.g., `TestAlloyDBCreateCluster/successful_creation`) could reuse a closing Keep-Alive socket from the prior test's server and fail immediately with `EOF` (`0.00s`). `StartCmd`'s `cleanup()` now idempotently stops the server, waits for shutdown, drops idle connections, and removes the temp file.
2. **MySQL & MSSQL `uniqueID` table isolation:** 
`CleanupMySQLTables` and `CleanupMSSQLTables` previously ran at test startup with no filter, dropping every table in `test_database` out from under any concurrent CI build (`Error 1146: Table ... doesn't exist` / `list_table_stats` mismatches). Scopes both helpers to `%_<uniqueID>` (plus sweeping leaked test tables > 1 hour old) at teardown (`t.Cleanup`) matching Postgres (#2377), and filters shared-schema list assertions (`list_tables`, `list_table_stats`, `list_table_fragmentation`, `list_tables_missing_unique_indexes`) to the current run's tables.
3. **MySQL & Postgres `list_active_queries` idle subtest contention:** 
Adds per-run `testUUID` `queryTag` / `appName` filtering to `tc1` (`invoke list_active_queries when the system is idle`) in `RunMySQLListActiveQueriesTest` and `RunPostgresListActiveQueriesTest` (completing #3901).
4. **Transient Gemini `503 UNAVAILABLE` / `429` capacity spikes in sample tests:**
 Adds bounded exponential-backoff retry (up to 3 attempts, resetting `$TABLE_NAME` between retries in `.ci/sample_tests/run_tests.sh` and in Go sample `*_test.go` harnesses) while keeping user-facing `quickstart.go` sample code untouched.
5. **serverless spark test batch contention:** 
 Adds a status filter to the serverless spark test so that there isn't contention between concurrent runs

PR Checklist

Thank you for opening a Pull Request! Before submitting your PR, there are a
few things you can do to make sure it goes smoothly:

  • Make sure you reviewed
    CONTRIBUTING.md
  • Make sure to open an issue as a
    bug/issue
    before writing your code! That way we can discuss the change, evaluate
    designs, and agree on the general idea
  • Ensure you have manually reviewed the entire diff before requesting a
    review
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Make sure to add ! if this involve a breaking change

🛠️ Fixes #<issue_number_goes_here>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces robust retry logic with exponential backoff for transient upstream LLM errors (such as 503 and 429) across CI test runners and Go integration tests. It also enhances test isolation and reliability in concurrent environments by generating unique run IDs for database tables, targeting cleanup to those IDs (while pruning stale tables older than an hour), and using 'DROP TABLE IF EXISTS'. Additionally, server lifecycle management in tests is simplified and made safer: 'StartCmd' now returns a unified cleanup function that handles graceful shutdown, port release, and idle HTTP connection dropping, which is verified by a new sequential integration test. I have no additional feedback to provide as there are no review comments.

@DylanVolz
DylanVolz force-pushed the fix/deflake-ci-tests branch from d31989a to 5d3f531 Compare November 5, 2026 16:17
@github-actions github-actions Bot removed the samples label Nov 5, 2026

@Yuan325 Yuan325 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR seems to be addressing multiple items, appreciate if you can split them to multiple PRs for easier reviews. Thank you

@DylanVolz

Copy link
Copy Markdown
Contributor Author

closed in favor of:
#4438
#4440
#4439
#4442
#4441

@DylanVolz DylanVolz closed this Nov 7, 2026
@github-actions

github-actions Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor

🧨 Preview deployments removed.

Cloudflare Pages environments for pr-4419 have been deleted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants