feat(lifecycle): skip index creation on large collections and mass-expiration rules#2748
Open
delthas wants to merge 6 commits into
Open
feat(lifecycle): skip index creation on large collections and mass-expiration rules#2748delthas wants to merge 6 commits into
delthas wants to merge 6 commits into
Conversation
…piration rules Add two pre-flight heuristics to the conductor's index-creation path: 1. Collection size guard: if the collection already has more than 1M documents or its on-disk storage exceeds 1 GiB, skip auto-creating the v2 lifecycle indexes. The build's transient disk usage on existing large collections is unpredictable (spill files plus the final index can easily exceed 5-10x the _id_ index size during the merge phase), and can put the cluster at risk on tight deployments. 2. Mass-expiration guard: if every enabled lifecycle rule has no filter (no prefix, no tags) and an Expiration action that is either already- past (Date) or smallest-legal (Days <= 1), skip auto-creating the indexes. The indexes provide little selectivity benefit and would be churned out before paying for themselves. Both guards complement (do not replace) the existing BB-753 free-disk check and the BB-778 SosAPI check. Buckets that already have the v2 indexes continue to use v2 regardless of these guards. The mass-expiration verdict is precomputed once per bucket in listMongodbBuckets and plumbed via task.allRulesAreMassExpiration. The collection-size check reuses the collStats fetch that BB-753 already does — no extra round trip. The rule helpers live in extensions/lifecycle/util/mongoRules.js and operate on the MongoDB internal lifecycle format (lowercase ruleStatus, actions, etc.), distinct from the AWS S3 format that extensions/lifecycle/util/rules.js and RulesReducer.js consume. Issue: BB-779
Contributor
Hello delthas,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 3 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.4 #2748 +/- ##
===================================================
- Coverage 74.65% 74.64% -0.02%
===================================================
Files 199 200 +1
Lines 13654 13684 +30
===================================================
+ Hits 10194 10215 +21
- Misses 3450 3459 +9
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…ongoDB calls The mass-expiration guard is intentionally placed before the diskUsage/collStats fetch to avoid the I/O cost on buckets we'll skip anyway. Add an explicit test where the MongoDB client throws if either call is made — the test passes iff the short-circuit holds. Issue: BB-779
Spec files for util/ helpers live flat under tests/unit/lifecycle/ (see rules.spec.js, RulesReducer.spec.js). Match that placement. Issue: BB-779
Drop function-level JSDocs (name + impl already convey the contract). Condense the mongoRules.js file header to the format-distinction warning (real footgun). Drop threshold-constants rationale (visible from names). Condense the mass-expiration guard comment to the non-obvious "churned out before paying for itself" bit. Keep the empty-array carve-out inside allRulesAreMassExpiration where the surprising no-skip behavior lives. Issue: BB-779
Previously allRulesAreMassExpiration returned false when no rule was enabled, on the theory that the user might re-enable a selective rule. But that's overly defensive: nothing's running today, so we shouldn't proactively pay the index build cost. Drop the special case so vacuous truth applies — every() over zero enabled rules returns true, and we skip indexing as desired. Issue: BB-779
A Date one day in the future is semantically equivalent to Days=1: by the next conductor cycle the rule will be expiring everything. Apply the same 1-day slack to the Date branch as Days <= 1 already implies. Issue: BB-779
|
LGTM — clean, well-tested implementation of the two pre-flight heuristics. |
DarkIsDude
approved these changes
Jun 5, 2026
|
|
||
| function allRulesAreMassExpiration(rules, currentDate = new Date()) { | ||
| return (rules || []) | ||
| .filter(r => r && r.ruleStatus === 'Enabled') |
There was a problem hiding this comment.
this is already checked inside the isMassExpirationRule ?
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
Add two pre-flight heuristics to the conductor's index-creation path so we keep lifecycle on v1 (full scan) in these cases instead of paying the build cost:
Collection too big — if
count > 1MORstorageSize > 1 GiB, skip auto-creating v2 lifecycle indexes. The build's transient disk usage on existing large collections is unpredictable; spill files plus the final index can easily exceed 5–10× the_id_index size during the merge phase. Risky on tight deployments.Mass-expiration rules — if every enabled rule has no filter (no
rulePrefix, notags) and anExpirationaction that is either already-past or within ~1 day in the future (date) or smallest-legal (days <= 1), skip auto-creating the indexes. They'd provide little selectivity benefit and be churned out before paying for themselves. The "all rules disabled / no rules at all" case also skips (vacuous truth — no enabled selective rule to benefit from indexing).Both guards complement (do not replace) the BB-753 free-disk check and the BB-778 SosAPI check. Buckets that already have the v2 indexes continue to use v2 regardless of these guards.
Implementation notes
listMongodbBucketsand plumbed viatask.allRulesAreMassExpiration.collStatsfetch BB-753 already performs — no extra round trip.ruleStatus,actions[], etc.), distinct from the AWS S3 format thatutil/rules.jsandRulesReducer.jsconsume. The precedent for parsing the MongoDB internal shape isLifecycleOplogPopulatorUtils.isBucketExtensionEnabled.LifecycleMetrics.onLegacyTask:'collectionTooBig'and'massExpirationRule'.Design decisions (open to bikeshedding)
util/mongoRules.jsfile rather than inlining inLifecycleConductor.jsor extendingLifecycleOplogPopulatorUtils.js. Rationale: the helpers are pure rule-shape predicates, not conductor- or populator-specific. Direct unit testability (19 edge-case tests inmongoRules.spec.js) is worth the extra file. Codebase precedent:util/rules.js+util/RulesReducer.jsare both single-consumer files with their own specs.task.isLifecycledandtask.hasSosApi— both flat primitives). The alternative —task.lifecycleRules: [...]with the helper called inside_indexesGetOrCreate— would offer flexibility for future rule-shape checks but breaks the existing flat-primitive convention. If/when we accumulate 3+ rule-based heuristics, that's the natural refactor moment.Full rationale in BB-779.
Issue: BB-779