github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3836623135
##########
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:
[P2] Account the retained authentication generation
This binding makes an admitted table value a strong owner of the catalog
generation's authenticator. For Kerberized catalogs that graph includes the
Hadoop authenticator/configuration, Subject/UGI, and credential collections. A
credential/storage ALTER clears the catalog reference without retiring this
cache group, so old values can retain old authentication generations, but the
entry estimator still returns complete without charging them; the new Paimon
table/snapshot bindings have the same ownership gap. Since the authenticator is
shared, please charge it once at a catalog/generation owner while any admitted
value retains it (or fail weighted admission closed when it cannot be bounded),
and cover credential growth across reset/rotation.
##########
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:
[P1] Keep the frozen table on its generation's execution context
This selects the relation-pinned G1 table, but `doInitialize()`
independently captures the catalog's current authenticator/storage properties
and `createTableScan()` uses its current pre-auth pool. A statement bound
before a credential/storage ALTER can therefore plan G1
`FrozenTableOperations`/FileIO under A2 and the G2 executor after
reinitialization. The table-value authenticator used while building the
projection is not carried into
`IcebergSnapshotCacheValue`/`IcebergMvccSnapshot`. Please retain the
generation-owned authenticator and executor (or fail before planning if that
generation retired), and add a bind -> ALTER/reinitialize -> scan latch test,
including a snapshot-selectable system table.
--
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]
