github-actions[bot] commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3798789863
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -216,27 +341,64 @@ private SchemaCacheValue
loadSchemaCacheValue(IcebergSchemaCacheKey key) {
key.getNameMapping().getLocalTblName(),
key.getSchemaId()));
}
- private IcebergSnapshotCacheValue loadSnapshotProjection(ExternalTable
dorisTable, Table icebergTable) {
+ private SchemaCacheValue loadSchemaCacheValue(IcebergSchemaCacheKey key,
Table retainedTable) {
+ ExternalTable dorisTable = findExternalTable(key.getNameMapping(),
ENGINE);
+ dorisTable.setUpdateTime(System.currentTimeMillis());
+ boolean isView = dorisTable instanceof IcebergExternalTable
+ && ((IcebergExternalTable) dorisTable).isView();
+ return IcebergUtils.loadSchemaCacheValue(
+ dorisTable, key.getSchemaId(), isView,
retainedTable).orElseThrow(() ->
+ new CacheException("failed to load iceberg schema cache value
for: %s.%s.%s, schemaId: %s",
+ null, key.getNameMapping().getCtlId(),
key.getNameMapping().getLocalDbName(),
+ key.getNameMapping().getLocalTblName(),
key.getSchemaId()));
+ }
+
+ private void retireTableGeneration(NameMapping nameMapping,
+ @Nullable IcebergTableCacheValue previousValue,
IcebergTableCacheValue currentValue) {
+ if (previousValue != null &&
previousValue.isSamePhysicalGeneration(currentValue)) {
Review Comment:
[P1] Refresh the snapshot handle when operational credentials change
This early return also preserves the old snapshot cache value when the
catalog refresh returns the same UUID and metadata file with renewed
operational resources. That value's `FrozenTableOperations` captures the
previous `FileIO`, encryption manager, and location provider; `IcebergScanNode`
later swaps to that frozen table and derives vended storage credentials from
it. For catalogs that rotate short-lived credentials without publishing new
metadata, subsequent scans can therefore keep using expired credentials even
though the base table entry refreshed successfully. Please retire or rebind the
operational snapshot value on table-handle refresh (schema projections can
remain generation-keyed), and cover same-UUID/same-location refreshes with
distinct credential-bearing `FileIO` instances.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -108,13 +128,87 @@ 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()).isWeightBounded();
+ 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.isWeightBounded();
+ IcebergSnapshotCacheValue snapshotValue = entry.get(key,
+ ignored -> executeAuthenticated(nameMapping.getCtlId(), () -> {
+ Table projectionTable = isolateForQueries
+ ? tableValue.newQueryScopedTable() :
tableValue.getIcebergTable();
+ IcebergSnapshotCacheValue value = loadSnapshotProjection(
+ dorisTable, projectionTable,
+ tableValue.getRetainedIcebergTable(),
+ tableValue.getRetainedCurrentSnapshotJson(),
isolateForQueries);
+ if (entry.isWeightBounded()) {
+ value.prepareForCachePublication(key);
+ }
+ return value;
+ }));
+ IcebergTableCacheValue currentTable =
tableEntry.get(nameMapping.getCtlId()).peekIfPresent(nameMapping);
+ if (currentTable != null &&
!tableValue.isSamePhysicalGeneration(currentTable)) {
Review Comment:
[P2] Do not retain children for a rejected table generation
`getWithManualLoad()` returns the loaded table even when weighted admission
rejects it, so this path can still publish snapshot and schema projections. In
that case `peekIfPresent()` is null, but both post-load guards treat null as
valid; no table replacement is published, so `retireTableGeneration` can never
remove these children. The analogous Paimon site is already under review, but
this separate Iceberg implementation still has the null-parent exemption. As
metadata locations advance, repeated oversized tables can therefore leave
multiple full table graphs in the count-bounded child caches despite
`meta.cache.iceberg.table.max-weight`. Please treat an absent current base as
stale (or prevent dependent publication unless base admission succeeds), and
add a repeated-rejection test that advances metadata locations.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalMetaCache.java:
##########
@@ -33,43 +35,58 @@
import java.util.Map;
import java.util.concurrent.ExecutorService;
+import javax.annotation.Nullable;
/**
* Paimon engine implementation of {@link AbstractExternalMetaCache}.
*
* <p>Registered entries:
* <ul>
* <li>{@code table}: loaded Paimon table handle per table mapping</li>
+ * <li>{@code snapshot}: immutable partition projection keyed by a captured
snapshot/schema fence</li>
* <li>{@code schema}: schema cache keyed by table identity + schema id</li>
* </ul>
*
- * <p>Latest snapshot metadata is modeled as a runtime projection memoized
inside the table cache
- * value instead of as an independent cache entry.
+ * <p>The latest main-branch snapshot is captured once as a fence and loaded
through an independent
+ * contextual entry. Branch/tag/options projections remain statement-local and
are not aliased to
+ * this main-snapshot key.
*
* <p>Invalidation behavior:
* <ul>
- * <li>db/table invalidation clears table and schema entries by matching
local names</li>
+ * <li>db/table invalidation clears table, snapshot and schema entries by
matching local names</li>
* <li>partition-level invalidation falls back to table-level
invalidation</li>
* </ul>
*/
public class PaimonExternalMetaCache extends AbstractExternalMetaCache {
public static final String ENGINE = "paimon";
public static final String ENTRY_TABLE = "table";
+ public static final String ENTRY_SNAPSHOT = "snapshot";
public static final String ENTRY_SCHEMA = "schema";
private final EntryHandle<NameMapping, PaimonTableCacheValue> tableEntry;
+ private final EntryHandle<PaimonSnapshotEntryKey,
PaimonSnapshotCacheValue> snapshotEntry;
private final EntryHandle<PaimonSchemaCacheKey, SchemaCacheValue>
schemaEntry;
private final PaimonTableLoader tableLoader;
private final PaimonLatestSnapshotProjectionLoader
latestSnapshotProjectionLoader;
public PaimonExternalMetaCache(ExecutorService refreshExecutor) {
- super(ENGINE, refreshExecutor);
+ this(refreshExecutor, new
ExternalMetaCacheBudgetManager(java.util.OptionalLong.empty()));
+ }
+
+ public PaimonExternalMetaCache(ExecutorService refreshExecutor,
ExternalMetaCacheBudgetManager budgetManager) {
+ super(ENGINE, refreshExecutor, budgetManager);
tableLoader = new PaimonTableLoader();
latestSnapshotProjectionLoader = new
PaimonLatestSnapshotProjectionLoader(
new PaimonPartitionInfoLoader(),
this::getPaimonSchemaCacheValue);
tableEntry = registerEntry(MetaCacheEntryDef.of(ENTRY_TABLE,
NameMapping.class, PaimonTableCacheValue.class,
this::loadTableCacheValue, defaultEntryCacheSpec(),
- MetaCacheEntryInvalidation.forNameMapping(nameMapping ->
nameMapping)));
+ MetaCacheEntryInvalidation.forNameMapping(nameMapping ->
nameMapping))
+ .withSizeEstimator((key, value) ->
value.prepareForCachePublication(key))
+ .withReplacementListener(this::retireTableGeneration));
Review Comment:
[P2] Retire children when an admitted base entry is removed
This replacement listener runs only when a new value is successfully
published. Capacity eviction, expiry, soft-value collection, and peer reclaim
remove an already-admitted table through the normal removal path without
calling `retireTableGeneration`; a later load receives a fresh synthetic
generation, so the old snapshot/schema keys are no longer addressable. This is
the already-admitted removal path, distinct from the existing
rejected-admission and successful-replacement threads. Because those child
entries have independent limits, evicting cold base tables can leave their
children consuming capacity and budget until the child TTL expires. Please add
a generation-aware removal callback (fenced so a delayed callback cannot retire
a successor), and test base capacity/expiry removal before any replacement is
published.
--
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]
