deardeng commented on code in PR #64647:
URL: https://github.com/apache/doris/pull/64647#discussion_r3441192051
##########
cloud/src/meta-service/meta_service_txn.cpp:
##########
@@ -1853,7 +1853,11 @@ void MetaServiceImpl::commit_txn_immediately(
int64_t table_id = i.first;
std::string ver_key = table_version_key({instance_id, db_id,
table_id});
std::string ver_val;
- err = txn->get(ver_key, &ver_val);
+ // snapshot read: the returned table version is only a hint
for FE's version
+ // cache; the real increment is done by update_table_version()
via atomic_add.
+ // A non-snapshot read would add ver_key to the read-conflict
set and make
+ // concurrent commits on the same table conflict
(KV_TXN_CONFLICT).
+ err = txn->get(ver_key, &ver_val, true);
Review Comment:
Thanks for the careful review. You're right that with a snapshot read the
returned table_version is no longer unique per commit under concurrency — two
concurrent commits can both read N and both
return N+1. But this value is an advisory cache hint, and I don't think
it's blocking:
- SQL result cache doesn't key on it. CacheAnalyzer invalidates on
partition visible version/time (CacheAnalyzer.java:463-466), which this change
doesn't touch.
- The table-version cache is bounded, not authoritative.
getVisibleVersion() is TTL-gated and falls back to an authoritative get_version
RPC; CloudSyncVersionDaemon (~:128) reconciles to the real
value. So the worst case is bounded staleness of an optimization — data
visibility is governed by the partition version, which is unaffected.
- No test regresses. The CommitTxn* tests are sequential and re-read the
stored counter (meta_service_test.cpp:~1875), so they still pass since
atomic_add is unchanged.
The real residual you flagged — the advisory token losing per-commit
uniqueness — slightly weakens cache freshness (bounded by TTL + the sync
daemon). I'd keep this PR scoped to removing the
KV_TXN_CONFLICT regression and revisit the advisory-value design in a
follow-up (e.g. not populating it on the non-versioned path, or deriving a
conflict-free unique token).
--
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]
