feat(spill): support to create ExternalSortBuffer in WriteBuffer#265
Merged
lxy-9602 merged 7 commits intoalibaba:mainfrom May 9, 2026
Merged
feat(spill): support to create ExternalSortBuffer in WriteBuffer#265lxy-9602 merged 7 commits intoalibaba:mainfrom
lxy-9602 merged 7 commits intoalibaba:mainfrom
Conversation
…spill-to-disk - Refactor MergeTreeWriter constructor to factory method Create() that returns Result<shared_ptr<MergeTreeWriter>>, enabling fallible initialization - Refactor WriteBuffer constructor to factory method Create() that returns Result<unique_ptr<WriteBuffer>> - Add IOManager parameter to support ExternalSortBuffer when spillable is enabled - Add FlushMemory() method to MergeTreeWriter and WriteBuffer for spill support - Change Write() return type from Status to Result<bool> to signal buffer quota - Rename internal Flush() to FlushWriteBuffer() for clarity - Add user_defined_seq_comparator parameter to WriteBuffer::Create - Fix typo: TestWithPosistion -> TestWithPosition - Update all related test cases to use new factory methods
zjw1111
changed the title
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors WriteBuffer and MergeTreeWriter to enable spill-to-disk support by switching to fallible factory initialization, adding IOManager plumbing, and introducing a memory-flush API used by a new writer memory manager.
Changes:
- Replaced
WriteBuffer/MergeTreeWriterconstructors withCreate()factory methods and updated call sites/tests accordingly. - Added spill-related APIs (
FlushMemory(),Write(): Result<bool>) and routed buffering throughSortBuffer(in-memory vs external). - Introduced
WriterMemoryManagerintegration inAbstractFileStoreWriteand added a spill behavior test using.channelfiles.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/paimon/testing/utils/test_helper.h | Adds helper to count spill “channel” files in tests. |
| src/paimon/core/operation/key_value_file_store_write_test.cpp | Adds spill-to-disk behavior test and updates includes. |
| src/paimon/core/operation/key_value_file_store_write.cpp | Switches writer creation to MergeTreeWriter::Create(...) and passes io_manager_. |
| src/paimon/core/operation/abstract_file_store_write.h | Adds WriterMemoryManager member wiring. |
| src/paimon/core/operation/abstract_file_store_write.cpp | Registers/unregisters writers and refreshes memory tracking around writes/commits. |
| src/paimon/core/mergetree/write_buffer_test.cpp | Updates tests to use WriteBuffer::Create() and Result<bool> write API. |
| src/paimon/core/mergetree/write_buffer.h | Introduces factory Create(), FlushMemory(), and new Write() return type. |
| src/paimon/core/mergetree/write_buffer.cpp | Implements spillable SortBuffer selection and forwards Write/FlushMemory. |
| src/paimon/core/mergetree/merge_tree_writer_test.cpp | Migrates to MergeTreeWriter::Create() and parameterizes tests for IOManager presence. |
| src/paimon/core/mergetree/merge_tree_writer.h | Adds Create(), FlushMemory(), and renames internal flush to FlushWriteBuffer(). |
| src/paimon/core/mergetree/merge_tree_writer.cpp | Implements factory construction, spill flush flow, and new write/flush behavior. |
| src/paimon/core/mergetree/lookup_levels_test.cpp | Updates to MergeTreeWriter::Create() and fixes test typo name. |
| src/paimon/core/mergetree/lookup/remote_lookup_file_manager_test.cpp | Updates to MergeTreeWriter::Create(). |
| src/paimon/core/mergetree/compact/lookup_merge_tree_compact_rewriter_test.cpp | Updates to MergeTreeWriter::Create(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zjw1111
changed the title
lxy-9602
reviewed
May 8, 2026
lxy-9602
reviewed
May 8, 2026
lxy-9602
reviewed
May 8, 2026
lxy-9602
reviewed
May 8, 2026
lxy-9602
reviewed
May 8, 2026
Co-authored-by: Copilot <copilot@github.com>
Collaborator
|
+1 |
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.
Purpose
Linked issue: #149
Decide whether to create a spill-capable
ExternalSortBufferinWriteBufferbased on thewrite-buffer-spillableoption andIOManager.Tests
KeyValueFileStoreWriteTest.TestSpillSimple
API and Format
No storage format or protocol changes
Documentation
No documentation changes needed.
Generative AI tooling
Generated-by: Aone Copilot(Claude 4.6) and Github Copilot(GPT-5.4)