feat: add early termination for compaction plan with max_compaction_bytes option#6890
feat: add early termination for compaction plan with max_compaction_bytes option#6890Jay-ju wants to merge 5 commits into
Conversation
…ytes option Add budget-based early termination to DefaultCompactionPlanner to prevent OOM when planning compaction on datasets with many fragments (e.g., hundreds of thousands). Changes: - Add max_compaction_bytes option to CompactionOptions - Refactor max_source_fragments from post-hoc truncation to in-loop early termination, stopping fragment metrics collection once budget is exceeded - Add exceeds_budget() helper checking both fragment count and byte limits during the planning loop - Update Python bindings and TypedDict docs - Add functional tests for early termination behavior - Add benchmark tests for plan performance at scale Closes: lance-format#6039
…imits - Add apply_budget_limits() for strict post-hoc truncation on task list - Move early termination check before fragment is added to bin - Guarantee at least 1 task is always included - Fix test_max_source_fragments CI failure
- Fix Issue 1: Remove first-task exemption in apply_budget_limits, budget is now a strict hard limit (0 tasks if first task exceeds it) - Fix Issue 2: Early termination now tracks effective (non-noop) candidate fragments only, preventing budget waste on bins that will be filtered by is_noop() - Fix Issue 3: Mark benchmark tests as #[ignore] to reduce CI cost - Update docs to clarify hard-limit semantics
|
@claude review |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The test uses IVF with 2 partitions but default nprobes=1, which only probes 1 partition per segment. With delta indices (2 segments), the search may miss the partition containing ID 0 in the first segment, causing the assertion to fail non-deterministically (e.g., returning [889, 1000] instead of [0, 1000]). Setting nprobes=2 ensures all partitions are probed, making the search exhaustive and the test deterministic.
|
Hi @hamersaw. Fragment planning consumes much time in large data scenarios. I have discussed with @zhangyue19921010 . Based on the discussion of #6039 , we revised the original logic of full planning followed by trimming to on-demand planning. Planning will stop once reaching the threshold. Could you take a look when you have time? |
|
@claude review |
There was a problem hiding this comment.
An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.
Once the cap resets or is raised, comment @claude review on this pull request to trigger a review.
| (Some(candidacy), Some(bin)) => { | ||
| // We cannot mix "indexed" and "non-indexed" fragments and so we only consider | ||
| // the existing bin if it contains the same indices | ||
| if bin.indices == indices { |
There was a problem hiding this comment.
If we match this branch, the early termination would never happen.
| } | ||
|
|
||
| #[tokio::test] | ||
| #[ignore] |
There was a problem hiding this comment.
Many ignored test cases, used for benchmark?
…e benchmark tests The early termination check was missing in the branch where a fragment is appended to an existing bin with matching indices. In the common case of an unindexed dataset, all fragments share the same (empty) indices list and always take this path, so the budget was never enforced during the loop -- only post-hoc via apply_budget_limits, defeating the PR's goal of reducing I/O and memory. Now, after appending a fragment to an existing bin, we compute the running fragment/byte counts and break immediately if the budget is exceeded. Also removes three #[ignore] benchmark tests that are not suitable for CI per project guidelines.
Summary
Add budget-based early termination to
DefaultCompactionPlannerto prevent OOM when planning compaction on datasets with many fragments (e.g., hundreds of thousands).Closes: #6039
Problem
When a dataset has hundreds of thousands of fragments,
plan_compactioncollects metrics for all fragments before producing the plan. This leads to:The existing
max_source_fragmentsoption was a post-hoc truncation — it collected all metrics first, then truncated the output. This did not reduce planning time or memory.Benchmark data (10K fragments, no deletions):
Plan time barely changed because all metrics were still collected.
Solution
Refactor
max_source_fragmentsfrom post-hoc truncation to in-loop early termination, and add a newmax_compaction_bytesoption. The planner now trackstotal_candidate_fragmentsandtotal_candidate_bytesduring the metrics collection loop and breaks out as soon as either budget is exceeded.Key changes:
max_source_fragments: Now terminates metrics collection early (was post-hoc truncation)max_compaction_bytes: New option to limit by cumulative fragment byte sizeexceeds_budget(): Helper method checking both limits during the planning loop.buffered(io_parallelism())) — unlike PR feat: support bounded compaction planner #6095 which used serial I/ODesign Rationale
This approach follows hamersaw's review feedback on PR #6095: extending
CompactionOptionsrather than adding a newBoundedCompactionPlannertype. Users configure limits directly without needing to choose a planner implementation.Changes
Rust
CompactionOptions: Addmax_compaction_bytes: Option<usize>fieldDefaultCompactionPlanner::plan(): Replace post-hoc truncation with in-loop early terminationDefaultCompactionPlanner::exceeds_budget(): New helper methodCompactionOptions::apply_dataset_config(): Supportlance.compaction.max_compaction_bytesPython
CompactionOptionsTypedDict: Addmax_compaction_bytesfield with docsmax_compaction_byteskeyUsage
Comparison with PR #6095
BoundedCompactionPlannertypeDefaultCompactionPlannerplanner="bounded"+ limitsmax_*options