rishabhdaim commented on PR #3125: URL: https://github.com/apache/jackrabbit-oak/pull/3125#issuecomment-5732815591
Thanks @jsedding — you're right about the partially-compacted cases. I verified that partial compaction is currently unreachable: `AbstractCompactionStrategy` only takes the `compactDown` (partial-state) path when `stateSaveTrigger.isCancelable()` or `softCanceller.isCancelable()` is true, and both come from `AbstractGarbageCollectionStrategy#getSoftCanceller()` / `getStateSaveTriggerSupplier()`, which return the non-cancelable `Canceller.newCanceller()`. So Oak always takes `compactUp(...)`, the head is never a partially-compacted state, and a live head reference to a strictly-newer compacted segment can't occur in production today. The generation predicate is therefore safe as-is — but I agree it just swaps one fragile proxy (physical position) for another (generation), which is not ideal. Alternative approach in #3135: instead of deciding *which* "future" compacted segments are dangling, skip the future-reclaim in pre-compaction cleanup entirely (behind a flag). `shouldReclaim` then relies only on generational (`isOldDataSegment`) and reference-based (`isUnreferencedBulkSegment`) reclamation. That: - never reclaims a head-referenced (live) segment, so it fixes the promoted-standby SNFE regardless of physical/gc.log ordering, and is immune to both directions of your example; - still collects genuine aborted-compaction leftovers — just via normal generational GC once the head advances past their generation, rather than eagerly. The only trade-off is losing the *eager* reclamation the `aheadOfRoot` heuristic (OAK-3349) provided: on an instance where compaction repeatedly fails and the head never advances, those leftovers linger until a compaction succeeds. It's disk-space only, no correctness impact. This also dovetails with your suggestion to drop partial-compaction support to simplify GC — with partial compaction gone, the "future compacted segment" notion during pre-compaction cleanup largely goes away. Happy to go whichever way you and the others prefer between this PR and #3135. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
