refactor: simplify auto compaction planning#126
Merged
Conversation
Li0k
changed the title
Collaborator
|
I think we should consider table total size to determine how long we should compact/eliminate the equality files, since the ultimate goal of compaction it to increase the query performance. There is a trade-off between query performance and compaction cost/resources. So we can categorize iceberg tables based on sizes:
|
Li0k
changed the title
Li0k
force-pushed
the
li0k/improve_full_compaction
branch
from
1be6603 to
d18e313
Compare
There was a problem hiding this comment.
Pull request overview
This PR refactors Auto compaction planning to be snapshot-local and repeat-safe by removing the previous Full fallback and limiting Auto to selecting between localized FilesWithDeletes and SmallFiles plan sets, with a planner-level plan-count budget.
Changes:
- Add
AutoPlanReport/AutoPlanReason/AutoSelectedStrategyand a new report-returning planning API forAuto. - Simplify auto thresholds and snapshot stats (rename to
delete_heavy_files_count, drop impact-ratio + full-fallback config, addmax_auto_plans_per_run). - Add a design/contract document describing compaction strategy boundaries and
Autoplanner semantics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
docs/compaction-strategy-contract.md |
New contract doc describing strategy model, Auto selection order, and budget/return semantics. |
core/src/file_selection/mod.rs |
Rename snapshot stat field to delete_heavy_files_count to match updated semantics. |
core/src/config/mod.rs |
Remove full fallback + impact ratio from auto thresholds; add max_auto_plans_per_run; split auto “resolve” into candidate helpers and update tests. |
core/src/compaction/mod.rs |
Re-export new Auto report/enum types from the compaction module. |
core/src/compaction/auto.rs |
Implement new Auto planning flow that produces a report, selects delete-first then small-files, and caps by max plans. |
core/src/compaction/auto.rs
Outdated
Comment on lines
+151
to
+159
| let delete_report = if let Some(planning_config) = delete_candidate { | ||
| Some(Self::build_report( | ||
| tasks.clone(), | ||
| planning_config, | ||
| to_branch, | ||
| snapshot_id, | ||
| total_data_bytes, | ||
| AutoPlanReason::Recommended, | ||
| )?) |
Comment on lines
+15
to
+17
| - `data file`: a `FileScanTask` where `data_file_content == Data` | ||
| - `delete-heavy`: `deletes.len() >= min_delete_file_count_threshold` | ||
| - `candidate set`: the set of data files that a strategy is allowed to include in compaction |
Comment on lines
+40
to
+44
| - Intended use: timely cleanup of delete-heavy files | ||
| - Candidate set: `deletes.len() >= min_delete_file_count_threshold` | ||
| - May use `group_filters` for group gating | ||
| - `Auto` does not rewrite or override caller-provided group gating for this strategy | ||
| - Must be fixed-point: rewritten delete-heavy input files should leave the candidate set in the newly committed snapshot |
Comment on lines
582
to
596
| #[test] | ||
| fn test_resolve_strategy_priority() { | ||
| fn test_files_with_deletes_candidate_priority() { | ||
| let config = AutoCompactionConfigBuilder::default() | ||
| .thresholds(AutoThresholds { | ||
| min_files_with_deletes_count: 3, | ||
| min_delete_heavy_files_count: 3, | ||
| min_small_files_count: 5, | ||
| min_impact_ratio: None, | ||
| }) | ||
| .build() | ||
| .unwrap(); | ||
|
|
||
| // Priority 1: FilesWithDeletes wins when both thresholds met | ||
| let stats = create_test_stats(10, 6, 4); | ||
| assert!(matches!( | ||
| config.resolve(&stats).unwrap(), | ||
| config.files_with_deletes_candidate(&stats).unwrap(), | ||
| CompactionPlanningConfig::FilesWithDeletes(_) |
There was a problem hiding this comment.
Pull request overview
This PR refactors auto-compaction planning to be snapshot-local and fixed-order, removing the previous “fallback to Full” behavior and documenting the strategy contract.
Changes:
- Add a compaction strategy contract design doc describing
Autoselection semantics, fixed-point expectations, and budget behavior. - Update
AutoCompactionConfig/thresholds to remove impact-ratio + full fallback, addmax_auto_plans_per_run, and rename delete-related stats to “delete-heavy”. - Extend the
AutoCompactionPlannerto return anAutoPlanReport(strategy, costs, reason) and apply the plan-count budget inside the planner.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/compaction-strategy-contract.md | New design doc specifying the simplified strategy model and planner budget semantics. |
| core/src/file_selection/mod.rs | Renames snapshot delete stat to delete_heavy_files_count. |
| core/src/config/mod.rs | Removes full fallback + impact ratio; introduces candidate helpers and max_auto_plans_per_run. |
| core/src/compaction/mod.rs | Re-exports new auto-planning report types. |
| core/src/compaction/auto.rs | Implements report-based planning, fixed-order strategy selection, and budget capping. |
Comment on lines
110
to
+123
| /// Plans compaction for a table branch. | ||
| /// | ||
| /// Returns empty vector if no files need compaction. | ||
| pub async fn plan_compaction_with_branch( | ||
| &self, | ||
| table: &Table, | ||
| to_branch: &str, | ||
| ) -> Result<Vec<CompactionPlan>> { | ||
| let report = self | ||
| .plan_compaction_report_with_branch(table, to_branch) | ||
| .await?; | ||
|
|
||
| Ok(report.plans) | ||
| } |
Comment on lines
+151
to
+168
| let delete_report = if let Some(planning_config) = delete_candidate { | ||
| let report = Self::build_report( | ||
| tasks.clone(), | ||
| planning_config, | ||
| to_branch, | ||
| snapshot_id, | ||
| total_data_bytes, | ||
| AutoPlanReason::Recommended, | ||
| )?; | ||
| if report.plans.is_empty() { | ||
| Some(report) | ||
| } else { | ||
| return Ok(Self::cap_report_plans( | ||
| report, | ||
| total_data_bytes, | ||
| self.config.max_auto_plans_per_run, | ||
| )); | ||
| } |
core/src/compaction/auto.rs
Outdated
Comment on lines
+233
to
+239
| return AutoPlanReport { | ||
| selected_strategy: report.selected_strategy, | ||
| plans: vec![], | ||
| planned_input_bytes: 0, | ||
| planned_input_files: 0, | ||
| rewrite_ratio: 0.0, | ||
| reason: AutoPlanReason::BudgetCapped, |
Comment on lines
+545
to
+556
| if stats.small_files_count >= self.thresholds.min_small_files_count { | ||
| Some(CompactionPlanningConfig::SmallFiles(SmallFilesConfig { | ||
| target_file_size_bytes: self.target_file_size_bytes, | ||
| min_size_per_partition: self.min_size_per_partition, | ||
| max_file_count_per_partition: self.max_file_count_per_partition, | ||
| max_input_parallelism: self.max_input_parallelism, | ||
| max_output_parallelism: self.max_output_parallelism, | ||
| enable_heuristic_output_parallelism: self.enable_heuristic_output_parallelism, | ||
| small_file_threshold_bytes: self.small_file_threshold_bytes, | ||
| grouping_strategy: self.grouping_strategy.clone(), | ||
| })); | ||
| group_filters: self.group_filters.clone(), | ||
| })) |
Collaborator
Author
Hi @chenzl25, Your suggestion is great. I’m thinking: is this also a kind of budget concept? Once we set a daily usable budget, would the fully compaction cycle become longer for tables of different sizes? |
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
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
This PR simplifies
Autocompaction planning so that it stays safe under repeated invocation without relying onFullas a fallback.Autonow only plans localized rewrites from two strategy families:FilesWithDeletesSmallFilesIt scans the current snapshot once, evaluates those candidates in a fixed order, and applies a planner-level plan budget before returning executable plans.
This PR also intentionally updates the pre-release
AutoAPI/config surface to match the new planner contract.Design Changes
1. Remove Auto fallback to
FullAutono longer falls back toFullwhen specialized strategies do not match.This avoids repeatedly rewriting healthy parquet files that are already near
target_file_sizeunder high-frequency calls.Fullremains available as an explicit/manual full-table rewrite strategy.2. Simplify Auto planning
Autonow follows a fixed planning flow:FilesWithDeletescandidate reportSmallFilescandidate reportmax_auto_plans_per_runto the selected plan setThis replaces the previous candidate-chain / anti-starvation / full-like branching logic.
3. Keep planner-level observability
AutoPlanReportis introduced as the planner output model and reflects the final selected or capped plan set:selected_strategyplansplanned_input_bytesplanned_input_filesrewrite_ratioreasonThe
reasonnow distinguishes:RecommendedBudgetCappedNoSnapshotNoCandidateNoPlansProduced4. Preserve caller-owned group gating
For
Auto-selectedFilesWithDeletes, caller-providedgroup_filtersare propagated unchanged.Autois responsible for choosing between localized strategies, but it does not override caller policy such asmin_group_size_bytes.5. Clarify zero-threshold behavior
Under
Auto,min_delete_file_count_threshold == 0disables delete-heavy detection and therefore disables theFilesWithDeletescandidate.Likewise, a zero-valued auto threshold disables the corresponding candidate instead of forcing it to match every snapshot.
6. Update the pre-release Auto config/API surface
This PR intentionally changes the not-yet-stable
Autoconfig model:enable_full_fallbackmin_impact_ratiomin_files_with_deletes_counttomin_delete_heavy_files_countmax_auto_plans_per_runas a planner-level budgetplan_compaction_report_with_branch()for callers that need structured planning reasons7. Add a design document
Added:
docs/compaction-strategy-contract.mdThe document describes the current strategy model, planner budget semantics, return semantics, and the responsibility split between the library and external systems.
Reviewer Focus
Please focus on:
Automodel is clear enoughmax_auto_plans_per_runis the right planner-level budget boundaryAutoAutoAPI/config changes feel coherent with the new planner contractValidation
cargo test -p iceberg-compaction-core --libcargo clippy -p iceberg-compaction-core --lib -- -D warnings