This is an automated email from the ASF dual-hosted git repository.
liaoxin01 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 28eb75d0d3d [fix](cloud) Skip global packed index probe when opening
files of committed rowsets (#69980)
28eb75d0d3d is described below
commit 28eb75d0d3dbf481c7e00c7e4e0c2ec9b1307347
Author: Xin Liao <[email protected]>
AuthorDate: Thu Nov 5 10:02:15 2026 +0800
[fix](cloud) Skip global packed index probe when opening files of committed
rowsets (#69980)
### What problem does this PR solve?
Issue Number: None
Related PR: #64995
Problem Summary:
A cloud BE logs a burst of WARN lines with a full stack trace like
meet error status: [NOT_FOUND]File not found in global packed index:
data/<tablet>/<rowset>_0.idx
while compaction reads its input rowsets. In one instance a 4-hour BE
log had 42k
such lines over 21k paths in 158 tablets, yet every compaction
succeeded.
Reproduce: write a rowset whose segment file is smaller than
small_file_threshold_bytes (so it is packed) but whose inverted index
file is
larger (so it is uploaded as a regular object), then compact or query
it. Each
open of the regular .idx file prints the WARN with a stack, twice per
compaction
(eligibility check and the merge itself).
Root cause: #64995 made PackedFileSystem::open_file_impl fall back to
PackedFileManager::get_packed_slice_location when a path is not in
_index_map,
so that the file cache preload can read back a segment of the rowset
being
written, whose rowset meta does not exist yet. The fallback was added
unconditionally, so it also runs for a PackedFileSystem built from a
rowset
meta. That index map already holds every packed slice of the rowset, so
a miss
means a regular file, and the probe can only fail. The failure is a
Status::NotFound, and NOT_FOUND records a stack trace and logs a WARN as
soon as
it is constructed, before the caller falls back to opening the regular
file.
Fix:
- Only consult the manager when the index map is not initialized, i.e.
on the
write path, the same split exists_impl and file_size_impl already make.
- Return NOT_FOUND without a stack trace from get_packed_slice_location:
on the
write path a miss is also expected, for files too large to be packed.
After the fix, reading committed rowsets no longer takes the global
index lock
or logs anything for regular files; the write-path preload still finds
packed
slices through the manager.
### Release note
Fix noisy "File not found in global packed index" WARN logs with stack
traces
when reading rowsets that mix packed and regular files in cloud mode.
---
be/src/io/fs/packed_file_manager.cpp | 4 ++-
be/src/io/fs/packed_file_system.cpp | 7 ++++-
be/test/io/fs/packed_file_system_test.cpp | 50 +++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/be/src/io/fs/packed_file_manager.cpp
b/be/src/io/fs/packed_file_manager.cpp
index ccbc64f975f..a0a367debd8 100644
--- a/be/src/io/fs/packed_file_manager.cpp
+++ b/be/src/io/fs/packed_file_manager.cpp
@@ -524,7 +524,9 @@ Status PackedFileManager::get_packed_slice_location(const
std::string& path,
std::lock_guard<std::mutex> lock(_global_index_mutex);
auto it = _global_slice_locations.find(path);
if (it == _global_slice_locations.end()) {
- return Status::NotFound("File not found in global packed index: {}",
path);
+ // Not an error for the caller: a file too large to be packed is never
indexed here
+ return Status::Error<ErrorCode::NOT_FOUND, false>(
+ "File not found in global packed index: {}", path);
}
*location = it->second->location();
diff --git a/be/src/io/fs/packed_file_system.cpp
b/be/src/io/fs/packed_file_system.cpp
index fe45970e882..02ef672cfe0 100644
--- a/be/src/io/fs/packed_file_system.cpp
+++ b/be/src/io/fs/packed_file_system.cpp
@@ -119,9 +119,14 @@ Status PackedFileSystem::open_file_impl(const Path& file,
FileReaderSPtr* reader
const PackedSliceLocation* packed_location = nullptr;
if (it != _index_map.end()) {
packed_location = &it->second;
- } else if (PackedFileManager::instance()
+ } else if (!_index_map_initialized &&
+ PackedFileManager::instance()
->get_packed_slice_location(file_path,
&manager_location)
.ok()) {
+ // Write path: the rowset meta is not built yet, so a file of the
rowset being written
+ // (e.g. a segment read back for file cache preload) can only be found
in the manager.
+ // A rowset opened from its meta has every packed slice in the index
map, so a miss
+ // there means a regular file and the manager is not consulted.
packed_location = &manager_location;
}
diff --git a/be/test/io/fs/packed_file_system_test.cpp
b/be/test/io/fs/packed_file_system_test.cpp
index f4333a1bcab..6abb68c34d8 100644
--- a/be/test/io/fs/packed_file_system_test.cpp
+++ b/be/test/io/fs/packed_file_system_test.cpp
@@ -30,6 +30,7 @@
#include "io/fs/file_reader.h"
#include "io/fs/file_system.h"
#include "io/fs/file_writer.h"
+#include "io/fs/packed_file_manager.h"
#include "io/fs/packed_file_writer.h"
#include "io/fs/path.h"
#include "util/slice.h"
@@ -131,6 +132,7 @@ public:
void set_file_size(int64_t size) { _file_size = size; }
MockFileWriterForMerge* last_writer() const { return _last_writer; }
+ const std::string& last_opened_path() const { return _last_opened_path; }
protected:
Status create_file_impl(const Path& file, FileWriterPtr* writer,
@@ -149,6 +151,7 @@ protected:
if (!_open_status.ok()) {
return _open_status;
}
+ _last_opened_path = file.native();
// Create a mock reader with some content
auto mock_reader = std::make_shared<MockFileReader>("mock_content");
*reader = std::move(mock_reader);
@@ -196,6 +199,7 @@ private:
bool _exists_result = true;
int64_t _file_size = 0;
MockFileWriterForMerge* _last_writer = nullptr;
+ std::string _last_opened_path;
};
// Test fixture for PackedFileSystem
@@ -380,6 +384,52 @@ TEST_F(PackedFileSystemTest, OpenFileInMergeFile) {
EXPECT_EQ(reader->size(), index.size);
}
+// A file of the rowset being written is not in any index map yet, so the
writer side file
+// system has to find its slice through the manager
+TEST_F(PackedFileSystemTest, OpenFileOfRowsetBeingWrittenUsesManagerLocation) {
+ PackedSliceLocation location;
+ location.packed_file_path = "packed_file_being_written";
+ location.offset = 4;
+ location.size = 6;
+ auto& global_locations =
PackedFileManager::instance()->global_slice_locations_for_test();
+ global_locations["being_written"] =
std::make_shared<PackedSliceHandle>(location);
+
+ PackedFileSystem merge_fs(_inner_fs, _append_info);
+
+ FileReaderSPtr reader;
+ Status st = merge_fs.open_file(Path("being_written"), &reader, nullptr);
+ global_locations.erase("being_written");
+ ASSERT_TRUE(st.ok()) << st;
+ EXPECT_EQ(_inner_fs->last_opened_path(), location.packed_file_path);
+ EXPECT_EQ(reader->size(), location.size);
+}
+
+// The index map of a rowset opened from its meta holds every packed slice of
the rowset, so a
+// file missing from it is a regular file, whatever the manager still
remembers about the path
+TEST_F(PackedFileSystemTest, OpenRegularFileOfOpenedRowsetIgnoresManager) {
+ PackedSliceLocation stale_location;
+ stale_location.packed_file_path = "stale_packed_file";
+ stale_location.offset = 4;
+ stale_location.size = 6;
+ auto& global_locations =
PackedFileManager::instance()->global_slice_locations_for_test();
+ global_locations["regular_file"] =
std::make_shared<PackedSliceHandle>(stale_location);
+
+ std::unordered_map<std::string, PackedSliceLocation> index_map;
+ PackedSliceLocation index;
+ index.packed_file_path = "packed_file_path";
+ index.offset = 10;
+ index.size = 20;
+ index_map["packed_file"] = index;
+ PackedFileSystem merge_fs(_inner_fs, index_map, _append_info);
+
+ FileReaderSPtr reader;
+ Status st = merge_fs.open_file(Path("regular_file"), &reader, nullptr);
+ global_locations.erase("regular_file");
+ ASSERT_TRUE(st.ok()) << st;
+ EXPECT_EQ(_inner_fs->last_opened_path(), "regular_file");
+ EXPECT_EQ(reader->size(), std::string("mock_content").size());
+}
+
TEST_F(PackedFileSystemTest, ExistsForMergeFile) {
std::unordered_map<std::string, PackedSliceLocation> index_map;
PackedSliceLocation index;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
