This is an automated email from the ASF dual-hosted git repository.
gavinchou 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 110be2f7667 [fix](storage) Do not cache the failure of loading segment
rows in BetaRowset (#68675)
110be2f7667 is described below
commit 110be2f7667afcf247ef61fa80075329e2caaa86
Author: Xin Liao <[email protected]>
AuthorDate: Wed Sep 30 18:43:28 2026 +0800
[fix](storage) Do not cache the failure of loading segment rows in
BetaRowset (#68675)
Related PR: #46570
Problem Summary:
`BetaRowset::get_segment_num_rows()` loads segment rows via
`DorisCallOnce`, which also caches a failed `Status`. Since a
`BetaRowset` lives as long as its tablet version, a transient IO error
(e.g. S3 SlowDown) on the first load makes every later caller fail with
the same stale error until BE restarts. MOW compaction, which needs
segment rows for rowid conversion, then keeps failing on that tablet.
Fix: replace `DorisCallOnce` with a double-checked atomic flag and a
mutex. The flag is only set after a successful load, so a failed load is
retried by the next caller, while the loaded rows are still read without
lock.
### Release note
Fix compaction repeatedly failing with the same stale error after a
transient IO error while loading segment rows.
---
be/src/storage/rowset/beta_rowset.cpp | 17 +++++++--
be/src/storage/rowset/beta_rowset.h | 5 ++-
be/test/storage/rowset/beta_rowset_test.cpp | 54 +++++++++++++++++++++++++++++
3 files changed, 73 insertions(+), 3 deletions(-)
diff --git a/be/src/storage/rowset/beta_rowset.cpp
b/be/src/storage/rowset/beta_rowset.cpp
index a261f9fd9e2..075896fe599 100644
--- a/be/src/storage/rowset/beta_rowset.cpp
+++ b/be/src/storage/rowset/beta_rowset.cpp
@@ -115,7 +115,9 @@ Status
BetaRowset::get_segment_num_rows(std::vector<uint32_t>* segment_rows,
// So here `ROWSET_UNLOADING` is allowed.
DCHECK_NE(_rowset_state_machine.rowset_state(), ROWSET_UNLOADED);
#endif
- RETURN_IF_ERROR(_load_segment_rows_once.call([this, enable_segment_cache,
read_stats, io_ctx] {
+ auto load_segment_rows = [this, enable_segment_cache, read_stats,
io_ctx]() -> Status {
+ // Start from scratch, a previous failed attempt may have left partial
rows.
+ _segments_rows.clear();
auto segment_count = num_segments();
if (segment_count == 0) {
return Status::OK();
@@ -165,7 +167,18 @@ Status
BetaRowset::get_segment_num_rows(std::vector<uint32_t>* segment_rows,
auto self = std::dynamic_pointer_cast<BetaRowset>(shared_from_this());
return load_segment_rows_from_footer(self, &_segments_rows,
enable_segment_cache,
read_stats, io_ctx);
- }));
+ };
+
+ // Only remember a successful load. A failure (e.g. a transient S3
SlowDown) must not be
+ // cached, otherwise every later caller of this long-lived rowset gets the
same stale error.
+ if (!_segment_rows_loaded.load(std::memory_order_acquire)) {
+ std::lock_guard lock(_segment_rows_mutex);
+ if (!_segment_rows_loaded.load(std::memory_order_relaxed)) {
+ RETURN_IF_ERROR(load_segment_rows());
+ // `_segments_rows` is never modified once loaded, so it can be
read without lock.
+ _segment_rows_loaded.store(true, std::memory_order_release);
+ }
+ }
segment_rows->assign(_segments_rows.cbegin(), _segments_rows.cend());
return Status::OK();
}
diff --git a/be/src/storage/rowset/beta_rowset.h
b/be/src/storage/rowset/beta_rowset.h
index ce4f74b94a0..9b239a368cf 100644
--- a/be/src/storage/rowset/beta_rowset.h
+++ b/be/src/storage/rowset/beta_rowset.h
@@ -20,8 +20,10 @@
#include <stddef.h>
+#include <atomic>
#include <cstdint>
#include <memory>
+#include <mutex>
#include <string>
#include <vector>
@@ -115,7 +117,8 @@ private:
friend class RowsetFactory;
friend class BetaRowsetReader;
- DorisCallOnce<Status> _load_segment_rows_once;
+ std::mutex _segment_rows_mutex;
+ std::atomic<bool> _segment_rows_loaded {false};
std::vector<uint32_t> _segments_rows;
};
diff --git a/be/test/storage/rowset/beta_rowset_test.cpp
b/be/test/storage/rowset/beta_rowset_test.cpp
index e2ef4982f26..52ca6c1be89 100644
--- a/be/test/storage/rowset/beta_rowset_test.cpp
+++ b/be/test/storage/rowset/beta_rowset_test.cpp
@@ -691,6 +691,60 @@ TEST_F(BetaRowsetTest, GetSegmentNumRowsCorruptedMeta) {
sp->clear_trace();
}
+TEST_F(BetaRowsetTest, GetSegmentNumRowsRetryAfterFailure) {
+ // A failed load must not be cached. The rowset lives as long as its
tablet version, so a
+ // cached transient error (e.g. S3 SlowDown) would fail every later
caller, such as compaction.
+ auto tablet_schema = std::make_shared<TabletSchema>();
+ create_tablet_schema(tablet_schema);
+
+ auto rowset_meta = std::make_shared<RowsetMeta>();
+ init_rs_meta(rowset_meta, 1, 1);
+ // Use a dedicated rowset id so that no segment of other tests can be hit
in segment cache.
+ RowsetId rowset_id;
+ rowset_id.init(540099);
+ rowset_meta->set_rowset_id(rowset_id);
+ rowset_meta->set_num_segments(2);
+ // No segment rows in meta and no segment files, so loading from segment
footer fails.
+
+ auto rowset = std::make_shared<BetaRowset>(tablet_schema, rowset_meta, "");
+
+ auto sp = SyncPoint::get_instance();
+ int meta_path_count = 0;
+ int footer_path_count = 0;
+
+
sp->set_call_back("BetaRowset::get_segment_num_rows:use_segment_rows_from_meta",
+ [&](auto&& args) { meta_path_count++; });
+
+
sp->set_call_back("BetaRowset::get_segment_num_rows:load_from_segment_footer",
+ [&](auto&& args) { footer_path_count++; });
+
+ sp->enable_processing();
+
+ std::vector<uint32_t> segment_rows;
+ Status st = rowset->get_segment_num_rows(&segment_rows, false, &_stats);
+ ASSERT_FALSE(st.ok());
+ ASSERT_EQ(footer_path_count, 1);
+
+ // The failure is not cached, so the next call loads again and succeeds.
+ rowset_meta->set_num_segment_rows({100, 200});
+ st = rowset->get_segment_num_rows(&segment_rows, false, &_stats);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(segment_rows, (std::vector<uint32_t> {100, 200}));
+ ASSERT_EQ(meta_path_count, 1);
+
+ // The success is cached, so the following call does not load again.
+ std::vector<uint32_t> segment_rows_2;
+ st = rowset->get_segment_num_rows(&segment_rows_2, false, &_stats);
+ ASSERT_TRUE(st.ok()) << st;
+ ASSERT_EQ(segment_rows_2, (std::vector<uint32_t> {100, 200}));
+ ASSERT_EQ(meta_path_count, 1);
+ ASSERT_EQ(footer_path_count, 1);
+
+ sp->clear_all_call_backs();
+ sp->disable_processing();
+ sp->clear_trace();
+}
+
TEST_F(BetaRowsetTest, GetNumSegmentRowsAPI) {
// Test the simple get_num_segment_rows API (without loading)
auto tablet_schema = std::make_shared<TabletSchema>();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
