github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3836983314
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalMetaCache.java:
##########
@@ -86,41 +120,324 @@ public Table getPaimonTable(NameMapping nameMapping) {
public PaimonSnapshotCacheValue getSnapshotCache(ExternalTable dorisTable)
{
NameMapping nameMapping = dorisTable.getOrBuildNameMapping();
- return
tableEntry.get(nameMapping.getCtlId()).get(nameMapping).getLatestSnapshotCacheValue();
+ MetaCacheEntry<NameMapping, PaimonTableCacheValue> tables =
tableEntry.get(nameMapping.getCtlId());
+ PaimonTableCacheValue tableValue = tables.get(nameMapping);
+ if (!tables.isEffectivelyEnabled()) {
+ // Projections are keyed by the synthetic generation of a
published table handle. An
+ // ineffective table entry publishes nothing, so nothing keyed by
this load could ever
+ // be looked up again: serve it directly instead of churning the
snapshot entry.
+ PaimonSnapshot fence = loadLatestSnapshotFence(nameMapping,
tableValue).getSnapshot();
+ return executeForGeneration(tableValue, nameMapping,
+ () -> latestSnapshotProjectionLoader.loadAtFence(
+ nameMapping, fence, tableValue.getGeneration()))
+ .bindCapturedAuthenticator(tableValue.getAuthenticator());
+ }
+ LatestFenceOwner owner = new LatestFenceOwner(nameMapping,
tableValue.getGeneration());
+ // Serve the memoized latest projection of this table generation while
it is still
+ // published: the latest read is as stale-until-TTL/refresh as the
cached table
+ // handle itself and costs no snapshot IO, preserving the pre-existing
external
+ // metadata cache contract. The fence is re-observed only when no
projection of this
+ // generation is reachable anymore (first read, expiry, weight
eviction, explicit
+ // invalidation), which is also when rollback ordering below matters.
+ ObservedFence observed = latestObservedFences.get(owner);
+ if (observed != null) {
+ PaimonSnapshotCacheValue memoized =
+
snapshotEntry.get(nameMapping.getCtlId()).peekIfPresent(observed.key);
+ if (memoized != null) {
+ return memoized;
+ }
+ }
+ // Order fence observations, not snapshot ids: a rollback moves the
latest snapshot
+ // backwards, and a concurrent call may finish after a later
observation (reversed
+ // completion). Either way the most recently observed fence is the one
future lookups
+ // read. Capture and number assignment are serialized per owner so the
observation order
+ // always matches the fence-read order; without this, a capture
pausing between the read
+ // and the increment could replace a newer already-published fence
with an older one.
+ PaimonSnapshot fence;
+ long observation;
+ Object captureLock = fenceCaptureLocks.computeIfAbsent(owner, ignored
-> new Object());
Review Comment:
[P2] Clean up capture locks when projection loading fails
After this `computeIfAbsent`, both `loadLatestSnapshotFence()` and the later
projection loader can throw before the unpublished-generation cleanup at lines
180-187. `tables.get()` still returns a freshly generated table value when
weighted admission rejects it, so repeated failures for an
oversized/unsupported table strand a different `fenceCaptureLocks` owner on
every request; those rejected values have no replacement/removal callback,
leaving this map outside all configured cache budgets. Please cover capture
through projection publication with failure cleanup that conditionally removes
the unpublished owner (preferably `remove(owner, captureLock)`), and add
repeated rejected-load tests for both exception points.
--
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]
