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
commit 0cd51660c4b42c16e0abb841b95eda88df17a518
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Oct 8 09:31:45 2026 +0800
branch-4.1: [fix](storage) Do not cache the failure of loading segment rows
in BetaRowset #68675 (#68683)
Cherry-picked from #68675
Co-authored-by: Xin Liao <[email protected]>
---
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 637b0aa6fb1..5a9ac59375b 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 8a2c13efe92..07182f34ede 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>
@@ -114,7 +116,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 07d135702e8..e9c2cda99a4 100644
--- a/be/test/storage/rowset/beta_rowset_test.cpp
+++ b/be/test/storage/rowset/beta_rowset_test.cpp
@@ -555,6 +555,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]
