ci: bounded FIFO queue for integration test runs - #1439
Merged
Conversation
Coverage report
This PR does not seem to contain any modification to coverable code. |
|
Integration results for PR #1439 — UC cluster ❌ cancelled · SQL warehouse ❌ cancelled · All-purpose cluster ❌ cancelled |
sd-db
added a commit
that referenced
this pull request
May 4, 2026
Initial verification of PR #1439 showed run #2's gate exiting in 7s while run #1 was still in_progress — the opposite of the intended serialization. Reading turnstyle's source (src/wait.ts at v3.2.2) shows queue-name is a substring filter against run.display_title / run.name, not an arbitrary tag: matchesQueue = run.display_title?.includes(queueName) || run.name?.includes(queueName); Our workflow name is "Integration Tests" and runs are auto-titled "Integration Tests #N" — neither contains "integration-shared-compute". The filter matched nothing, so turnstyle saw no other runs to wait on and proceeded immediately. With queue-name omitted, turnstyle defaults to filtering by workflow_id only (the initial fetch path in Waiter.wait), which is exactly the serialization scope we want for one workflow.
|
Integration results for PR #1439 — UC cluster ❌ cancelled · SQL warehouse ❌ cancelled · All-purpose cluster ❌ cancelled |
|
Integration results for PR #1439 — UC cluster ❌ cancelled · SQL warehouse ❌ cancelled · All-purpose cluster ❌ cancelled |
|
Integration results for PR #1439 — UC cluster ❌ cancelled · SQL warehouse ❌ cancelled · All-purpose cluster ❌ cancelled |
tejassp-db
approved these changes
May 7, 2026
Background: integration runs share Databricks compute (clusters + SQL warehouse). Until now nothing prevented multiple runs from piling onto that compute simultaneously. The existing per-PR concurrency block only cancels stale re-dispatches of the *same* PR; it does not coordinate across different PRs or against the nightly schedule. Reviewers triggering /integration-test on several PRs in quick succession, or a batch dispatch with several pr_numbers, can multiply the load. Change: 1. New `gate` job sits between `prepare` and the three e2e test jobs. It uses softprops/turnstyle (pinned to v3.2.4 by SHA) as a single-slot serializer with a repo-scoped queue name and same-branch-only=false, so every workflow run — PR triggers, schedule runs, ad-hoc git_ref dispatches — funnels through the same FIFO. Wait is bounded by abort-after-seconds=14400 (~4h, ≈ two runs ahead before timeout). 2. The three test jobs gain `needs: [prepare, gate]` so they only start after a slot is acquired. They also drop `max-parallel: 2` to `max-parallel: 1` in the matrix — without this, a batch dispatch would hold one gate slot but still run two PRs' worth of cluster load inside it, defeating the gate's intent. What does NOT change: - The existing per-PR `concurrency:` block stays — it handles same-PR re-dispatches, which turnstyle does not. - `needs: prepare`'s clean skip propagation on empty targets is preserved by guarding the gate with an `if:` on the same condition. - The post-run `report-status` comment job is unchanged; if a run hits the gate timeout, the per-job results show as `skipped`, with the run-details link pointing to the gate step's log.
Use the older, more battle-tested v3.2.2 release of softprops/turnstyle rather than the latest v3.2.4. SHA-pinned to commit 15f9da4059166900981058ba251e0b652511c68f.
Initial verification of PR #1439 showed run #2's gate exiting in 7s while run #1 was still in_progress — the opposite of the intended serialization. Reading turnstyle's source (src/wait.ts at v3.2.2) shows queue-name is a substring filter against run.display_title / run.name, not an arbitrary tag: matchesQueue = run.display_title?.includes(queueName) || run.name?.includes(queueName); Our workflow name is "Integration Tests" and runs are auto-titled "Integration Tests #N" — neither contains "integration-shared-compute". The filter matched nothing, so turnstyle saw no other runs to wait on and proceeded immediately. With queue-name omitted, turnstyle defaults to filtering by workflow_id only (the initial fetch path in Waiter.wait), which is exactly the serialization scope we want for one workflow.
Trim turnstyle implementation detail out of the YAML comment; keep only the behavior the workflow promises.
sd-db
force-pushed
the
sd-db/ci/integration-bounded-queue
branch
from
May 7, 2026 17:39
3cf2934 to
7b97d9e
Compare
|
Integration results for PR #1439 — UC cluster ✅ success · SQL warehouse ✅ success · All-purpose cluster ✅ success · Shard coverage ✅ success |
Shard arrays now live in a single place — `prepare` job outputs — and drive both `prepare-shards`'s NUM_SHARDS map and each test job's `matrix.shard` + `max-parallel`. Reshaping the per-profile shard fan-out is a one-line edit instead of touching three call sites.
GitHub Actions workflow expressions don't have a length() function for arrays, so deriving max-parallel from the shard array via length() at matrix-expansion time is rejected by the parser. Emit a matching `count_*` output for each shard array; both are computed from the same bash variable in `parse` so the single-source-of-truth invariant holds.
Both `max-parallel` and `prepare-shards` need shard counts as integers, so reuse `prepare.outputs.count_*` (already pre-computed via jq) instead of injecting the array via env and re-running jq. Saves the env block and three jq calls; integer substitution from a trusted output is bash-safe.
- Drop the 6-line outputs preamble; keep the one non-obvious bit (no length() in GHA expressions). - Trim the run-uc-cluster-e2e-tests rationale to the if-always/empty- matrix WHY; drop the matrix/needs/max-parallel restating. - Drop duplicated "Matrix shape contract" boilerplate from the two sibling test jobs — the matrix block is self-documenting. - Remove redundant '!= ""' from gate's if; siblings only check '!= "[]"'.
|
Integration results for PR #1439 — UC cluster ❌ cancelled · SQL warehouse ❌ cancelled · All-purpose cluster ❌ cancelled · Shard coverage ❌ failure |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Today there is no coordination between integration runs that share Databricks compute. The existing per-PR
concurrency:block inintegration.yml:46-48only cancels stale re-dispatches of the same PR — it does not stop different PRs (or PR + nightly schedule) from running simultaneously and dog-piling the cluster/warehouse. Reviewers triggering/integration-teston several PRs in quick succession, or a batchpr_numbers: "100,200,300"workflow_dispatch, can multiply the load.This PR adds a repo-wide bounded FIFO queue: at most one PR's worth of compute runs at a time; everything else queues in arrival order; waiters time out after 4 hours so the queue is bounded.
Test plan
Pre-merge verification (manual
workflow_dispatchwith--refpointing at this branch)Single-run sanity — gate acquires slot immediately, test jobs proceed.
Watch the
gatejob log: a single poll, then "no other runs found", proceed.Concurrency / FIFO ordering — second run queues behind the first.
# Kick off two in quick succession (different PRs so per-PR concurrency doesn't cancel) gh workflow run integration.yml --repo databricks/dbt-databricks --ref ci/integration-bounded-queue --field pr_numbers=1438 gh workflow run integration.yml --repo databricks/dbt-databricks --ref ci/integration-bounded-queue --field pr_numbers=1439Confirm in the Actions tab: the second run's
gatestep polls and waits