Copilot commented on code in PR #66598:
URL: https://github.com/apache/doris/pull/66598#discussion_r3746468215
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -3941,6 +3941,122 @@ void _write_delete_bitmap_kvs(MetaServiceCode& code,
std::string& msg, std::stri
<< " key_size: " << key.size() << " value_size: " << val.size();
}
+static bool commit_pre_rowset_delete_bitmap_removal(
+ MetaServiceCode& code, std::string& msg, std::stringstream& ss,
+ const std::shared_ptr<TxnKv>& txn_kv, std::unique_ptr<Transaction>&
txn, KVStats& stats,
+ UpdateDeleteBitmapTxnStats& txn_stats, int64_t tablet_id, const
std::string& rowset_id) {
+ auto txn_size = txn->approximate_bytes();
+ LOG(INFO) << "commit delete bitmap point deletes before transaction size
exceeds limit, "
+ "tablet_id="
+ << tablet_id << ", rowset=" << rowset_id << ", txn_size=" <<
txn_size;
+ auto err = txn->commit();
+ TEST_SYNC_POINT_CALLBACK("update_delete_bitmap:remove_pre_rowsets:commit",
txn_size);
+ txn_stats.total_txn_put_keys += txn->num_put_keys();
+ txn_stats.total_txn_put_bytes += txn->put_bytes();
+ txn_stats.total_txn_size += txn_size;
+ txn_stats.total_txn_count++;
+ if (err != TxnErrorCode::TXN_OK) {
+ code = cast_as<ErrCategory::COMMIT>(err);
+ ss << "failed to remove pre rowsets delete bitmap, err=" << err
+ << " tablet_id=" << tablet_id << " rowset_id=" << rowset_id << "
txn_size=" << txn_size;
+ msg = ss.str();
+ g_bvar_update_delete_bitmap_fail_counter << 1;
+ return false;
+ }
+ stats.get_bytes += txn->get_bytes();
+ stats.put_bytes += txn->put_bytes();
+ stats.del_bytes += txn->delete_bytes();
+ stats.get_counter += txn->num_get_keys();
+ stats.put_counter += txn->num_put_keys();
+ stats.del_counter += txn->num_del_keys();
+ txn_stats.current_key_count = 0;
+ txn_stats.current_value_count = 0;
+ err = txn_kv->create_txn(&txn);
+ if (err != TxnErrorCode::TXN_OK) {
+ code = cast_as<ErrCategory::CREATE>(err);
+ msg = "failed to init txn when removing pre rowsets delete bitmap";
+ return false;
+ }
+ return true;
+}
+
+static bool remove_pre_rowset_delete_bitmap(
+ MetaServiceCode& code, std::string& msg, std::stringstream& ss,
+ const std::shared_ptr<TxnKv>& txn_kv, std::unique_ptr<Transaction>&
txn, KVStats& stats,
+ const UpdateDeleteBitmapRequest* request, const std::string&
instance_id,
+ const std::set<std::string>& non_exist_rowset_ids,
UpdateDeleteBitmapTxnStats& txn_stats) {
+ if (!request->has_pre_rowset_agg_start_version() ||
+ !request->has_pre_rowset_agg_end_version() ||
+ request->pre_rowset_agg_start_version() >=
request->pre_rowset_agg_end_version()) {
+ return true;
+ }
+
+ auto tablet_id = request->tablet_id();
+ if (request->pre_rowset_delete_bitmap_stats_size() == 0) {
+ std::string pre_rowset_id;
+ for (size_t i = 0; i < request->rowset_ids_size(); ++i) {
+ if (request->rowset_ids(i) == pre_rowset_id) {
+ continue;
+ }
+ if (non_exist_rowset_ids.contains(request->rowset_ids(i))) {
+ LOG(INFO) << "skip remove pre rowsets delete bitmap,
rowset_id="
+ << request->rowset_ids(i) << " tablet_id=" <<
tablet_id
+ << " because the rowset does not exist";
+ continue;
+ }
+ pre_rowset_id = request->rowset_ids(i);
+ auto delete_bitmap_start =
+ meta_delete_bitmap_key({instance_id, tablet_id,
request->rowset_ids(i),
+
request->pre_rowset_agg_start_version(), 0});
+ auto delete_bitmap_end =
+ meta_delete_bitmap_key({instance_id, tablet_id,
request->rowset_ids(i),
+
request->pre_rowset_agg_end_version(), 0});
+ txn->remove(delete_bitmap_start, delete_bitmap_end);
+ LOG(INFO) << "remove pre rowsets delete bitmap by range,
tablet_id=" << tablet_id
+ << ", rowset=" << request->rowset_ids(i)
+ << ", start_version=" <<
request->pre_rowset_agg_start_version()
+ << ", end_version=" <<
request->pre_rowset_agg_end_version()
+ << ", start_key=" << hex(delete_bitmap_start)
+ << ", end_key=" << hex(delete_bitmap_end);
+ }
+ return true;
+ }
+
+ for (const auto& rowset_stats : request->pre_rowset_delete_bitmap_stats())
{
+ if (non_exist_rowset_ids.contains(rowset_stats.rowset_id())) {
+ LOG(INFO) << "skip remove pre rowsets delete bitmap, rowset_id="
+ << rowset_stats.rowset_id() << " tablet_id=" << tablet_id
+ << " because the rowset does not exist";
+ continue;
+ }
+ uint64_t delete_key_count = 0;
+ for (const auto& delete_bitmap_stat :
rowset_stats.delete_bitmap_stats()) {
+ auto delete_bitmap_key = meta_delete_bitmap_key(
+ {instance_id, tablet_id, rowset_stats.rowset_id(),
delete_bitmap_stat.version(),
+ delete_bitmap_stat.segment_id()});
+ for (const auto& remove_key :
+ blob_remove_keys(delete_bitmap_key,
delete_bitmap_stat.delete_bitmap_size())) {
+ auto txn_size = txn->approximate_bytes();
+ if (txn_size > 0 &&
+ txn_size + remove_key.size() * 3 >
+ static_cast<size_t>(config::max_txn_commit_byte) &&
+ !commit_pre_rowset_delete_bitmap_removal(code, msg, ss,
txn_kv, txn, stats,
+ txn_stats,
tablet_id,
+
rowset_stats.rowset_id())) {
+ return false;
+ }
+ txn->remove(remove_key);
+ delete_key_count++;
Review Comment:
`pre_rowset_delete_bitmap_stats` fields are proto2 `optional`, but the
key-removal path treats `rowset_id`, `segment_id`, `version`, and
`delete_bitmap_size` as always present. If any of these are omitted, they
default to 0/empty and can lead to deleting the wrong delete-bitmap keys (or
skipping split-key cleanup). Please validate the required fields and fail the
request with `INVALID_ARGUMENT` when they are missing.
--
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]
