CalvinKirs commented on code in PR #66717:
URL: https://github.com/apache/doris/pull/66717#discussion_r3836819904
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -216,27 +472,85 @@ 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();
+ SchemaCacheValue value = 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()));
+ // Contextual miss loaders bypass the default-loader schema validator;
ambiguous
+ // case-insensitive column names must be rejected on this path too.
+ value.validateSchema();
+ return value;
+ }
+
+ private void retireTableGeneration(NameMapping nameMapping,
+ @Nullable IcebergTableCacheValue previousValue,
IcebergTableCacheValue currentValue) {
+ if (previousValue != null &&
previousValue.isSameOperationalGeneration(currentValue)) {
Review Comment:
Fixed in the follow-up commit: the captured execution context is now part of
the Iceberg operational generation - isSameOperationalGeneration compares
authenticator identity, and hit-side sharesOperationalResources revalidation
compares the table generation context against the projection captured context -
so an auth-only ALTER followed by a same-UUID/metadata refresh with equivalent
FileIO retires the old-context projection on replacement, and a hit rebuilds it
bound to the new context instead of serving a permanently unplannable value.
The planning fence also now applies only when a frozen handle is actually
planned, so count-mode values are unaffected. Regression
testAuthOnlyAlterRetiresProjectionsOfTheOldContext covers A1 -> auth-only A2 ->
same-metadata refresh at both the equality and replacement-retirement levels.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/AbstractExternalMetaCache.java:
##########
@@ -222,8 +330,35 @@ protected final ExternalTable
findExternalTable(NameMapping nameMapping, String
nameMapping.getLocalTblName(), engineNameForError));
}
+ // A contended cache-policy handoff resolves within this window; see
requireCatalogEntryGroup.
+ private static final long PREPARE_RETRY_WINDOW_NANOS = 2_000_000_000L;
+ private static final long PREPARE_RETRY_SLEEP_MS = 50L;
+
private CatalogEntryGroup requireCatalogEntryGroup(long catalogId) {
CatalogEntryGroup group = catalogEntries.get(catalogId);
+ if (group == null && catalogPreparer != null) {
+ // The caller prepared the catalog before capturing this engine,
but a cache-policy
+ // ALTER retired the group in between. Re-prepare under the
lifecycle fence so the
+ // lookup observes the new policy instead of failing a valid
catalog. The preparer
+ // never blocks on the fence (a nested default loader may hold a
Caffeine bin lock
+ // that retirement itself needs), so a contended handoff is
absorbed with a bounded
+ // sleep-and-retry: the ALTER finishes within the window, or the
lookup fails as
+ // before without any deadlock.
+ long deadlineNanos = System.nanoTime() +
PREPARE_RETRY_WINDOW_NANOS;
+ while (true) {
+ catalogPreparer.accept(catalogId);
Review Comment:
Fixed in the follow-up commit: onCatalogPermanentlyRemoved now records a
tombstone in AbstractExternalMetaCache (catalog ids are never reused;
defensively cleared if the id is ever re-initialized), and
requireCatalogEntryGroup consults it before and during the bounded retry, so a
lookup after DROP CATALOG fails immediately with a dropped-catalog message
instead of sleeping through the two-second window, while rename and
contended-ALTER handoffs (no tombstone) keep the existing bounded retry. Both
engine overrides of the hook call super so the tombstone is engine-wide.
Latency-bounded regression testPermanentlyDroppedCatalogFailsLookupsImmediately
covers the terminal drop (well under the window, preparer not spun) and the
transient-absence retry that still re-prepares.
--
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]
