github-actions[bot] commented on code in PR #68104:
URL: https://github.com/apache/doris/pull/68104#discussion_r4035088962
##########
fe/fe-core/src/main/java/org/apache/doris/cloud/catalog/CloudPartition.java:
##########
@@ -203,14 +202,37 @@ private long getVisibleVersionFromMs(boolean
waitForPendingTxns) {
if (LOG.isDebugEnabled()) {
LOG.debug("get version from meta service, version: {},
partition: {}", version, super.getId());
}
- setCachedVisibleVersion(version, mTime);
- refreshedVersionCacheEpoch.accumulateAndGet(cacheEpoch, Math::max);
+ cacheVisibleVersion(version, mTime, -1, cacheEpoch,
isVersionCacheable(resp, 0, 1));
return version;
} catch (RpcException e) {
throw new RuntimeException("get version from meta service failed");
}
}
+ private void cacheVisibleVersion(long version, long updateTimeMs, long
tso, long cacheEpoch, boolean cacheable) {
+ if (!cacheable) {
+ // Invalidate before touching the timestamp, even when the cache
was valid before this read.
+ invalidateCachedVisibleVersion();
+ }
+ setCachedVisibleVersion(version, updateTimeMs, tso);
+ if (cacheable) {
+ // A response must not acknowledge an invalidation that happened
after its request started.
+ refreshedVersionCacheEpoch.accumulateAndGet(cacheEpoch, Math::max);
+ }
+ }
+
+ private static boolean isVersionCacheable(Cloud.GetVersionResponse
response, int index, int size) {
+ if (response.getHasPendingTxnsCount() == 0) {
Review Comment:
[P1] Treat omitted pending state as unknown
During a mixed-MS rollout, an older node omits this field even when
`VersionPB` still has a pending lazy transaction. Both foreground paths are
non-waiting, so returning `true` acknowledges and caches the pre-commit
version; the default TTL is infinite and neither push nor the table-token
daemon guarantees repair. For a pending first write, the old batch producer can
also return protobuf default version `0`, which this client leaves unchanged
and `ScanNode` rejects. Keep zero-count non-waiting responses uncacheable and
normalize the legacy `0` to the initial version (or gate both behaviors by
capability); reverse the legacy-cache test and cover scalar/batch first writes.
##########
gensrc/proto/cloud.proto:
##########
@@ -1300,6 +1300,10 @@ message GetVersionResponse {
repeated int64 versions = 6;
repeated int64 version_update_time_ms = 7;
repeated int64 commit_tsos = 8;
+
+ // Pending state for partition versions, after any requested wait.
+ // One entry for scalar reads (including VERSION_NOT_FOUND), or per
partition in batch order.
+ repeated bool has_pending_txns = 9;
Review Comment:
[P1] Negotiate pending-state support for old FE clients
An old FE ignores unknown response field 9 and its scalar/batch paths still
validate the returned version. A new MS can therefore let its daemon cache a
pending old or first-write version, then install the already-advanced table
token; a missed push plus equal tokens and infinite TTL can preserve that
cache. Add a request capability set by upgraded FEs and use an old-client-safe
path such as waiting when it is absent. Also require an upgrade-completion
invalidation/sweep/restart, because old endpoints can seed these caches earlier
in a mixed-MS rollout; cover scalar, batch, and first-write old-client
descriptors.
--
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]
