CalvinKirs commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3836645366
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/source/IcebergScanNode.java:
##########
@@ -792,8 +792,9 @@ private Table useFrozenTableGeneration(Table currentTable) {
if
(snapshot.filter(IcebergMvccSnapshot.class::isInstance).isPresent()) {
IcebergSnapshotCacheValue cacheValue =
((IcebergMvccSnapshot)
snapshot.get()).getSnapshotCacheValue();
- if (cacheValue.getIcebergTable().isPresent()) {
- Table frozenBaseTable = cacheValue.getIcebergTable().get();
+ Optional<Table> frozenTable = cacheValue.getIcebergTable();
Review Comment:
Fixed in 246b5445097: Iceberg snapshot projections now carry the
ExecutionAuthenticator captured from the table generation that built them
(bound at both projection branches in getSnapshotCache), and
IcebergScanNode.useFrozenTableGeneration - the shared entry for regular
relations and snapshot-selectable system tables, called from both doInitialize
and createTableScan - validates the pinned projection against the catalog
current context via ensurePlannableUnder before any frozen table is planned. A
statement bound before a credential/storage ALTER now fails retryably before
planning instead of running G1 FrozenTableOperations/FileIO under the replaced
authenticator, storage state and executor. Regressions cover context
propagation from the published generation and the fence accepting the captured
context while rejecting a replaced one (system tables share the same validated
path).
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -139,45 +312,116 @@ public ManifestCacheValue
getManifestCacheValue(ExternalTable dorisTable,
MetaCacheEntry<IcebergManifestEntryKey, ManifestCacheValue>
manifestEntry =
this.manifestEntry.get(nameMapping.getCtlId());
IcebergManifestEntryKey key = IcebergManifestEntryKey.of(manifest);
- boolean hit = manifestEntry.getIfPresent(key) != null;
+ boolean hit = manifestEntry.peekIfPresent(key) != null;
if (cacheHitRecorder != null) {
cacheHitRecorder.accept(hit);
}
- return manifestEntry.get(key, ignored ->
loadManifestCacheValue(manifest, icebergTable, key.getContent()));
+ return manifestEntry.get(key,
+ ignored -> loadManifestCacheValue(
+ manifest, icebergTable, key.getContent(),
manifestEntry.isWeightAccounting()));
}
@Override
public void invalidateCatalog(long catalogId) {
- dropManifestFileIoCacheForCatalog(catalogId);
+ // Collect while the entries are still enumerable, drop only after the
entries are
+ // detached: a load racing a pre-detach drop could repopulate the SDK
content cache
+ // for a FileIO this reset already cleaned.
+ List<FileIO> retainedFileIos = collectManifestFileIos(catalogId);
super.invalidateCatalog(catalogId);
+ dropManifestFileIoCaches(retainedFileIos);
}
@Override
public void invalidateCatalogEntries(long catalogId) {
- dropManifestFileIoCacheForCatalog(catalogId);
+ List<FileIO> retainedFileIos = collectManifestFileIos(catalogId);
super.invalidateCatalogEntries(catalogId);
+ dropManifestFileIoCaches(retainedFileIos);
}
private IcebergTableCacheValue loadTableCacheValue(NameMapping
nameMapping) {
- CatalogIf catalog =
Env.getCurrentEnv().getCatalogMgr().getCatalog(nameMapping.getCtlId());
+ CatalogIf catalog = getCatalog(nameMapping.getCtlId());
if (catalog == null) {
throw new RuntimeException(String.format("Cannot find catalog %d
when loading table %s/%s.",
nameMapping.getCtlId(), nameMapping.getLocalDbName(),
nameMapping.getLocalTblName()));
}
+ // One catalog generation must supply the ops, the loaded table and
the bound
+ // authenticator together: re-reading the mutable catalog after the
load could bind a
+ // handle of the old generation to the execution context a concurrent
ALTER installed.
+ // The acquisition is re-validated before publication; a mid-flight
reset fails the miss
+ // (the caller retries against the reinitialized catalog) instead of
publishing a splice.
+ ExecutionAuthenticator authenticator =
requireExecutionAuthenticator(catalog);
IcebergMetadataOps ops = resolveMetadataOps(catalog);
+ IcebergTableCacheValue value = execute(authenticator, () -> {
+ Table table = ops.loadTable(nameMapping.getRemoteDbName(),
nameMapping.getRemoteTblName());
+ IcebergTableCacheValue loaded = new IcebergTableCacheValue(table);
+ loaded.bindAuthenticator(authenticator);
Review Comment:
Fixed in 246b5445097: both the Iceberg and Paimon estimators now add a flat
rounded-up retained-context allowance (AUTHENTICATION_CONTEXT_WEIGHT = 16KB,
mirroring the existing per-owner FileIO and encryption-manager allowances of
the coarse model) to every admitted table or snapshot value that carries a
bound execution context, so a retired authentication generation kept alive only
by old cached values is never entirely unaccounted; per-value charging matches
the established independent-owner-lifetimes accounting stance. Regression
asserts the exact allowance delta for bound vs unbound table and snapshot
values in both engines.
--
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]
