github-actions[bot] commented on code in PR #69632:
URL: https://github.com/apache/doris/pull/69632#discussion_r4385468005
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -721,6 +712,70 @@ public boolean unregisterTableForReplay(String tableName) {
return true;
}
+ public void invalidateTableLookup(String tableName) {
+ if (metaCache != null) {
+ String localName =
getTableForReplay(tableName).map(ExternalTable::getName).orElse(tableName);
+ metaCache.invalidateLookup(localName,
extCatalog.getLowerCaseTableNames() != 0);
+ }
+ }
+
+ /** Historical spellings require typed engine invalidation and an
independent DB row-count fence. */
+ public void invalidateTableLookupDependencies() {
+ ExternalMetaCacheMgr cacheMgr =
Env.getCurrentEnv().getExtMetaCacheMgr();
+ long catalogId = getCatalog().getId();
+ cacheMgr.invalidateRowCountCache(catalogId, getId());
+ try {
+ cacheMgr.invalidateDb(this);
+ } finally {
+ cacheMgr.invalidateRowCountCache(catalogId, getId());
+ }
+ }
+
+ public Optional<Runnable> beginTableMetadataRefresh() {
+ return metaCache == null ? Optional.empty() :
Optional.of(metaCache.beginMetadataRefresh());
+ }
+
+ /** A missing result is authorized only against the retained DB, names and
object generations. */
+ @SuppressWarnings("unchecked")
+ public Optional<Runnable> observeTableLookup(ExternalTable table) {
+ MetaCache<T> cache = metaCache;
+ if (!initialized || cache == null
+ ||
extCatalog.shouldBypassTableNameCache(SessionContext.current())) {
+ return Optional.empty();
+ }
+ // Observation must not initialize a retired catalog before a pinned
local schema read.
+ TableMetadataLoadEpoch retainedEpoch = metadataLoadEpoch.get();
+ if
(!extCatalog.isMetadataLoadEpochCurrent(retainedEpoch.catalogEpoch)) {
+ return Optional.empty();
+ }
+ long epoch = retainedEpoch.value;
+ boolean allObjects = extCatalog.getLowerCaseTableNames() != 0;
+ Optional<Runnable> observed = cache.observeLookup(table.getName(),
table.getRemoteName(), (T) table,
+ allObjects, () -> {
+ if (allObjects) {
+ invalidateTableLookupDependencies();
+ } else {
+
Env.getCurrentEnv().getExtMetaCacheMgr().invalidateTableCache(table);
+ }
+ });
+ return observed.map(action -> () ->
MetaCacheEntry.runWithDeferredRemovalCallbacks(() -> {
+ // A parent object loader can acquire the catalog monitor while
holding its key lock.
+ // Pin that key first, then enter the same catalog -> database ->
table-cache order.
+ extCatalog.executeIfDatabaseCurrent(this, () -> {
+ synchronized (extCatalog) {
Review Comment:
[P1] Avoid the parent-key/catalog lock cycle with Iceberg DDL
This callback retains the parent DB key through `executeIfMetaObjCurrent()`
while waiting for the catalog monitor, but
`IcebergMetadataOps.performCreateTable()` calls synchronized
`IcebergExternalCatalog.getDbForCatalogOperation()` -> `getDbNullable()` in the
opposite order. A possible interleaving is: CREATE holds the catalog monitor;
repair holds the DB key and has passed its resident-parent check; an
already-running load for another DB publishes and capacity-evicts that parent;
CREATE's now-cold DB lookup waits for the same key. Repair waits for CREATE's
monitor indefinitely, blocking further catalog operations. The resident check
does not pin the Caffeine entry, and the epoch recheck is reached only after
acquiring the monitor. HMS Iceberg resolution has the same inverse order. Make
both resolution and repair follow a consistent order, and cover parent eviction
against real DDL resolution; the new mocked ordering test cannot detect this
cycle.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/metacache/MetaCache.java:
##########
@@ -662,6 +687,99 @@ public void invalidate(String localName, long id) {
}
}
+ /** Fences old object loads while retaining unrelated cached objects and
their ID mappings. */
+ public void invalidateLookup(String localName, boolean allObjects) {
+ metaObjLifecycleLock.writeLock().lock();
+ try {
+ synchronized (namesMutationLock) {
+ invalidateNames();
+ metaObjGeneration.incrementAndGet();
+ lookupObservationGeneration++;
+ if (allObjects) {
+ metaObjCache.invalidateAll();
+ idToName.clear();
Review Comment:
[P2] Preserve numeric lookup routes for surviving sibling tables
With `lower_case_table_names=2`, warm existing tables A and B, then
successfully `REFRESH TABLE A`. The new `invalidateTableLookup()` passes
`allObjects=true`, so this branch removes B's numeric route even though B still
exists. `getMetaObjById(B.id)` then returns empty before attempting a load, and
`SHOW TABLES` rebuilds only names. Consequently
`StatisticsJobAppender.appendColumnsToJobs()` polls and discards queued B work
as a missing table; an in-flight B analysis also cannot finalize through
`AnalysisManager.updateTableStats()`. Recovery requires a named B
lookup/registration. Retain or rebuild recoverable routes for unrelated
siblings while fencing stale objects. This is a distinct successful-refresh
trigger from the existing transient-enumeration and missing-Paimon-branch
threads; those fixes do not cover it.
--
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]
