Speed up loading bundles with many included files - #6195
Merged
janniklasrose merged 11 commits intoSep 16, 2026
Merged
Conversation
Each file in `include` was applied as its own mutator. Entering a mutator converts the whole configuration between its typed and dynamic representations, so file N re-converted the N-1 resources already merged, making load quadratic in the number of included files. A bundle with 6000 job files spent ~20 minutes in configuration load before any API call. Apply the per-file includes within the enclosing mutator scope instead, so the configuration is converted once per load. `ProcessInclude` merges through `config.Root.Merge`, which keeps both representations in sync, so it does not need a scope of its own. The expanded include list now goes through `Mutate` for the same reason: nothing converts the typed field back afterwards. Measured with `bundle validate` on generated bundles of 8-task jobs, one job per file: 3000 files 301s -> 35s, 5000 files 1237s -> 93s, 6000 files 1208s -> 131s. Merging itself is still quadratic: `mergeMap` allocates a new mapping and copies every existing entry, so file N also copies the N-1 resources already merged. That is untouched here and becomes the dominant term, so load is much faster but not linear. Co-authored-by: Isaac
janniklasrose
force-pushed
the
janniklasrose/bundle-large-number-of-includes
branch
from
August 7, 2026 08:31
22fb541 to
66cb8ab
Compare
Collaborator
Integration test reportCommit: bfd593d
Top 3 slowest tests (at least 2 minutes):
|
Adds an end-to-end acceptance test that plans a bundle whose resources are defined across two include globs (resources/*.yml, resources/*/*.yml) and the root databricks.yml, and asserts every resource appears in the plan exactly once. A target override for one job lives in a separate file from its base definition, verifying that a cross-file merge lands in the plan rather than being dropped or duplicated. This guards the include loading and merge path exercised by the switch to ApplySeqInScopeContext: a regression that dropped or duplicated a resource, or lost a cross-file override, would change the plan summary. Co-authored-by: Isaac
…large-number-of-includes
The leaf test.toml only repeated Local/Cloud/EnvMatrix, which are inherited from the root acceptance/test.toml. The `Local` key was since removed from TestConfig on main (tests always run locally), so decoding the leaf now errors with an undecoded key. Remove the leaf entirely and inherit from the root instead. Co-authored-by: Isaac
Cover the three properties the include loading path relies on: mutators are applied once in order, application stops at the first mutator that logs an error (later mutators do not run), and changes made through Root.Mutate survive without a per-mutator scope, becoming visible in both the typed and dynamic configuration once the enclosing scope exits. Co-authored-by: Isaac
janniklasrose
requested review from
andrewnester,
denik,
pietern and
shreyas-goenka
August 12, 2026 11:20
| } | ||
|
|
||
| // Swap out the original includes list with the expanded globs. | ||
| b.Config.Include = files |
Contributor
There was a problem hiding this comment.
I think includes were part of validate -o json output. If so, the output is different now and can break some worfklows. I think just calling it out would be enough though, no need to have specific handling
Member
Author
There was a problem hiding this comment.
we have acceptance/bundle/templates/default-python/integration_classic/out.validate.dev.json and that isn't affected by this change.
the output is different now
in which way? maybe you have a case in mind that isn't covered by that golden
| // (which keeps both representations in sync). A mutator that assigns to a typed field | ||
| // directly relies on the scope entry to carry that value into the dynamic tree, and | ||
| // would lose it here. | ||
| func ApplySeqInScopeContext(ctx context.Context, b *Bundle, mutators ...Mutator) { |
Contributor
There was a problem hiding this comment.
Do we really need this function? We can just call mutators in place directly like we do everywhere else
Member
Author
There was a problem hiding this comment.
Prefer to leave this logic here, so it's unit-testable and it keeps bundle/config/loader/process_root_includes.go cleaner
An unknown field in a nested included file makes bundle validate report its location; asserting that location proves source locations still point to the right included file after includes are applied within the caller's mutator scope. Co-authored-by: Isaac <no-reply@databricks.com>
TestProcessRootIncludesMergesAcrossFiles duplicated merge coverage that the plan_multifile acceptance test already exercises (two globs, a root-file resource, and a cross-file target override), including the location-preservation assertion now covered there. Co-authored-by: Isaac <no-reply@databricks.com>
…large-number-of-includes
janniklasrose
force-pushed
the
janniklasrose/bundle-large-number-of-includes
branch
from
September 15, 2026 20:34
3c10a5e to
efe6d6b
Compare
andrewnester
approved these changes
Sep 16, 2026
janniklasrose
deleted the
janniklasrose/bundle-large-number-of-includes
branch
September 16, 2026 09:33
deco-sdk-tagging Bot
added a commit
that referenced
this pull request
Sep 16, 2026
## Release v1.17.0 ### Notable Changes * Bump the direct deployment state version to 3. Clients older than v1.8.0 will reject bundles deployed with this release. ([#6713](#6713)) ### CLI * Add an `INVALID_REFRESH_TOKEN` error code to `databricks auth token --output json` failures. ([#6684](#6684)) * Add experimental `databricks auth docker configure` to configure Docker credential helper access for Databricks Artifact Registry. ([#6700](#6700)) * Add experimental `databricks auth docker token` to generate Docker credentials for Databricks Artifact Registry. ([#6699](#6699)) * `databricks environments setup-local` now reports the `E_PROVISION_CONFLICT` error code instead of the generic `E_PROVISION` when `uv sync` fails to resolve a dependency conflict. ([#6666](#6666)) * Preserve SSH sessions across temporary tunnel disconnects, with bounded replay and backpressure for large transfers. ([#6650](#6650)) * Allow OAuth U2M logins to override the CLI client ID with `--client-id`, profile `client_id`, or `DATABRICKS_CLIENT_ID`. ([#6594](#6594)) ### Bundles * direct: Store a dashboard's `serialized_dashboard` in state as a content hash instead of its full contents. ([#6105](#6105)) * direct: Fix pipelines recreation when the whole `ingestion_definition` block is added or removed. ([#6589](#6589)) * `bundle plan`, `deploy`, and `destroy` no longer report removing `permissions`, `grants`, or secret scope ACLs from a bundle as a deletion, since it leaves the resource untouched. ([#6647](#6647)) * `bundle plan` and `deploy` no longer list or count a resource that was already deleted remotely as a deletion, matching `bundle destroy`; applying still cleans up its stale state entry. ([#6675](#6675)) * Fix `bundle run` failing with `expected an int, found a string` when an unrelated resource references another resource that is not deployed. `bundle run` now resolves `${resources.*}` references only within the resource being run. ([#6690](#6690)) * Add grants support for the AI Gateway `model_service`, `mcp_service`, and `model_provider_service` resources (direct engine). ([#6635](#6635)) * Add bundle support for the AI Gateway `mcp_service` resource (direct engine). ([#6633](#6633)) * Add bundle support for the AI Gateway `model_provider_service` resource (direct engine). ([#6634](#6634)) * Add bundle support for the AI Gateway `model_service` resource (direct engine). ([#6525](#6525)) * Prevent resource drift on catalogs if `storage_root` contained a trailing slash in the URL. ([#6622](#6622)) * Fixed a "lineage mismatch in state files" error that could occur after destroying a bundle and redeploying it from another machine. `bundle destroy` now removes the local state file so no stale lineage is left behind, and prunes the state directories it leaves empty (such as `.internal/` and `sync-snapshots/`). ([#6210](#6210), [#6685](#6685)) * direct: `bundle plan` no longer reports a permanent update on a cluster that uses a cluster policy: when the cluster spec sets `policy_id`, a field present in the remote but absent from the bundle config is not treated as drift. ([#6531](#6531)) * `bundle deploy` on the direct engine now reports each resource as soon as it is deployed, instead of listing them all after the deployment finishes. A deploy that fails part way through now reports the resources it did apply. ([#6361](#6361)) * Direct-engine bundles no longer flag phantom drift on server-populated nested fields under reused config types (e.g. `external_locations` file-event-queue resource IDs, `database_instances` parent-instance refs, `apps` git credential ID). ([#6618](#6618)) * `databricks bundle generate app` now reproduces a git-backed app's `git_repository` and `git_source` configuration instead of emitting a workspace `source_code_path`, so generating from a Git-deployed app no longer silently converts it to workspace source. ([#6656](#6656)) * Improved configuration load time for bundles with many included files. ([#6195](#6195)) * `bundle destroy` no longer deletes triggered job runs, leaving them untouched on the backend. ([#6672](#6672)) * direct: resources.job\_runs: new lifecycle.triggers.on\_file\_change setting to restart the run when monitored files change. Can be set to a series of paths or globs. ([#6309](#6309)) * Bundle summary now shows a name for Postgres branches, endpoints, databases, and roles instead of a blank Name field. ([#6663](#6663)) * Added PyDABs (Python) support for cluster policies, dashboards, and Genie spaces. ([#6585](#6585)) * CLI commands no longer imply that a resource whose type has no workspace URL is merely not deployed yet. ([#6583](#6583)) * Capture the implicit dependency a vector search index has on a catalog or schema defined in the same bundle, so the catalog and schema are deployed first. ([#6655](#6655)) ### Dependency Updates * Bump dependencies with known vulnerabilities. ([#6695](#6695)) * Bump `github.com/databricks/databricks-sdk-go` from v0.177.0 to v0.178.0. ([#6673](#6673)) * Bump Terraform provider from v1.131.0 to v1.132.0. ([#6671](#6671))
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.
Summary
Each file in
includewas applied as its own mutator. Entering a mutator converts the whole configuration between its typed and dynamic representations (config.Root.MarkMutatorEntry→convert.FromTyped), so file N re-converted the N-1 resources already merged — making configuration load quadratic in the number of included files.This came from a customer bundle with ~6000 jobs, one job per YAML file (the documented layout), plus ~18000 notebooks. Their CI spent ~29 minutes in configuration load before any API call.
Fix
Apply the per-file includes within the enclosing mutator scope, so the configuration is converted once per load rather than once per file.
ProcessIncludemerges throughconfig.Root.Merge, which keeps both representations in sync, so it does not need a scope of its own.The expanded include list now goes through
Mutatefor the same reason: with no per-file scope, nothing converts the typed field back into the dynamic tree afterwards, and the nextToTypedwould restore the raw glob patterns. An empty list is dropped rather than written as[], since the typed field isomitempty.Results
To size this, I generated bundles matching the customer's shape — real files on disk, one 8-task job per YAML file across nested directories, 4 wildcard include patterns, plus ~18000 notebook files at the 6000-job size. Measured with
bundle validate(the load path, no deploy) on an Apple M4 Max:At the customer's size, ~20 minutes becomes ~2 minutes (~89% less). Before the fix, doubling the file count roughly quadrupled the time; repeated runs at 3000 files spanned 301-316 s, so the growth is well outside measurement noise. A CPU profile at 800 files attributed 22% of samples to
MarkMutatorEntry, nearly all underconvert.FromTyped, plus the allocation and GC churn it caused.Defining several jobs per file is an effective workaround for affected bundles and needs no CLI change: 1000 jobs one-per-file took 36 s, and the same jobs in a single file took 1.5 s.
What this does not fix
merge.Mergeis independently quadratic —mergeMapallocates a new mapping and copies every existing entry, so file N also copies the N-1 resources already merged. That is untouched here and becomes the dominant term once this change lands, so load is much faster but not linear: at 100/200/400 files, time per file still rises from 91 µs to 120 µs to 159 µs.A prototype that accumulates included resources into a single mapping instead of folding each file into the whole configuration was linear across 500-6000 files and reached ~1.2 s at 6000 files. It is deliberately not part of this PR: it has to preserve the merge semantics the current pairwise merge provides, and that deserves review on its own. Worth recording for whoever picks it up — a naive version that replaces instead of deep-merging on key collision drops fields from the earlier definition and collapses the accumulated locations, which also stops
validate:unique_resource_keysfrom reporting duplicate resource keys, since it detects them by counting locations per key.One behavior change worth flagging: per-include mutator timings are no longer recorded individually in telemetry; they roll up into
ProcessRootIncludes.Tests
Three unit tests in
process_root_includes_test.go, covering what this change could plausibly break:bundle validate -o jsondoes not gain an emptyinclude(my first attempt at the above regressed this)Full unit suite (6235 tests) and acceptance suite (2524) pass.
bundle/templates/lakeflow-integrationsfails identically on a clean tree in my environment (local Python/uv build issue), unrelated to this change.This pull request and its description were written by Isaac.