Skip to content

Commit a19ec32

Browse files
branch-4.1: [improvement](be) Limit packed file writes to rowset first segment #62342 (#62508)
Cherry-picked from #62342 Co-authored-by: Xin Liao <liaoxin@selectdb.com>
1 parent f1f4850 commit a19ec32

2 files changed

Lines changed: 93 additions & 0 deletions

File tree

be/src/io/fs/packed_file_system.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
#include "io/fs/packed_file_system.h"
1919

20+
#include <string_view>
2021
#include <utility>
2122

2223
#include "common/status.h"
@@ -26,6 +27,42 @@
2627

2728
namespace doris::io {
2829

30+
namespace {
31+
32+
// Only keep packed-file aggregation for the first segment in a rowset.
33+
// The path handled here is expected to look like:
34+
// local/remote segment file: .../{rowset_id}_{segment_id}.dat
35+
// local/remote index file: .../{rowset_id}_{segment_id}.idx
36+
// The .idx case here is V2 only. V1 inverted-index tablets are filtered before
37+
// PackedFileSystem is enabled, so V1 names like
38+
// {rowset_id}_{segment_id}_{index_id}@{suffix}.idx never reach this helper.
39+
// Multi-segment rowsets usually come from large loads or memory-pressure flushes,
40+
// and continuing to buffer later segments in packed files can amplify memory usage.
41+
// Non-rowset file names keep the legacy behavior to avoid changing unrelated callers.
42+
bool should_use_packed_writer(std::string_view file_name) {
43+
constexpr std::string_view kSegmentSuffix = ".dat";
44+
constexpr std::string_view kIndexSuffix = ".idx";
45+
46+
size_t suffix_len = 0;
47+
if (file_name.ends_with(kSegmentSuffix)) {
48+
suffix_len = kSegmentSuffix.size();
49+
} else if (file_name.ends_with(kIndexSuffix)) {
50+
suffix_len = kIndexSuffix.size();
51+
} else {
52+
return true;
53+
}
54+
55+
file_name.remove_suffix(suffix_len);
56+
size_t pos = file_name.rfind('_');
57+
if (pos == std::string_view::npos || pos + 1 >= file_name.size()) {
58+
return true;
59+
}
60+
61+
return file_name.substr(pos + 1) == "0";
62+
}
63+
64+
} // namespace
65+
2966
PackedFileSystem::PackedFileSystem(FileSystemSPtr inner_fs, PackedAppendContext append_info)
3067
: FileSystem(inner_fs->id(), inner_fs->type()),
3168
_inner_fs(std::move(inner_fs)),
@@ -54,6 +91,11 @@ Status PackedFileSystem::create_file_impl(const Path& file, FileWriterPtr* write
5491
FileWriterPtr inner_writer;
5592
RETURN_IF_ERROR(_inner_fs->create_file(file, &inner_writer, opts));
5693

94+
if (!should_use_packed_writer(file.filename().native())) {
95+
*writer = std::move(inner_writer);
96+
return Status::OK();
97+
}
98+
5799
// Wrap with PackedFileWriter
58100
*writer = std::make_unique<PackedFileWriter>(std::move(inner_writer), file, _append_info);
59101
return Status::OK();

be/test/io/fs/packed_file_system_test.cpp

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,57 @@ TEST_F(PackedFileSystemTest, CreateFileWrapsWithPackedFileWriter) {
233233
EXPECT_TRUE(st.ok());
234234
}
235235

236+
TEST_F(PackedFileSystemTest, FirstSegmentDataFileUsesPackedWriter) {
237+
PackedFileSystem merge_fs(_inner_fs, _append_info);
238+
239+
Path file_path("rowset_1_0.dat");
240+
FileWriterPtr writer;
241+
ASSERT_TRUE(merge_fs.create_file(file_path, &writer, nullptr).ok());
242+
ASSERT_NE(writer, nullptr);
243+
244+
std::string data = "test";
245+
Slice slice(data);
246+
ASSERT_TRUE(writer->appendv(&slice, 1).ok());
247+
248+
ASSERT_NE(_inner_fs->last_writer(), nullptr);
249+
EXPECT_EQ(_inner_fs->last_writer()->bytes_appended(), 0);
250+
EXPECT_TRUE(writer->is_in_packed_file());
251+
}
252+
253+
TEST_F(PackedFileSystemTest, LaterSegmentDataFileUsesDirectWriter) {
254+
PackedFileSystem merge_fs(_inner_fs, _append_info);
255+
256+
Path file_path("rowset_1_1.dat");
257+
FileWriterPtr writer;
258+
ASSERT_TRUE(merge_fs.create_file(file_path, &writer, nullptr).ok());
259+
ASSERT_NE(writer, nullptr);
260+
261+
std::string data = "test";
262+
Slice slice(data);
263+
ASSERT_TRUE(writer->appendv(&slice, 1).ok());
264+
265+
ASSERT_NE(_inner_fs->last_writer(), nullptr);
266+
EXPECT_EQ(_inner_fs->last_writer()->bytes_appended(), data.size());
267+
EXPECT_FALSE(writer->is_in_packed_file());
268+
}
269+
270+
TEST_F(PackedFileSystemTest, LaterSegmentIndexFileUsesDirectWriter) {
271+
PackedFileSystem merge_fs(_inner_fs, _append_info);
272+
273+
Path file_path("rowset_1_2.idx");
274+
FileWriterPtr writer;
275+
ASSERT_TRUE(merge_fs.create_file(file_path, &writer, nullptr).ok());
276+
ASSERT_NE(writer, nullptr);
277+
278+
std::string data = "idx";
279+
Slice slice(data);
280+
ASSERT_TRUE(writer->appendv(&slice, 1).ok());
281+
282+
ASSERT_NE(_inner_fs->last_writer(), nullptr);
283+
EXPECT_EQ(_inner_fs->last_writer()->bytes_appended(), data.size());
284+
EXPECT_FALSE(writer->is_in_packed_file());
285+
}
286+
236287
TEST_F(PackedFileSystemTest, OpenFileNotInMergeFile) {
237288
PackedFileSystem merge_fs(_inner_fs, _append_info);
238289

0 commit comments

Comments
 (0)