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 33f2c10f72f [fix](be) Return errors for missing rowset segment
IDs(caused by segment compaction race) (#68482)
33f2c10f72f is described below
commit 33f2c10f72f1b9150f81798007e24befe413c317
Author: meiyi <[email protected]>
AuthorDate: Mon Sep 28 19:03:38 2026 +0800
[fix](be) Return errors for missing rowset segment IDs(caused by segment
compaction race) (#68482)
Related PR: #65190
Problem Summary: Load-time segment compaction can remove an original
segment before an asynchronous delete bitmap task builds its temporary
rowset. Looking up that segment in the explicit ID list then throws an
uncaught check exception and aborts the BE. Return NOT_FOUND from
position_of for missing segment IDs, including out-of-range IDs in
legacy rowsets, and propagate the result through all callers. Include
the segment, rowset and tablet in the error. Preserve the existing index
compaction failure handling. This changes the failure into a returned
operation error; it does not fix the underlying segment lifecycle race.
---
be/src/storage/compaction/compaction.cpp | 6 +++++-
be/src/storage/rowset/beta_rowset.cpp | 4 ++--
be/src/storage/rowset/rowset_meta.cpp | 15 +++++++++++----
be/src/storage/rowset/rowset_meta.h | 2 +-
be/src/storage/tablet/base_tablet.cpp | 6 ++++--
be/test/storage/rowset/beta_rowset_test.cpp | 12 ++++++++++--
be/test/storage/rowset/rowset_meta_test.cpp | 28 ++++++++++++++++++++++++----
be/test/testutil/index_storage_test_util.cpp | 6 +++++-
8 files changed, 62 insertions(+), 17 deletions(-)
diff --git a/be/src/storage/compaction/compaction.cpp
b/be/src/storage/compaction/compaction.cpp
index e81e1c7f8e8..dc8c272cf87 100644
--- a/be/src/storage/compaction/compaction.cpp
+++ b/be/src/storage/compaction/compaction.cpp
@@ -1042,7 +1042,11 @@ Status Compaction::do_inverted_index_compaction() {
auto* rowset = find_it->second;
auto seg_pos = rowset->rowset_meta()->position_of(seg_id);
- auto seg = rowset->segment(seg_pos);
+ if (!seg_pos.has_value()) {
+ mark_skip_index_compaction(ctx, error_handler);
+ return seg_pos.error();
+ }
+ auto seg = rowset->segment(seg_pos.value());
auto fs = rowset->rowset_meta()->fs();
DBUG_EXECUTE_IF("Compaction::do_inverted_index_compaction_get_fs_error", { fs =
nullptr; })
if (!fs) {
diff --git a/be/src/storage/rowset/beta_rowset.cpp
b/be/src/storage/rowset/beta_rowset.cpp
index 80f6c07d97d..a261f9fd9e2 100644
--- a/be/src/storage/rowset/beta_rowset.cpp
+++ b/be/src/storage/rowset/beta_rowset.cpp
@@ -255,8 +255,8 @@ Status
BetaRowset::load_segments(std::vector<segment_v2::SegmentSharedPtr>* segm
Status BetaRowset::load_segment(int64_t seg_id, OlapReaderStatistics* stats,
segment_v2::SegmentSharedPtr* segment,
const io::IOContext* io_ctx) {
- return
load_segment(_rowset_meta->segment_ref(_rowset_meta->position_of(seg_id)),
stats,
- segment, io_ctx);
+ auto pos = DORIS_TRY(_rowset_meta->position_of(seg_id));
+ return load_segment(_rowset_meta->segment_ref(pos), stats, segment,
io_ctx);
}
Status BetaRowset::load_segment(RowsetSegmentRef seg, OlapReaderStatistics*
stats,
diff --git a/be/src/storage/rowset/rowset_meta.cpp
b/be/src/storage/rowset/rowset_meta.cpp
index 05ca4be46b7..869e2187c20 100644
--- a/be/src/storage/rowset/rowset_meta.cpp
+++ b/be/src/storage/rowset/rowset_meta.cpp
@@ -349,16 +349,23 @@ void RowsetMeta::set_segment_ids(const
std::vector<int64_t>& segment_ids) {
_validate_segment_ids();
}
-size_t RowsetMeta::position_of(int64_t seg_id) const {
+Result<size_t> RowsetMeta::position_of(int64_t seg_id) const {
DORIS_CHECK_GE(seg_id, 0);
if (!has_segment_ids()) {
- DORIS_CHECK_LT(seg_id, num_segments());
+ if (seg_id >= num_segments()) {
+ return ResultError(Status::Error<ErrorCode::NOT_FOUND>(
+ "segment {} not found in rowset {}, tablet {}", seg_id,
rowset_id().to_string(),
+ tablet_id()));
+ }
return cast_set<size_t>(seg_id);
}
const auto& segment_ids = _rowset_meta_pb.segment_ids();
auto it = std::lower_bound(segment_ids.begin(), segment_ids.end(), seg_id);
- DORIS_CHECK(it != segment_ids.end());
- DORIS_CHECK_EQ(*it, seg_id);
+ if (it == segment_ids.end() || *it != seg_id) {
+ return ResultError(
+ Status::Error<ErrorCode::NOT_FOUND>("segment {} not found in
rowset {}, tablet {}",
+ seg_id,
rowset_id().to_string(), tablet_id()));
+ }
return cast_set<size_t>(std::distance(segment_ids.begin(), it));
}
diff --git a/be/src/storage/rowset/rowset_meta.h
b/be/src/storage/rowset/rowset_meta.h
index 9255cdb9a09..e4dc1b1e5c6 100644
--- a/be/src/storage/rowset/rowset_meta.h
+++ b/be/src/storage/rowset/rowset_meta.h
@@ -329,7 +329,7 @@ public:
RowsetSegmentMetaRange segments() const;
- size_t position_of(int64_t seg_id) const;
+ Result<size_t> position_of(int64_t seg_id) const;
// Convert to RowsetMetaPB, skip_schema is only used by cloud to separate
schema from rowset meta.
void to_rowset_pb(RowsetMetaPB* rs_meta_pb, bool skip_schema = false)
const;
diff --git a/be/src/storage/tablet/base_tablet.cpp
b/be/src/storage/tablet/base_tablet.cpp
index 71a5cd6c641..99ed5f24536 100644
--- a/be/src/storage/tablet/base_tablet.cpp
+++ b/be/src/storage/tablet/base_tablet.cpp
@@ -1923,7 +1923,8 @@ Status BaseTablet::check_rowid_conversion(
for (auto& [src, dst] : locations) {
std::string src_key;
std::string dst_key;
- const size_t src_segment_pos =
src_rowset->rowset_meta()->position_of(src.segment_id);
+ const size_t src_segment_pos =
+
DORIS_TRY(src_rowset->rowset_meta()->position_of(src.segment_id));
Status s =
segments[src_segment_pos]->read_key_by_rowid(src.row_id, &src_key);
if (UNLIKELY(s.is<NOT_IMPLEMENTED_ERROR>())) {
LOG(INFO) << "primary key index of old version does not "
@@ -1937,7 +1938,8 @@ Status BaseTablet::check_rowid_conversion(
return s;
}
- const size_t dst_segment_pos =
dst_rowset->rowset_meta()->position_of(dst.segment_id);
+ const size_t dst_segment_pos =
+
DORIS_TRY(dst_rowset->rowset_meta()->position_of(dst.segment_id));
s = dst_segments[dst_segment_pos]->read_key_by_rowid(dst.row_id,
&dst_key);
if (UNLIKELY(!s)) {
LOG(WARNING) << "failed to get dst key: |" << dst.rowset_id <<
"|" << dst.segment_id
diff --git a/be/test/storage/rowset/beta_rowset_test.cpp
b/be/test/storage/rowset/beta_rowset_test.cpp
index c7337ff4f9b..e2ef4982f26 100644
--- a/be/test/storage/rowset/beta_rowset_test.cpp
+++ b/be/test/storage/rowset/beta_rowset_test.cpp
@@ -536,10 +536,18 @@ TEST_F(BetaRowsetTest, TmpRowsetUsesCompletedSegmentIds) {
ASSERT_TRUE(writer.build_tmp(tmp_rowset).ok());
ASSERT_NE(tmp_rowset, nullptr);
EXPECT_EQ(tmp_rowset->num_segments(), 2);
- EXPECT_EQ(tmp_rowset->rowset_meta()->position_of(2), 0);
- EXPECT_EQ(tmp_rowset->rowset_meta()->position_of(6), 1);
+ EXPECT_EQ(TEST_TRY(tmp_rowset->rowset_meta()->position_of(2)), 0);
+ EXPECT_EQ(TEST_TRY(tmp_rowset->rowset_meta()->position_of(6)), 1);
EXPECT_EQ(tmp_rowset->rowset_meta()->segment_id(0), 2);
EXPECT_EQ(tmp_rowset->rowset_meta()->segment_id(1), 6);
+
+ auto* beta_rowset = static_cast<BetaRowset*>(tmp_rowset.get());
+ segment_v2::SegmentSharedPtr segment;
+ for (int64_t seg_id : {3, 7}) {
+ auto status = beta_rowset->load_segment(seg_id, nullptr, &segment);
+ EXPECT_TRUE(status.is<NOT_FOUND>()) << status;
+ EXPECT_EQ(segment, nullptr);
+ }
}
TEST_F(BetaRowsetTest, GetSegmentNumRowsFromMeta) {
diff --git a/be/test/storage/rowset/rowset_meta_test.cpp
b/be/test/storage/rowset/rowset_meta_test.cpp
index 633c4f4999a..1a7a20a548e 100644
--- a/be/test/storage/rowset/rowset_meta_test.cpp
+++ b/be/test/storage/rowset/rowset_meta_test.cpp
@@ -626,7 +626,7 @@ TEST_F(RowsetMetaTest, TestSegmentIdsAccessors) {
EXPECT_EQ(rowset_meta.num_segments(), 3);
EXPECT_EQ(rowset_meta.segment_id(0), 0);
EXPECT_EQ(rowset_meta.segment_id(2), 2);
- EXPECT_EQ(rowset_meta.position_of(2), 2);
+ EXPECT_EQ(TEST_TRY(rowset_meta.position_of(2)), 2);
// Non-contiguous segment_ids: position <-> real id mapping.
rowset_meta.set_segment_ids({0, 2, 5});
@@ -635,9 +635,29 @@ TEST_F(RowsetMetaTest, TestSegmentIdsAccessors) {
EXPECT_EQ(rowset_meta.segment_id(0), 0);
EXPECT_EQ(rowset_meta.segment_id(1), 2);
EXPECT_EQ(rowset_meta.segment_id(2), 5);
- EXPECT_EQ(rowset_meta.position_of(0), 0);
- EXPECT_EQ(rowset_meta.position_of(2), 1);
- EXPECT_EQ(rowset_meta.position_of(5), 2);
+ EXPECT_EQ(TEST_TRY(rowset_meta.position_of(0)), 0);
+ EXPECT_EQ(TEST_TRY(rowset_meta.position_of(2)), 1);
+ EXPECT_EQ(TEST_TRY(rowset_meta.position_of(5)), 2);
+}
+
+TEST_F(RowsetMetaTest, TestPositionOfMissingSegment) {
+ RowsetMeta rowset_meta;
+ ASSERT_TRUE(rowset_meta.init_from_json(_json_rowset_meta));
+
+ rowset_meta.set_num_segments(0);
+
EXPECT_TRUE(TEST_RESULT_ERROR(rowset_meta.position_of(0)).is<ErrorCode::NOT_FOUND>());
+ rowset_meta.set_num_segments(3);
+
EXPECT_TRUE(TEST_RESULT_ERROR(rowset_meta.position_of(3)).is<ErrorCode::NOT_FOUND>());
+
+ rowset_meta.set_segment_ids({2, 6});
+ for (int64_t seg_id : {0, 3, 7}) {
+ auto status = TEST_RESULT_ERROR(rowset_meta.position_of(seg_id));
+ EXPECT_TRUE(status.is<ErrorCode::NOT_FOUND>()) << status;
+ EXPECT_NE(status.to_string().find("segment " +
std::to_string(seg_id)), std::string::npos);
+
EXPECT_NE(status.to_string().find(rowset_meta.rowset_id().to_string()),
std::string::npos);
+ EXPECT_NE(status.to_string().find("tablet " +
std::to_string(rowset_meta.tablet_id())),
+ std::string::npos);
+ }
}
TEST_F(RowsetMetaTest, TestSegmentIdsMustBeStrictlyIncreasing) {
diff --git a/be/test/testutil/index_storage_test_util.cpp
b/be/test/testutil/index_storage_test_util.cpp
index e72a35a58cc..344a2f50614 100644
--- a/be/test/testutil/index_storage_test_util.cpp
+++ b/be/test/testutil/index_storage_test_util.cpp
@@ -617,7 +617,11 @@ void collect_variant_column_layout(const ColumnMetaPB&
column_meta, IndexSegment
}
Result<IndexSegmentLayout> probe_segment(const RowsetSharedPtr& rowset,
int64_t segment_id) {
- auto seg = rowset->segment(rowset->rowset_meta()->position_of(segment_id));
+ auto seg_pos = rowset->rowset_meta()->position_of(segment_id);
+ if (!seg_pos.has_value()) {
+ return ResultError(seg_pos.error());
+ }
+ auto seg = rowset->segment(seg_pos.value());
auto segment_path = seg.path();
if (!segment_path.has_value()) {
return ResultError(segment_path.error());
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
