github-actions[bot] commented on code in PR #67855:
URL: https://github.com/apache/doris/pull/67855#discussion_r4011431081
##########
cloud/src/meta-service/meta_service_txn.cpp:
##########
@@ -1960,6 +1966,29 @@ void MetaServiceImpl::commit_txn_immediately(
}
}
+ std::vector<int64_t> repair_tablet_ids;
+ for (const auto& [tablet_id, tablet_idx] : tablet_ids) {
+ if (!tablet_idx.has_db_id()) {
+ repair_tablet_ids.push_back(tablet_id);
+ }
+ }
+ bool need_repair_tablet_idx = !repair_tablet_ids.empty();
+
+
TEST_SYNC_POINT_CALLBACK("commit_txn_immediately::need_repair_tablet_idx",
+ &need_repair_tablet_idx);
+ if (need_repair_tablet_idx) {
+ stats.get_bytes += txn->get_bytes();
+ stats.get_counter += txn->num_get_keys();
+ txn.reset();
+ repair_tablet_index(txn_kv_, code, msg, instance_id, db_id,
txn_id, repair_tablet_ids,
Review Comment:
The repair batches are invisible to this RPC's KV accounting. This block
records only the abandoned outer transaction before `txn.reset()`, while
`repair_tablet_index` creates private transactions and never adds their
`batch_get`/put counters or bytes to `stats`; immediate and sub-transaction
repairs therefore contribute none of their repair I/O to the `commit_txn` KV
bvars, even for thousands of tablets. Please pass `KVStats` into the helper (or
return its totals) and account each batch on every exit so the existing
`RPC_PREPROCESS(commit_txn, get, put, del)` metrics include the work introduced
here.
##########
cloud/test/txn_lazy_commit_test.cpp:
##########
@@ -538,7 +536,11 @@ TEST(TxnLazyCommitTest, RepairTabletIndexTest) {
MetaServiceCode code = MetaServiceCode::UNDEFINED_ERR;
std::string msg;
- repair_tablet_index(txn_kv, code, msg, mock_instance, db_id, txn_id,
tmp_rowsets_meta, false);
+ std::vector<int64_t> tablet_ids;
+ for (int i = 0; i < 2001; ++i) {
+ tablet_ids.push_back(tablet_id_base + i);
+ }
+ repair_tablet_index(txn_kv, code, msg, mock_instance, db_id, txn_id,
tablet_ids, false);
Review Comment:
Now that the helper accepts `tablet_ids` directly, the rowset setup above is
dead work: `tmp_rowsets_meta` is never consumed, and the 2,001 `prepare_rowset`
plus 2,001 `commit_rowset` calls create state that this helper test never reads
or asserts. `create_tablet_without_db_id` already creates every index needed
for the batch-repair check, while the later integration tests cover rowset
commit paths. Please remove the unused rowset construction/preparation/commit
loop body here so this unit test does not pay for 4,002 unrelated service
operations.
--
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]
