direct: add DATABRICKS_BUNDLE_RESOURCE_MAX_WAIT to cap resource waits - #6263
Open
denik wants to merge 11 commits into
Open
direct: add DATABRICKS_BUNDLE_RESOURCE_MAX_WAIT to cap resource waits#6263denik wants to merge 11 commits into
denik wants to merge 11 commits into
Conversation
Collaborator
Integration test reportCommit: 165db3c
7 interesting tests: 3 SKIP, 2 RECOVERED, 2 flaky
Top 12 slowest tests (at least 2 minutes):
|
Co-authored-by: Isaac
Co-authored-by: Isaac
denik
force-pushed
the
denik/resource-max-wait
branch
from
August 18, 2026 09:22
a399df2 to
0036738
Compare
A .permissions or .grants child references only its parent's id, which DoCreate returns before the wait starts, so it does not need a provisioned parent. Exclude child nodes when deciding whether a resource has a dependent, and cap the wait in the vector search index grants test. Co-authored-by: Isaac
Co-authored-by: Isaac
A zero cap built an already-expired context and still spent one poll learning what the caller said it did not care about. Returning early makes it observable: the new acceptance test deploys a cluster, which the test server only advances from PENDING to RUNNING on read, so the absent poll is what proves the resource was left unwaited. Co-authored-by: Isaac
They were excluded from cloud for taking 10-20 minutes, all of it waiting for the index to provision. None of the invariants need a queryable index, so cap the wait per config instead of skipping the coverage. Co-authored-by: Isaac
# Conflicts: # acceptance/bundle/invariant/test.toml
Co-authored-by: Isaac
Jobs have no WaitAfterCreate, so a zero cap there asserted nothing -- the step passed whether or not the zero path worked. resource-max-wait-zero covers it on a cluster, which does wait. Co-authored-by: Isaac
The assertion is whether the poll GET happened, so the cluster body was churn: any change to the test server's cluster defaults would have broken it. Co-authored-by: Isaac
Co-authored-by: Isaac
shreyas-goenka
approved these changes
Aug 20, 2026
| # provisioned index, so the wait is capped here too. Runtime becomes ~2x the cap. | ||
| Env.DATABRICKS_BUNDLE_RESOURCE_MAX_WAIT = "30" | ||
|
|
||
| # No longer slow once the wait is capped, so run it on every PR. |
Contributor
There was a problem hiding this comment.
does this generalize to more cloudslow tests?
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
3 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.
Changes
DATABRICKS_BUNDLE_RESOURCE_MAX_WAIT(in seconds) caps how long deploy and destroy wait for a resource to reach its target state. On expiry the wait is abandoned with a warning rather than failing the deploy: state is saved before the wait, so the resource stays tracked and the next deploy reconciles it.0skips the wait without issuing a request.A resource keeps its full wait when something that runs after it needs it provisioned. Its own
.permissions/.grantschildren do not count: they reference nothing but the parent's id, whichDoCreatereturns before the wait starts. Deletes are capped regardless of dependents, except the delete inside recreate, which has to release the name for the create that follows. Direct engine only.Applied to the vector search index tests that only need the index to exist, not to be queryable:
basic,grants/select, and thevector_search_indexinvariant configs. All three now run on every PR rather than nightly-only, and the invariants are no longer excluded from cloud at all.Why
Vector Search index provisioning takes 15-30 minutes, which made these by far the slowest tests on cloud.
Before is nightly run 32294089064 (
basicandgrants/selectareCloudSlow, so the nightly is the only place they ran). The invariant config was excluded from cloud altogether, so it has no nightly figure; the uncapped number quoted for it is a local run against theaws-clienv with the cap removed andTimeoutCloudraised to 90m. That last part matters: two of the six variants took over 25m uncapped, which is the defaultTimeoutCloud, so enabling them without the cap would time out rather than merely be slow. After is run 32253634089 on this PR. Theinvariantsrange spans six variants per env:no_drift,delete_idempotentanddestroy_idempotent, each with and withoutREADPLAN.What stays slow
recreate/embedding_dimension(13-21m) keeps its full waits and staysCloudSlow. Capping it breaks the test: the capped create leaves the index provisioning, so the recreate then deletes a provisioning index and exceeds the 15mdeleteIndexTimeout. Capping the recreate's delete wait too would just move the failure topendingDeletionTimeout.The cap is set per config rather than for the whole invariant directory on purpose. This index is three orders of magnitude slower than the next slowest invariant config (
cluster, ~100s), and capping every config would leave nothing exercising a realWaitAfterCreateagainst a real backend.Tests
Unit tests for parsing, the child-dependency rule and the wait semantics, including that a genuine failure still fails the deploy and that cancelling the deployment is not swallowed. Two acceptance tests:
resource-max-waitcovers parsing, andresource-max-wait-zerocovers what the cap does to a wait — it deploys a cluster with the cap at 0 and asserts the poll GET is absent, since the test server only advances a cluster from PENDING to RUNNING on read, so the missing poll is what proves the resource was left unwaited.Cloud-checked rather than assumed: a capped create does not produce spurious drift.
grants/selectre-plans to0 to add, 0 to change, 0 to deleteandno_driftpassesverify_no_drift.py(which requires every action to beskip) on all six clouds. The grant also applies to an index that is not ready yet, and the two idempotency invariants survive deleting an index whose first delete was cut short.