This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 9e4b43ca578 branch-4.2: [fix](packed-file) Honor write_file_cache
option for packed small files #69693 (#70114)
9e4b43ca578 is described below
commit 9e4b43ca578d799725bd3cc533e04f47b2b2491f
Author: Xin Liao <[email protected]>
AuthorDate: Wed Nov 4 08:33:01 2026 +0800
branch-4.2: [fix](packed-file) Honor write_file_cache option for packed
small files #69693 (#70114)
### What problem does this PR solve?
Issue Number: None
Related PR: #69693
Problem Summary:
Backport #69693 to `branch-4.2`.
With packed files enabled, a load using `disable_file_cache=true` could
still populate the file cache because the packed writer ignored the
caller's option. Always propagate `FileWriterOptions::write_file_cache`
to the packed append context.
The test insertion conflict was resolved without importing the unrelated
nonzero-first-segment feature/test, which is absent on this branch.
Includes the cache-option unit test and preserves the original accessor
cleanup.
### Release note
Fix packed-file loads with `disable_file_cache=true` unexpectedly
populating the file cache.
### Check List (For Author)
- Test: clang-format 16 and `git diff --check` passed. BE unit tests are
included but were not run locally: the test entry point was blocked by
the Mac's unaccepted Xcode license. CI `run buildall` is requested
separately after PR creation.
- Behavior changed: Yes. Packed writes now honor the caller's file-cache
setting.
- Does this need documentation: No.
---
be/src/io/fs/packed_file_system.cpp | 5 +++--
be/src/io/fs/packed_file_writer.h | 4 ----
be/test/io/fs/packed_file_system_test.cpp | 28 ++++++++++++++++++++++++++++
be/test/io/fs/packed_file_writer_test.cpp | 4 ++--
4 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/be/src/io/fs/packed_file_system.cpp
b/be/src/io/fs/packed_file_system.cpp
index 7c1fa5bf6ff..f83cdcdc996 100644
--- a/be/src/io/fs/packed_file_system.cpp
+++ b/be/src/io/fs/packed_file_system.cpp
@@ -20,7 +20,6 @@
#include <string_view>
#include <utility>
-#include "cloud/config.h"
#include "common/status.h"
#include "io/fs/file_reader.h"
#include "io/fs/packed_file_reader.h"
@@ -97,8 +96,10 @@ Status PackedFileSystem::create_file_impl(const Path& file,
FileWriterPtr* write
return Status::OK();
}
+ // Follow the caller's file cache decision so that packed small files
behave the same as
+ // files written directly, e.g. a load with disable_file_cache=true must
not populate cache.
auto append_info = _append_info;
- if (config::enable_file_cache_write_index_file_only && opts != nullptr) {
+ if (opts != nullptr) {
append_info.write_file_cache = opts->write_file_cache;
}
diff --git a/be/src/io/fs/packed_file_writer.h
b/be/src/io/fs/packed_file_writer.h
index f571b188acb..9f03777c59b 100644
--- a/be/src/io/fs/packed_file_writer.h
+++ b/be/src/io/fs/packed_file_writer.h
@@ -57,10 +57,6 @@ public:
// Returns empty index if file is not in merge file
Status get_packed_slice_location(PackedSliceLocation* location) const
override;
-#ifdef BE_TEST
- size_t buffer_capacity_for_test() const { return _buffer.capacity(); }
-#endif
-
private:
void _release_buffer();
diff --git a/be/test/io/fs/packed_file_system_test.cpp
b/be/test/io/fs/packed_file_system_test.cpp
index a5d37ee8f99..4c9597ac720 100644
--- a/be/test/io/fs/packed_file_system_test.cpp
+++ b/be/test/io/fs/packed_file_system_test.cpp
@@ -285,6 +285,34 @@ TEST_F(PackedFileSystemTest,
LaterSegmentIndexFileUsesDirectWriter) {
EXPECT_EQ(dynamic_cast<PackedFileWriter*>(writer.get()), nullptr);
}
+TEST_F(PackedFileSystemTest, CreateFileFollowsWriteFileCacheOption) {
+ ASSERT_FALSE(config::enable_file_cache_write_index_file_only);
+ PackedFileSystem merge_fs(_inner_fs, _append_info);
+
+ // A load with disable_file_cache=true must not write packed small files
to file cache.
+ FileWriterOptions no_cache_opts {.write_file_cache = false};
+ FileWriterPtr no_cache_writer;
+ ASSERT_TRUE(
+ merge_fs.create_file(Path("rowset_1_0.dat"), &no_cache_writer,
&no_cache_opts).ok());
+ auto* packed_writer =
dynamic_cast<PackedFileWriter*>(no_cache_writer.get());
+ ASSERT_NE(packed_writer, nullptr);
+ EXPECT_FALSE(packed_writer->_append_info.write_file_cache);
+
+ FileWriterOptions cache_opts {.write_file_cache = true};
+ FileWriterPtr cache_writer;
+ ASSERT_TRUE(merge_fs.create_file(Path("rowset_1_0.idx"), &cache_writer,
&cache_opts).ok());
+ packed_writer = dynamic_cast<PackedFileWriter*>(cache_writer.get());
+ ASSERT_NE(packed_writer, nullptr);
+ EXPECT_TRUE(packed_writer->_append_info.write_file_cache);
+
+ // Without options, keep the write_file_cache from append context.
+ FileWriterPtr default_writer;
+ ASSERT_TRUE(merge_fs.create_file(Path("rowset_1_0.dat"), &default_writer,
nullptr).ok());
+ packed_writer = dynamic_cast<PackedFileWriter*>(default_writer.get());
+ ASSERT_NE(packed_writer, nullptr);
+ EXPECT_TRUE(packed_writer->_append_info.write_file_cache);
+}
+
TEST_F(PackedFileSystemTest, OpenFileNotInMergeFile) {
PackedFileSystem merge_fs(_inner_fs, _append_info);
diff --git a/be/test/io/fs/packed_file_writer_test.cpp
b/be/test/io/fs/packed_file_writer_test.cpp
index d5b7dc2da79..bf0ec042f3e 100644
--- a/be/test/io/fs/packed_file_writer_test.cpp
+++ b/be/test/io/fs/packed_file_writer_test.cpp
@@ -181,14 +181,14 @@ TEST_F(PackedFileWriterTest,
SwitchToDirectWriteReleasesBufferedMemory) {
std::string small_data(80, 'a');
Slice small_slice(small_data);
ASSERT_TRUE(writer.appendv(&small_slice, 1).ok());
- EXPECT_GT(writer.buffer_capacity_for_test(), default_capacity);
+ EXPECT_GT(writer._buffer.capacity(), default_capacity);
std::string large_data(30, 'b');
Slice large_slice(large_data);
ASSERT_TRUE(writer.appendv(&large_slice, 1).ok());
EXPECT_GT(inner_writer_ptr->append_calls(), 0);
- EXPECT_LE(writer.buffer_capacity_for_test(), default_capacity);
+ EXPECT_LE(writer._buffer.capacity(), default_capacity);
}
TEST_F(PackedFileWriterTest, CloseAsync) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
