feat(compaction): support compaction for key table in framework by lucasfang · Pull Request #195 · alibaba/paimon-cpp

lucasfang · 2026-03-23T01:39:57Z

Purpose

Add key-table compaction support in framework.
Follow up with stability and maintainability fixes, including full-compaction behavior, cancellation wiring, logging improvements, and related refactor/cleanup.

TODO: support lookup mode framework.

Tests

UT
- MergeTreeCompactManagerTest
  - TestOutputToZeroLevel
  - TestCompactToPenultimateLayer
  - TestNoCompaction
  - TestNormal
  - TestUpgrade
  - TestSmallFiles
  - TestSmallFilesNoCompact
  - TestSmallFilesCrossLevel
  - TestComplex
  - TestSmallInComplex
  - TestIsCompacting
  - TestTriggerFullCompaction
  - TestRejectReentrantFullCompaction
IT
- KeyValueCompactionInteTest
  - TestKeyValueTableCompactionWithIOException
  - TestKeyValueTableStreamWriteFullCompaction (parameterized by file format)
- AppendCompactionInteTest
  - TestAppendTableStreamWriteFullCompaction (parameterized by file format)
  - TestAppendTableStreamWriteFullCompactionWithDv (parameterized by file format)
  - TestAppendTableStreamWriteBestEffortCompaction (parameterized by file format)
  - TestAppendTableStreamWriteCompactionWithExternalPath (parameterized by file format)
  - TestAppendTableCompactionWithIOException

API and Format

Add options for pk compaction.

compaction.max-size-amplification-percent
compaction.size-ratio
num-sorted-run.compaction-trigger
num-sorted-run.stop-trigger
num-levels
lookup-compact
compaction.force-up-level-0
lookup-compact.max-interval

Generative AI tooling

Generated-by: GitHub Copilot (GPT-5.3-Codex)

Copilot

Pull request overview

Adds merge-tree compaction support for key-table writes in the C++ framework by wiring a real CompactManager into MergeTreeWriter, introducing new compaction/lookup-related options, and adding unit/integration tests around the new behavior.

Changes:

