This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch branch-incremental-computation
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-incremental-computation
by this push:
new 12c179e3036 [fix](be) Return errors for missing rowset segment IDs
12c179e3036 is described below
commit 12c179e30364d7bada1dafa57a6a0c585097f9d8
Author: meiyi <[email protected]>
AuthorDate: Thu Sep 24 16:29:12 2026 +0800
[fix](be) Return errors for missing rowset segment IDs
### What problem does this PR solve?
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.
### Release note
Missing segment IDs during delete bitmap calculation now return an error
instead
of causing an uncaught check exception to terminate the BE.
### Check List (For Author)
- Test: Unit test cases added for missing segment IDs and load error
propagation;
not compiled or run at user request. Clang-format v16, build hygiene and
diff
whitespace checks passed.
- Behavior changed: Yes, missing segment lookups return NOT_FOUND instead of
throwing a check exception.
- Does this need documentation: No
(cherry picked from commit 8435c88f2ec71dda5e64615fed4d89bf32a6690a)
---
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 61f409a1de1..e8f55280951 100644
--- a/be/src/storage/compaction/compaction.cpp
+++ b/be/src/storage/compaction/compaction.cpp
@@ -1028,7 +1028,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 e1259bd4c0f..cb947792db9 100644
--- a/be/src/storage/rowset/rowset_meta.cpp
+++ b/be/src/storage/rowset/rowset_meta.cpp
@@ -338,16 +338,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 9aa4b471f0c..3fe55c70624 100644
--- a/be/src/storage/rowset/rowset_meta.h
+++ b/be/src/storage/rowset/rowset_meta.h
@@ -318,7 +318,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 f9dc3d3ed4a..14dbd90cdd7 100644
--- a/be/src/storage/tablet/base_tablet.cpp
+++ b/be/src/storage/tablet/base_tablet.cpp
@@ -1858,7 +1858,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 "
@@ -1872,7 +1873,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 45ca3a5f251..7611d4686b9 100644
--- a/be/test/storage/rowset/rowset_meta_test.cpp
+++ b/be/test/storage/rowset/rowset_meta_test.cpp
@@ -621,7 +621,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});
@@ -630,9 +630,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 21720fb87cb..ba70f175964 100644
--- a/be/test/testutil/index_storage_test_util.cpp
+++ b/be/test/testutil/index_storage_test_util.cpp
@@ -625,7 +625,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]
