github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3827448943
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalMetaCache.java:
##########
@@ -86,41 +114,173 @@ 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);
+ PaimonSnapshot fence = loadLatestSnapshotFence(nameMapping,
tableValue.getPaimonTable()).getSnapshot();
Review Comment:
[P1] Keep this fence load on the retained catalog generation
This lookup has already captured `tableValue` from the old Paimon cache
group, but the new fence refresh resolves
`catalog(nameMapping).getExecutionAuthenticator()` at execution time. During a
concurrent catalog ALTER, property notification calls `resetToUninitialized()`
first, nulling that authenticator and closing the current SDK catalog, and
retires the old Paimon group only afterward. A lookup paused after the table
hit can therefore fail here; the group-handoff retry cannot help because it
already owns the old entry. Reinitializing the current authenticator would also
pair replacement credentials/resources with the retained old table/FileIO
generation. Please retain a matching authenticated context and SDK-resource
lifetime with each published table generation, or make invalidation/retirement
preserve in-flight old-generation work. A latch test between the table hit and
this fence load during ALTER should cover both snapshot and schema misses.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -108,13 +132,108 @@ public Table getIcebergTable(ExternalTable dorisTable) {
return
tableEntry.get(nameMapping.getCtlId()).get(nameMapping).getIcebergTable();
}
+ public Table getWritableIcebergTable(ExternalTable dorisTable) {
+ NameMapping nameMapping = dorisTable.getOrBuildNameMapping();
+ CatalogIf catalog = getCatalog(nameMapping.getCtlId());
+ if (catalog == null) {
+ throw new RuntimeException("Cannot find catalog " +
nameMapping.getCtlId()
+ + " when loading a writable Iceberg table");
+ }
+ IcebergMetadataOps ops = resolveMetadataOps(catalog);
+ // DDL/actions must start from the live catalog generation. DML that
was planned against a
+ // retained read generation wraps this live table separately in
IcebergTransaction.
+ return executeAuthenticated(catalog, () -> ops.loadTable(
+ nameMapping.getRemoteDbName(),
nameMapping.getRemoteTblName()));
+ }
+
+ Table getQueryScopedIcebergTable(ExternalTable dorisTable) {
+ NameMapping nameMapping = dorisTable.getOrBuildNameMapping();
+ MetaCacheEntry<NameMapping, IcebergTableCacheValue> entry =
+ tableEntry.get(nameMapping.getCtlId());
+ IcebergTableCacheValue tableValue =
+ entry.get(nameMapping);
+ return createQueryTable(nameMapping, tableValue);
+ }
+
+ private Table createQueryTable(
+ NameMapping nameMapping, IcebergTableCacheValue tableValue) {
+ boolean isolateForQueries = tableValue.isQueryIsolationPrepared()
+ ||
snapshotEntry.get(nameMapping.getCtlId()).isWeightAccounting();
+ if (!isolateForQueries) {
+ return tableValue.getIcebergTable();
+ }
+ Table queryTable = tableValue.newQueryScopedTable();
+ IcebergSnapshotCacheValue.loadQueryMetadataForStatement(queryTable);
+ return queryTable;
+ }
+
public IcebergSnapshotCacheValue getSnapshotCache(ExternalTable
dorisTable) {
NameMapping nameMapping = dorisTable.getOrBuildNameMapping();
- return
tableEntry.get(nameMapping.getCtlId()).get(nameMapping).getLatestSnapshotCacheValue();
+ IcebergTableCacheValue tableValue =
+ tableEntry.get(nameMapping.getCtlId()).get(nameMapping);
+ Table retainedTable = tableValue.getRetainedIcebergTable();
+ java.util.Optional<IcebergSnapshotEntryKey> optionalKey =
+ IcebergSnapshotEntryKey.tryCreate(nameMapping, retainedTable);
+ if (!optionalKey.isPresent()) {
+ boolean isolateForQueries = tableValue.isQueryIsolationPrepared();
+ return executeAuthenticated(nameMapping.getCtlId(),
+ () -> loadSnapshotProjection(
+ dorisTable,
+ isolateForQueries ?
tableValue.newQueryScopedTable()
+ : tableValue.getIcebergTable(),
+ tableValue.getRetainedIcebergTable(),
+ tableValue.getRetainedCurrentSnapshotJson(),
isolateForQueries));
+ }
+ IcebergSnapshotEntryKey key = optionalKey.get();
+ MetaCacheEntry<IcebergSnapshotEntryKey, IcebergSnapshotCacheValue>
entry =
+ snapshotEntry.get(nameMapping.getCtlId());
+ boolean isolateForQueries = tableValue.isQueryIsolationPrepared()
+ || entry.isWeightAccounting();
Review Comment:
[P1] Keep this projection load on the retained catalog generation
This miss loader runs after `getSnapshotCache()` has captured `tableValue`
from the old Iceberg group, but `executeAuthenticated(catalogId, ...)` resolves
the catalog object's current authenticator. A concurrent catalog ALTER calls
`resetToUninitialized()` first, nulling that authenticator and closing current
SDK resources, and retires the old group only afterward; an unrelated property
update can even leave the Iceberg group published. A lookup paused after the
table hit can therefore fail here, and the detach retry cannot help because it
already owns the old entry. Calling `makeSureInitialized()` is not enough
either: that would combine replacement credentials/resources with the retained
old table/FileIO generation. Please retain a matching authenticated context and
resource lifetime per published generation, or preserve in-flight
old-generation work across reset. Add a latch test between the table hit and
this loader during ALTER; the schema-miss sibling and `getNewestUpdateVer
sionOrTime()` path need coverage too.
--
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]