Implement merge-tree compaction orchestration (MergeTreeCompactManager + tasks) and integrate it into KeyValueFileStoreWrite/MergeTreeWriter.
Extend compaction configuration via new CoreOptions fields (sorted-run triggers, levels, lookup compact mode/interval, force up-level-0).
Add/adjust tests (unit + integration) and update interfaces to propagate Status/Result for compaction-related operations.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/inte/key_value_compaction_inte_test.cpp	New integration test exercising key-table compaction under injected IO failures.
test/inte/CMakeLists.txt	Registers the new integration test target.
src/paimon/core/options/lookup_strategy_test.cpp	Unit tests for `LookupStrategy::From` behavior and combinations.
src/paimon/core/options/lookup_strategy.h	Introduces `LookupStrategy` helper for deriving `need_lookup`.
src/paimon/core/options/lookup_compact_mode.h	Adds `LookupCompactMode` enum used by lookup compaction strategy selection.
src/paimon/core/operation/key_value_file_store_write.h	Extends writer to accept a comparator suitable for compaction and exposes compaction factory helpers.
src/paimon/core/operation/key_value_file_store_write.cpp	Creates `Levels`, compaction strategy, manager, and rewriter; wires compaction manager into `MergeTreeWriter`.
src/paimon/core/operation/file_store_write.cpp	Builds separate comparators (`use_view=true/false`) and DV maintainer factory; passes into key-value write path.
src/paimon/core/operation/abstract_file_store_write.cpp	Propagates `Result` from `CompactDeletionFile::GetOrCompute()` into prepare-commit flow.
src/paimon/core/mergetree/merge_tree_writer_test.cpp	Updates tests for new `MergeTreeWriter` ctor requiring a `CompactManager` (uses `NoopCompactManager`).
src/paimon/core/mergetree/merge_tree_writer.h	Adds `Compact/Sync/CompactNotCompleted` implementations and compaction state tracking members.
src/paimon/core/mergetree/merge_tree_writer.cpp	Implements compaction wiring: sync results, trigger compaction, track compact before/after, handle deletion files.
src/paimon/core/mergetree/lookup_levels_test.cpp	Updates test construction to pass `NoopCompactManager` into `MergeTreeWriter`.
src/paimon/core/mergetree/compact/merge_tree_compact_task.h	New compaction task for merge-tree compaction (rewrite/upgrade decisions + optional DV deletion file).
src/paimon/core/mergetree/compact/merge_tree_compact_manager_test.cpp	New unit tests validating `MergeTreeCompactManager` level/output behavior and lookup compaction “not completed” semantics.
src/paimon/core/mergetree/compact/merge_tree_compact_manager.h	New `MergeTreeCompactManager` interface/implementation declaration.
src/paimon/core/mergetree/compact/merge_tree_compact_manager.cpp	Implements compaction triggering, task submission, applying results to `Levels`, and metrics reporting.
src/paimon/core/mergetree/compact/interval_partition.h	Removes redundant forward declarations (relies on includes).
src/paimon/core/mergetree/compact/file_rewrite_compact_task.h	New task for file-rewrite style compaction units.
src/paimon/core/core_options_test.cpp	Adds coverage for new compaction/lookup options parsing/defaults and lookup strategy derivation.
src/paimon/core/core_options.h	Adds new getters for compaction knobs and lookup strategy/mode/levels.
src/paimon/core/core_options.cpp	Parses/stores new options, computes derived defaults, and centralizes `NeedLookup()` via `LookupStrategy`.
src/paimon/core/compact/noop_compact_manager.h	Updates `AddNewFile` signature to return `Status`.
src/paimon/core/compact/compact_manager.h	Changes `AddNewFile` to return `Status` for error propagation.
src/paimon/core/compact/compact_deletion_file_test.cpp	Adapts tests to `Result<optional<...>>` API and adds tests for lazy generation behavior.
src/paimon/core/compact/compact_deletion_file.h	Adds lazy deletion-file generation and changes `GetOrCompute()` to return `Result<optional<...>>`.
src/paimon/core/append/bucketed_append_compact_manager.h	Updates `AddNewFile` signature to return `Status`.
src/paimon/core/append/append_only_writer.cpp	Propagates `Status` from `CompactManager::AddNewFile`.
src/paimon/common/defs.cpp	Adds option key constants for new compaction/lookup configuration options.
src/paimon/CMakeLists.txt	Adds new sources and unit tests to the build.
include/paimon/defs.h	Documents new compaction/lookup configuration keys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/paimon/core/mergetree/merge_tree_writer.cpp

src/paimon/core/compact/compact_deletion_file.h

include/paimon/defs.h

test/inte/key_value_compaction_inte_test.cpp

src/paimon/core/mergetree/compact/merge_tree_compact_manager_test.cpp

include/paimon/defs.h

src/paimon/core/compact/compact_deletion_file_test.cpp

lxy-9602 · 2026-03-24T05:52:47Z

src/paimon/core/mergetree/compact/file_rewrite_compact_task.h

+        auto result = std::make_shared<CompactResult>();
+        for (const auto& file : files_) {
+            PAIMON_RETURN_NOT_OK(RewriteFile(file, result.get()));
+        }


Will RewriteFile be called by other functions in the future?
I'm trying to understand the purpose of the to_update parameter — currently, it seems like we could just return rewriter_->Rewrite() directly without merging.

lxy-9602 · 2026-03-24T05:55:52Z

src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter_test.cpp

            std::make_shared<ReducerMergeFunctionWrapper>(std::move(mfunc));

