Repository navigation
A per-collector database allow-list on the schedule row, so a fan-out pays for a sample (#3477) - #3509
Merged
Conversation
… pays for a sample (#3477) V125: config.config_collector_schedules gains a nullable databases text[] — empty/NULL is today's behavior, non-empty collects ONLY those databases, the fleet-wide row sets a default a server row overrides (NULL column falls through, an explicit empty array is the opt-back-out), and excludedDatabases still wins because the composed predicate is scoped-in AND NOT excluded, spliced engine-side in the exclusion filter's own parameter shape so both instruments share the engine's collation reality. One seam per fan-out family: the five enumeration-driven collectors compose the predicate through DatabaseScopeFilter.BuildEnumerationPredicate (a catalog-walking census pins that a sixth enumerator cannot skip it), and the per-database connection loop takes it inside both providers' BuildDatabaseListPlan. The worker resolves the scope live off the same schedule overrides the cadence gate reads; the Viewer's schedule editor gains a comma-separated Databases column that round-trips the null/empty distinction WYSIWYG. Verified against a live TimescaleDB pg17: ladder lands at 125, the three stored readings stay distinct, the beacon bumps on a scope write, and the on-engine enumeration returns scoped-in minus excluded with a never-named database staying out.
erikdarlingdata
enabled auto-merge (squash)
September 17, 2026 13:08
…s, the hygiene census's preferred shape LivePostgresCollectionHygieneTests requires every class reaching the shared store to serialize or say why not - and the rung file's live arm writes config_service and config_collector_schedules on the shared store directly, so the own-store exemption would be a lie. The split is the shape the census itself recommends: the live test carries Collection(live-postgres) in its own file, and the rung file's eleven pure pins stop paying serialization for one live arm.
… byte-identity census is the pin the blast-radius map called absent BothSkusToolDescriptions_StayByteIdentical holds the two SKUs' get_collection_health descriptions equal to the byte, and the empty-enumeration honesty clause went into Darling's only - the lane's map read the clause as unpinned because the pin lives in the OTHER SKU's test project. Mirrored with the house attribution for Darling-only knobs, identical bytes both sides, proven by extraction before commit.
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
1 participant
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.
Closes #3477 — ceremonially; the watcher closes it at merge. The changelog entry rides the batched entries PR, per the new parallel-lane buffering pattern.
The measured case, and the lever it earned
The reporter's evidence closed the design question their issue opened: a 72-database instance pays 43 minutes per index_object_stats pass, the slowest database accounts for 4.97% of it, and the store's second-largest object is the reward — width, not concentration, so a scope is the right lever, and a representative-database allow-list takes the pass to roughly one database's worth. This ships the general form: an optional
databases text[]allow-list on the schedule row, honored by every fan-out family, because the migration rung is the expensive part either way and the predicate is identical.Semantics, exactly as agreed on the issue
NULL or absent is today's behavior — no migration risk, nothing changes for anyone who doesn't ask. A non-empty list means only those databases, which is how "off everywhere except this one" is expressed, and a database created tomorrow stays out — the reporter's dev instances create databases daily, and the allow-list form exists precisely because a deny-list can't say that. Layering mirrors the schedule's own: per-server over fleet-default, with one array-specific rule stated in code — NULL falls through the layers, an explicit empty array stops the fall-through, the opt-back-out a deny-list also can't express.
excludedDatabasesstill wins by construction: the composed predicate is scoped-in minus excluded, in one statement, judged by the engine — the scope'sINparameters take the exact shape of the exclusion'sNOT IN, so the two instruments can never disagree about what a database name is (SQL Server folds case per its collation; PostgreSQL compares bytes — the same reality the exclusion already lives with, now pinned for both).Two seams, not five
The five enumeration-driven collectors share one composed predicate builder, with a catalog-walking census that fails at authoring time if a sixth enumerator skips the seam. The per-database connection loops (Azure's eight, PostgreSQL's) take the scope through the target providers' list plans, spliced beside the exclusion. Single-statement per-database readers are deliberately unscoped — they don't fan out, and scoping them would be exactly the collateral blinding the issue rejected
excludedDatabasesover. XeSessions provisioning and Azure's own-database registration are stated, pinned boundaries.The rung, by the recipe
V125 is one nullable schema-qualified column — no default, no CHECK, no grant carve, no beacon of its own (the V17 trigger already bumps), no data movement. The viewer probe takes its fourth part at ordinal 100; the V124 rung tests demote per the written handover recipe, with one reasoned exception documented in place: V124's END-position bind pins stay where they are, because they pin the alert-settings bind this rung never touches — the recipe's intent is that the END belongs to the newest rung of the same bind, and this rung binds nothing there.
There are no MCP schedule tools — the Viewer is the schedule surface, and its editor gains the scope column with round-trip semantics matching the layering (fleet blank = NULL, custom-server blank = explicit empty), while the enable/disable toggle's upsert deliberately still names only
enabled, pinned as the no-clobber property that keeps a toggle from destroying a scope.Verified live, not argued
A Docker TimescaleDB harness ran the ladder to 125 (replay no-op), proved NULL/empty/list distinct through the service's own read, exercised the resolver's layering, confirmed the beacon bumps on a scope write, and executed the composed predicate on-engine: scoped-in minus excluded, never-named databases absent, all five enumerations parameterized — 20/20. The probe SQL ran raw against migrated and unmigrated stores. Tests cannot run on macOS; the suite compiles at 0 errors and the live arm runs wherever the PG fixture is set. CI is the first suite execution, review-dark per the known window.