bobhan1 commented on code in PR #66598:
URL: https://github.com/apache/doris/pull/66598#discussion_r3803598502
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -4363,6 +4566,31 @@ void
MetaServiceImpl::get_delete_bitmap(google::protobuf::RpcController* control
msg = ss.str();
return;
}
+ if (!skip_last_delete_bitmap &&
!last_delete_bitmap_first_blob_key.empty()) {
Review Comment:
**The new retry handling does not cover same-key bitmap replacement**
`get_delete_bitmap` can still assemble one logical delete bitmap from
fragments read before and after a same-key replacement when recreating the FDB
transaction after `TXN_TOO_OLD`.
This race already existed before this PR. This PR improves the retry
handling for key-based cleanup by dropping a partial bitmap when its first
fragment has disappeared, but it does not cover replacement of the same
`(rowset_id, end_version, segment_id)`, because the replacement reuses the same
fragment keys.
For example:
1. A reader retrieves fragments 0 and 1 from the existing bitmap at
`(rowset_id, end_version, segment_id)`.
2. Post-compaction aggregation replaces that same logical bitmap with the
newly aggregated value, reusing fragment keys 0, 1, 2, and so on.
3. The reader's next range request returns `TXN_TOO_OLD`. The new
transaction finds that fragment key 0 still exists, so it retains fragments 0
and 1 read before the replacement and resumes from fragment 2.
4. It can then append fragment 2 from the newly aggregated bitmap. The
fragment sequence remains continuous, so the continuity check does not detect
that the bytes came from different snapshots.
5. If the replacement contains fewer fragments, the scan can instead finish
and return the partial pre-replacement value with `OK`, because the total
fragment count is unknown.
A complete fix would discard the partially assembled logical bitmap and
restart it from fragment 0 whenever the FDB transaction is recreated. A focused
test could force pagination, replace the same end-version key, inject
`TXN_TOO_OLD`, and verify that only the complete replacement value is returned.
This is a narrow, pre-existing race, and I do not consider it blocking for
this PR. It can be addressed as a follow-up.
--
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]
