github-actions[bot] commented on code in PR #63985:
URL: https://github.com/apache/doris/pull/63985#discussion_r3338432229
##########
cloud/src/meta-service/meta_service.cpp:
##########
@@ -2732,16 +2704,54 @@ void
MetaServiceImpl::commit_rowset(::google::protobuf::RpcController* controlle
bool is_versioned_read = is_version_read_enabled(instance_id);
auto recycle_rs_key = recycle_rowset_key({instance_id, tablet_id,
rowset_id});
- if (check_idempotent_for_txn_or_job(txn.get(), recycle_rs_key,
rowset_meta, instance_id,
- tablet_id, rowset_id,
request->tablet_job_id(),
- is_versioned_read,
resource_mgr_.get(), code, msg) != 0) {
- return;
+ if (!config::enable_recycle_delete_rowset_key_check) {
+ std::string tablet_job_id = request->tablet_job_id();
+ if (config::enable_tablet_job_check && !tablet_job_id.empty()) {
+ if (!check_job_existed(txn.get(), code, msg, instance_id,
tablet_id, rowset_id,
+ tablet_job_id, is_versioned_read,
resource_mgr_.get())) {
+ return;
+ }
+ }
+
+ // Check if the commit rowset request is invalid.
+ // If the transaction has been finished, it means this commit rowset
is a timeout retry request.
+ // In this case, do not write the recycle key again, otherwise it may
cause data loss.
+ // If the rowset had load id, it means it is a load request, otherwise
it is a
+ // compaction/sc request.
+ if (config::enable_load_txn_status_check && rowset_meta.has_load_id()
&&
+ !check_transaction_status(TxnStatusPB::TXN_STATUS_PREPARED,
txn.get(), instance_id,
+ rowset_meta.txn_id(), code, msg)) {
+ LOG(WARNING) << "commit rowset failed, txn_id=" <<
rowset_meta.txn_id()
+ << ", tablet_id=" << tablet_id << ", rowset_id=" <<
rowset_id
+ << ", rowset_state=" << rowset_meta.rowset_state() <<
", msg=" << msg;
+ return;
+ }
}
// Check if commit key already exists.
std::string existed_commit_val;
err = txn->get(tmp_rs_key, &existed_commit_val);
if (err == TxnErrorCode::TXN_OK) {
+ if (config::enable_recycle_delete_rowset_key_check) {
+ bool recycle_rs_key_exists = false;
+ if (!check_recycle_rowset_key(txn.get(), recycle_rs_key,
rowset_meta,
+ &recycle_rs_key_exists, code, msg)) {
+ return;
+ }
+ if (recycle_rs_key_exists) {
+ code = MetaServiceCode::INVALID_ARGUMENT;
Review Comment:
This rejects valid idempotent retries for delete-predicate rowsets.
`check_recycle_rowset_key()` sets `recycle_rs_key_exists = true` whenever
`rowset_meta.has_delete_predicate()` is true, without reading KV, for old-BE
compatibility. When a delete-predicate `commit_rowset` has already written
`tmp_rs_key`, a duplicate request enters this `TXN_OK` branch, the helper
reports the recycle key as existing, and this block returns `INVALID_ARGUMENT`
before comparing `existed_rowset_meta->rowset_id_v2()` with the request. The
previous code skipped the recycle-key check for delete-predicate rowsets, so
the retry could return OK/ALREADY_EXISTED. Please handle the delete-predicate
compatibility case separately here, or let the existing tmp-rowset idempotency
check run before treating a recycle key as mutually exclusive.
--
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]