+        auto cancellation_controller = std::make_shared<CancellationController>();
        auto writer = std::make_shared<MergeTreeWriter>(


Seems no need for line 94 in NewFiles?

lxy-9602 · 2026-03-24T06:05:36Z

src/paimon/core/mergetree/compact/merge_tree_compact_manager.h

+                            const std::shared_ptr<BucketedDvMaintainer>& dv_maintainer,
+                            bool lazy_gen_deletion_file, bool need_lookup,
+                            bool force_rewrite_all_files, bool force_keep_delete,
+                            const std::shared_ptr<CancellationController>& cancellation_controller);


Could executor be placed at the end of the parameter list, similar to memory_pool?

Could we consolidate these parameters into CoreOptions and destructure them in the MergeTreeCompactManager constructor?
Alternatively, should MergeTreeCompactManager just hold a reference to CoreOptions directly?

The current parameter list is getting quite long. TT

lxy-9602 · 2026-03-24T06:11:38Z

src/paimon/core/mergetree/compact/merge_tree_compact_manager.cpp

+namespace paimon {
+
+std::string RunsToString(const std::vector<LevelSortedRun>& runs) {
+    std::stringstream ss;


See VectorToString in StringUtils.

lxy-9602 · 2026-03-24T07:08:28Z

src/paimon/core/mergetree/compact/merge_tree_compact_task.cpp

+}
+
+Status MergeTreeCompactTask::Rewrite(std::vector<std::vector<SortedRun>>& candidate,
+                                     CompactResult* to_update) {


Please use std::vector<std::vector>* for out param.

lxy-9602 · 2026-03-24T07:28:44Z

src/paimon/core/mergetree/merge_tree_writer.h


 private:
    Status DoClose() {
+        // cancel compaction so that it does not block job cancelling


Consider put DoClose to .cpp?

Now where call DoClose as ~MergeTreeWriter() is deleted

lxy-9602 · 2026-03-24T07:49:15Z

src/paimon/core/operation/key_value_file_store_write.cpp

+    (void)key_comparator;
+    (void)user_defined_seq_comparator;
+    (void)levels;
+    LookupStrategy lookup_strategy = options_.GetLookupStrategy();


Is there a compile error for (void)key_comparator;?

src/paimon/core/operation/key_value_file_store_write.cpp

src/paimon/core/options/lookup_compact_mode.h

lszskye · 2026-03-24T08:08:20Z

src/paimon/core/options/lookup_strategy.h

+    const bool deletion_vector;
+
+ private:
+    LookupStrategy(bool is_first_row, bool produce_changelog, bool deletion_vector,


For readability, please distinguish the names of function parameters from the names of class members（e.g. please change is_first_row to _is_first_row）

Copilot AI review requested due to automatic review settings March 23, 2026 01:39

Copilot started reviewing on behalf of lucasfang March 23, 2026 01:40 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

lucasfang force-pushed the compaction_dev branch from 61ec1d8 to fad0bb6 Compare March 23, 2026 07:09

lszskye reviewed Mar 24, 2026

View reviewed changes

include/paimon/defs.h Show resolved Hide resolved

lucasfang force-pushed the compaction_dev branch from e091189 to 31cb7f9 Compare March 24, 2026 01:49

lucasfang added 11 commits March 24, 2026 10:41

feat(compaction): support compaction for key table in framework

9d3a0f1

fix

bfa2117

fix

0c404a6

fix

857000d

fix

40f4ecb

fix

3b66551

fix

6eb3bc8

fix

30d29aa

fix

4b7c16f

fix

1978d61

fix

5a90235

lucasfang force-pushed the compaction_dev branch from 7ffe066 to 5a90235 Compare March 24, 2026 03:51

lxy-9602 reviewed Mar 24, 2026

View reviewed changes

lszskye reviewed Mar 24, 2026

View reviewed changes

Conversation

lucasfang commented Mar 23, 2026 • edited by lxy-9602 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Tests

API and Format

Generative AI tooling

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lucasfang commented Mar 23, 2026 •

edited by lxy-9602

Loading