github-actions[bot] commented on code in PR #66598:
URL: https://github.com/apache/doris/pull/66598#discussion_r3746653971
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -4382,15 +4481,36 @@ void
MetaServiceImpl::get_delete_bitmap(google::protobuf::RpcController* control
// FIXME: Don't expose the implementation details of
splitting large value.
// merge splitted large values (>90*1000)
if (ver != last_ver || seg_id != last_seg_id) {
+ auto sequence =
+ out.size() == 8
+ ? static_cast<uint16_t>(
+
std::get<int64_t>(std::get<0>(out[7])) & 0xffff)
+ : uint16_t {0};
+ last_ver = ver;
+ last_seg_id = seg_id;
+ // Key-based pre-rowset cleanup may leave an obsolete
tail if its size estimate is
+ // too small. The bitmap is already aggregated, so the
tail can be skipped and will
+ // be recycled with its rowset.
+ skip_last_delete_bitmap = sequence != 0;
+ if (skip_last_delete_bitmap) {
+ LOG(WARNING)
+ << "skip incomplete delete bitmap whose
first blob "
+ "sequence is not zero"
+ << ", tablet_id=" << tablet_id
+ << ", rowset_id=" << rowset_ids[i] << ",
version=" << ver
+ << ", segment_id=" << seg_id << ",
first_sequence=" << sequence;
+ continue;
+ }
response->add_rowset_ids(rowset_ids[i]);
response->add_segment_ids(seg_id);
response->add_versions(ver);
response->add_segment_delete_bitmaps(std::string(v));
- last_ver = ver;
- last_seg_id = seg_id;
delete_bitmap_num++;
delete_bitmap_byte += v.length();
} else {
+ if (skip_last_delete_bitmap) {
Review Comment:
**[P1] Validate suffix continuity after snapshot replacement**
A `TXN_TOO_OLD` retry creates a fresh FDB transaction at the advanced
`start_key` but preserves the response and `(last_ver, last_seg_id)`. With the
new point cleanup, a commit between pages can delete the next unseen suffixes
while leaving later ones visible. The fresh scan then returns a later suffix
for this already-started group, and this branch appends it without decoding or
validating the expected sequence, producing a gapped bitmap that BE passes to
`Roaring::readSafe`. Please restart this rowset's assembly after changing
snapshots, or track the exact next suffix and restart on a discontinuity, and
add a pagination + `TXN_TOO_OLD` + cleanup interleaving test.
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -4382,15 +4481,36 @@ void
MetaServiceImpl::get_delete_bitmap(google::protobuf::RpcController* control
// FIXME: Don't expose the implementation details of
splitting large value.
// merge splitted large values (>90*1000)
if (ver != last_ver || seg_id != last_seg_id) {
+ auto sequence =
+ out.size() == 8
+ ? static_cast<uint16_t>(
+
std::get<int64_t>(std::get<0>(out[7])) & 0xffff)
+ : uint16_t {0};
+ last_ver = ver;
+ last_seg_id = seg_id;
+ // Key-based pre-rowset cleanup may leave an obsolete
tail if its size estimate is
+ // too small. The bitmap is already aggregated, so the
tail can be skipped and will
+ // be recycled with its rowset.
+ skip_last_delete_bitmap = sequence != 0;
Review Comment:
**[P1] Keep retained tails compatible with old MS readers**
This creates persisted state that only the new Meta-Service reader
understands. Point cleanup deletes the legacy key and suffixes in order and can
commit a prefix; if a later cleanup transaction fails after retries, suffix N
remains first (the under-estimation test creates the same shape). This branch
skips it, but the pre-change reader unconditionally treats the first key for a
`(version, segment)` as a complete bitmap. With random/list or pooled routing
over the shared FDB, a new MS can write that state and an old MS can later
return the fragment for BE to pass to `Roaring::readSafe`. Please gate point
cleanup until all MS readers support tails, or use a representation that
remains safe for old readers, and add mixed-version coverage.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
