Copilot commented on code in PR #67761:
URL: https://github.com/apache/doris/pull/67761#discussion_r3975346719
##########
cloud/src/meta-service/meta_service_txn.cpp:
##########
@@ -4620,6 +4620,25 @@ void
MetaServiceImpl::abort_txn_with_coordinator(::google::protobuf::RpcControll
}
}
+std::string get_txn_info_key_from_txn_running_key(std::string_view
txn_running_key) {
+ std::string conflict_txn_info_key;
+ std::vector<std::tuple<std::variant<int64_t, std::string>, int, int>> out;
+ txn_running_key.remove_prefix(1);
+ int ret = decode_key(&txn_running_key, &out);
+ if (ret != 0) [[unlikely]] {
+ // decode version key error means this is something wrong,
+ // we can not continue this txn
+ LOG(WARNING) << "failed to decode key, ret=" << ret << " key=" <<
hex(txn_running_key);
+ } else {
+ DCHECK(out.size() == 5) << " key=" << hex(txn_running_key) << " " <<
out.size();
+ const std::string& decode_instance_id =
std::get<1>(std::get<0>(out[1]));
+ int64_t db_id = std::get<0>(std::get<0>(out[3]));
+ int64_t txn_id = std::get<0>(std::get<0>(out[4]));
+ conflict_txn_info_key = txn_info_key({decode_instance_id, db_id,
txn_id});
+ }
+ return conflict_txn_info_key;
+}
Review Comment:
This helper has undefined behavior risk in release builds:
`remove_prefix(1)` is unconditional (bad if the view is empty), and
`out[1]`/`out[3]`/`out[4]` are accessed even though the size check is only a
`DCHECK` (compiled out in non-DCHECK builds). Add explicit runtime validation
(e.g., ensure `txn_running_key.size() >= 1`, and `out.size() == 5` before
indexing) and return empty key (or propagate an error) when validation fails.
##########
cloud/src/meta-store/mem_txn_kv.cpp:
##########
@@ -815,8 +815,6 @@ TxnErrorCode
Transaction::batch_get(std::vector<std::optional<std::string>>* res
auto ret = inner_get(k, &val, opts.snapshot);
ret == TxnErrorCode::TXN_OK ? res->push_back(val) :
res->push_back(std::nullopt);
}
- kv_->get_count_ += keys.size();
- num_get_keys_ += keys.size();
return TxnErrorCode::TXN_OK;
Review Comment:
Removing the `get_count_` / `num_get_keys_` increments in
`Transaction::batch_get` will undercount reads for any code that relies on
`txn->num_get_keys()` / `txn->get_bytes()` (e.g., the new `TXN_TOO_OLD` retry
path in `get_prepare_txn_by_coordinator` that adds these into `stats`, plus the
new tests that assert bvar deltas). Restore counter/key accounting for
batch_get (or update the accounting source consistently) so batch_get
contributes to get counters/bytes the same way individual gets do.
##########
cloud/src/meta-service/meta_service_txn.cpp:
##########
@@ -4620,6 +4620,25 @@ void
MetaServiceImpl::abort_txn_with_coordinator(::google::protobuf::RpcControll
}
}
+std::string get_txn_info_key_from_txn_running_key(std::string_view
txn_running_key) {
+ std::string conflict_txn_info_key;
+ std::vector<std::tuple<std::variant<int64_t, std::string>, int, int>> out;
+ txn_running_key.remove_prefix(1);
+ int ret = decode_key(&txn_running_key, &out);
+ if (ret != 0) [[unlikely]] {
+ // decode version key error means this is something wrong,
+ // we can not continue this txn
Review Comment:
Correct spelling/grammar: change 'can not' to 'cannot' in the comment.
--
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]
