Skip to content

Commit d19b390

Browse files
zjw1111claude
andauthored
feat(spill): optimize external sort with SpillFileMerger to reduce write amplification (alibaba#288)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 40f5148 commit d19b390

43 files changed

Lines changed: 1135 additions & 243 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/code-style.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,21 @@ This document defines the coding conventions for the paimon-cpp project. All pul
2828

2929
---
3030

31+
## Integer Types
32+
33+
Use **fixed-width integer types** from `<cstdint>`. Do **not** use plain `int`, `long`, `short`, or `unsigned`.
34+
35+
| Use | Instead of |
36+
|-----|-----------|
37+
| `int8_t` / `uint8_t` | `char` (for numeric data) |
38+
| `int16_t` / `uint16_t` | `short` |
39+
| `int32_t` / `uint32_t` | `int` / `unsigned int` |
40+
| `int64_t` / `uint64_t` | `long` / `long long` |
41+
42+
**Exception**: Loop variables iterating over small, bounded ranges (e.g. `for (int32_t i = 0; i < n; ++i)`) must still use `int32_t`, not `int`.
43+
44+
---
45+
3146
## Formatting
3247

3348
Formatting is based on **Google C++ Style** with the following overrides (defined in `.clang-format`):

include/paimon/defs.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ struct PAIMON_EXPORT Options {
212212
static const char SNAPSHOT_TIME_RETAINED[];
213213

214214
/// "snapshot.expire.limit" - The maximum number of snapshots allowed to expire at a time.
215-
/// Default value is 10.
215+
/// Default value is 50.
216216
static const char SNAPSHOT_EXPIRE_LIMIT[];
217217

218218
/// "snapshot.clean-empty-directories" - Whether to try to clean empty directories when expiring
@@ -391,7 +391,7 @@ struct PAIMON_EXPORT Options {
391391
/// reading the audit_log or binlog system tables. This is only valid for primary key tables.
392392
/// Default value is "false".
393393
static const char TABLE_READ_SEQUENCE_NUMBER_ENABLED[];
394-
/// "key-value.sequence-number.enabled" - Whether to include the _SEQUENCE_NUMBER field when
394+
/// "key-value.sequence_number.enabled" - Whether to include the _SEQUENCE_NUMBER field when
395395
/// reading key-value data. This is an internal option used by AuditLogTable and BinlogTable
396396
/// when table-read.sequence-number.enabled is set to true. Default value is "false".
397397
static const char KEY_VALUE_SEQUENCE_NUMBER_ENABLED[];

include/paimon/write_context.h

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ class PAIMON_EXPORT WriteContext {
4040
public:
4141
WriteContext(const std::string& root_path, const std::string& commit_user,
4242
bool is_streaming_mode, bool ignore_num_bucket_check, bool ignore_previous_files,
43-
const std::optional<int32_t>& write_id, const std::string& branch,
44-
const std::vector<std::string>& write_schema,
43+
bool enable_multi_thread_spill, const std::optional<int32_t>& write_id,
44+
const std::string& branch, const std::vector<std::string>& write_schema,
4545
const std::shared_ptr<MemoryPool>& memory_pool,
4646
const std::shared_ptr<Executor>& executor, const std::string& temp_directory,
4747
const std::shared_ptr<FileSystem>& specific_file_system,
@@ -106,13 +106,18 @@ class PAIMON_EXPORT WriteContext {
106106
return specific_file_system_;
107107
}
108108

109+
bool EnableMultiThreadSpill() const {
110+
return enable_multi_thread_spill_;
111+
}
112+
109113
private:
110114
std::string root_path_;
111115
std::string commit_user_;
112116
std::string branch_;
113117
bool is_streaming_mode_;
114118
bool ignore_num_bucket_check_;
115119
bool ignore_previous_files_;
120+
bool enable_multi_thread_spill_;
116121
std::optional<int32_t> write_id_;
117122
std::vector<std::string> write_schema_;
118123
std::shared_ptr<MemoryPool> memory_pool_;
@@ -222,6 +227,13 @@ class PAIMON_EXPORT WriteContextBuilder {
222227
WriteContextBuilder& WithFileSystemSchemeToIdentifierMap(
223228
const std::map<std::string, std::string>& fs_scheme_to_identifier_map);
224229

230+
/// Set the thread number for write buffer spill operations. (default is 0)
231+
/// If <= 0, threading is disabled for spill IPC read/write.
232+
/// If > 0, sets arrow CPU thread pool capacity for spill operations.
233+
/// @param thread_number The thread number to use for spill operations.
234+
/// @return Reference to this builder for method chaining.
235+
WriteContextBuilder& SetWriteBufferSpillThreadNumber(int32_t thread_number);
236+
225237
/// Build and return a `WriteContext` instance with input validation.
226238
/// @return Result containing the constructed `WriteContext` or an error status.
227239
Result<std::unique_ptr<WriteContext>> Finish();

src/paimon/CMakeLists.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ set(PAIMON_CORE_SRCS
255255
core/mergetree/merge_tree_writer.cpp
256256
core/mergetree/in_memory_sort_buffer.cpp
257257
core/mergetree/external_sort_buffer.cpp
258+
core/mergetree/spill_file_merger.cpp
258259
core/mergetree/write_buffer.cpp
259260
core/mergetree/levels.cpp
260261
core/mergetree/lookup_file.cpp
@@ -651,6 +652,7 @@ if(PAIMON_BUILD_TESTS)
651652
core/mergetree/merge_tree_writer_test.cpp
652653
core/mergetree/write_buffer_test.cpp
653654
core/mergetree/sort_buffer_test.cpp
655+
core/mergetree/spill_file_merger_test.cpp
654656
core/mergetree/sorted_run_test.cpp
655657
core/mergetree/spill_channel_manager_test.cpp
656658
core/mergetree/spill_reader_writer_test.cpp

src/paimon/common/defs.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ const char Options::GLOBAL_INDEX_THREAD_NUM[] = "global-index.thread-num";
9898
const char Options::GLOBAL_INDEX_EXTERNAL_PATH[] = "global-index.external-path";
9999
const char Options::AGGREGATION_REMOVE_RECORD_ON_DELETE[] = "aggregation.remove-record-on-delete";
100100
const char Options::TABLE_READ_SEQUENCE_NUMBER_ENABLED[] = "table-read.sequence-number.enabled";
101-
const char Options::KEY_VALUE_SEQUENCE_NUMBER_ENABLED[] = "key-value.sequence-number.enabled";
101+
const char Options::KEY_VALUE_SEQUENCE_NUMBER_ENABLED[] = "key-value.sequence_number.enabled";
102102
const char Options::SCAN_TIMESTAMP_MILLIS[] = "scan.timestamp-millis";
103103
const char Options::SCAN_TIMESTAMP[] = "scan.timestamp";
104104
const char Options::SCAN_TAG_NAME[] = "scan.tag-name";

src/paimon/common/fs/file_system_test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class FileSystemTest : public ::testing::Test, public ::testing::WithParamInterf
173173
std::string test_root_;
174174
};
175175

176-
TEST_P(FileSystemTest, TestNoneFileSystemFactory) {
176+
TEST(FileSystemStaticTest, TestNoneFileSystemFactory) {
177177
std::map<std::string, std::string> fs_options;
178178
Result<std::unique_ptr<FileSystem>> fs =
179179
FileSystemFactory::Get("not exist", "/tmp", fs_options);
@@ -516,7 +516,7 @@ TEST_P(FileSystemTest, TestReadAndWriteAndAtomicStoreFile) {
516516
ASSERT_EQ(read_content, new_content);
517517
}
518518

519-
TEST_P(FileSystemTest, TestIsObjectStore) {
519+
TEST(FileSystemStaticTest, TestIsObjectStore) {
520520
ASSERT_OK_AND_ASSIGN(bool is_object_store, FileSystem::IsObjectStore("file:///tmp/test.txt"));
521521
ASSERT_FALSE(is_object_store);
522522
ASSERT_OK_AND_ASSIGN(is_object_store, FileSystem::IsObjectStore("/tmp/test.txt"));

src/paimon/common/global_index/btree/btree_global_index_integration_test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1491,7 +1491,7 @@ TEST_P(BTreeGlobalIndexIntegrationTest, WriteAndReadAllNull) {
14911491
}
14921492
}
14931493

1494-
TEST_P(BTreeGlobalIndexIntegrationTest, WriteAndReadLargeDataWithSmallBlocks) {
1494+
TEST_F(BTreeGlobalIndexIntegrationTest, WriteAndReadLargeDataWithSmallBlocks) {
14951495
// Use very small block size and cache size to force multiple block evictions
14961496
auto file_writer = std::make_shared<FakeGlobalIndexFileWriter>(fs_, base_path_);
14971497
auto field = arrow::field("int_field", arrow::int32());
@@ -1710,7 +1710,7 @@ TEST_P(BTreeGlobalIndexIntegrationTest, CreateReaderWithMultiFieldSchema) {
17101710
"supposed to have single field");
17111711
}
17121712

1713-
TEST_P(BTreeGlobalIndexIntegrationTest, CreateWriterWithMissingField) {
1713+
TEST_F(BTreeGlobalIndexIntegrationTest, CreateWriterWithMissingField) {
17141714
auto file_writer = std::make_shared<FakeGlobalIndexFileWriter>(fs_, base_path_);
17151715
auto type = arrow::struct_({arrow::field("existing_field", arrow::int32())});
17161716
auto struct_type = std::dynamic_pointer_cast<arrow::StructType>(type);

src/paimon/core/core_options.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -583,8 +583,8 @@ struct CoreOptions::Impl {
583583
int32_t snapshot_num_retain_min = 10;
584584
// Parse snapshot.num-retained.max - maximum completed snapshots to retain
585585
int32_t snapshot_num_retain_max = std::numeric_limits<int32_t>::max();
586-
// Parse snapshot.expire.limit - maximum snapshots allowed to expire at a time, default 10
587-
int32_t snapshot_expire_limit = 10;
586+
// Parse snapshot.expire.limit - maximum snapshots allowed to expire at a time, default 50
587+
int32_t snapshot_expire_limit = 50;
588588
// Parse snapshot.time-retained - maximum time of completed snapshots to retain
589589
int64_t snapshot_time_retained = 1 * 3600 * 1000; // 1 hour
590590
PAIMON_RETURN_NOT_OK(
@@ -642,7 +642,7 @@ struct CoreOptions::Impl {
642642
// Parse table-read.sequence-number.enabled - expose sequence number in system tables
643643
PAIMON_RETURN_NOT_OK(parser.Parse<bool>(Options::TABLE_READ_SEQUENCE_NUMBER_ENABLED,
644644
&table_read_sequence_number_enabled));
645-
// Parse key-value.sequence-number.enabled - internal sequence number read switch
645+
// Parse key-value.sequence_number.enabled - internal sequence number read switch
646646
PAIMON_RETURN_NOT_OK(parser.Parse<bool>(Options::KEY_VALUE_SEQUENCE_NUMBER_ENABLED,
647647
&key_value_sequence_number_enabled));
648648
// Parse partial-update.remove-record-on-sequence-group

src/paimon/core/core_options_test.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ TEST(CoreOptionsTest, TestDefaultValue) {
7171
ExpireConfig expire_config = core_options.GetExpireConfig();
7272
ASSERT_EQ(10, expire_config.GetSnapshotRetainMin());
7373
ASSERT_EQ(std::numeric_limits<int32_t>::max(), expire_config.GetSnapshotRetainMax());
74-
ASSERT_EQ(10, expire_config.GetSnapshotMaxDeletes());
74+
ASSERT_EQ(50, expire_config.GetSnapshotMaxDeletes());
7575
ASSERT_FALSE(expire_config.CleanEmptyDirectories());
7676
ASSERT_EQ(1 * 3600 * 1000L, expire_config.GetSnapshotTimeRetainMs());
7777
ASSERT_EQ(std::vector<std::string>(), core_options.GetSequenceField());

src/paimon/core/disk/file_io_channel.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
#pragma once
1818

1919
#include <cstdint>
20-
#include <memory>
2120
#include <random>
2221
#include <string>
2322

@@ -68,4 +67,9 @@ class PAIMON_EXPORT FileIOChannel {
6867
};
6968
};
7069

70+
struct FileChannelInfo {
71+
FileIOChannel::ID channel_id;
72+
int64_t file_size;
73+
};
74+
7175
} // namespace paimon

0 commit comments

Comments
 (0)