github-actions[bot] commented on code in PR #69629:
URL: https://github.com/apache/doris/pull/69629#discussion_r4376404476
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -680,6 +683,64 @@ 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) {
+ return Optional.empty();
+ }
+ long epoch = acquireTableMetadataLoadEpoch();
+ 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) {
+ synchronized (this) {
+ if (!initialized || metaCache != cache ||
!isTableMetadataLoadEpochCurrent(epoch)
+ ||
Env.getCurrentEnv().getCatalogMgr().getCatalog(extCatalog.getId()) !=
extCatalog) {
+ return false;
+ }
+ action.run();
Review Comment:
[P1] Avoid schema invalidation under the catalog monitor
With `enable_external_meta_cache_manual_miss_load=false`, count-bounded Hive
schema entries use Caffeine's synchronous loader. In a catalog using
`lower_case_table_names=1/2`, consider resident schema `an`, cold schema `c0`
(the names have equal Java hashes, so their `SchemaCacheKey` hashes also
collide), and a remotely dropped `gone` whose lookup is cached. The `c0` loader
can hold the Caffeine map bin while `ExternalCatalog.getSchema()` waits for the
catalog monitor; concurrently `gone`'s repair holds that monitor here and calls
`invalidateTableLookupDependencies()`, whose schema invalidation tries to
remove `an` from the same bin. Neither thread can finish, and other catalog
lookups block behind them. Deferring removal callbacks does not defer that map
mutation ([Caffeine
implementation](https://github.com/ben-manes/caffeine/blob/v2.9.3/caffeine/src/main/java/com/github/benmanes/caffeine/cache/BoundedLocalCache.java)).
Move engine invalidation outside these monitors while prese
rving the identity fence, or otherwise ensure a consistent lock order for the
supported synchronous-loading mode.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -403,7 +403,10 @@ public boolean isPartitionColumnAllowNull() {
@Override
public MvccSnapshot loadSnapshot(Optional<TableSnapshot> tableSnapshot,
Optional<TableScanParams> scanParams) {
- return new
PaimonMvccSnapshot(getPaimonSnapshotCacheValue(tableSnapshot, scanParams));
+ return withMissingTableRepair(() -> {
Review Comment:
[P2] Cover the latest-fence read with missing-table repair
For `SELECT * FROM ctl.db.t OPTIONS('scan.mode'='latest')`,
`StatementContext.loadSnapshots()` invokes `loadLatestSnapshotFence()` before
this wrapped overload. That method still calls
`PaimonUtils.loadLatestSnapshotFence()` directly. If the FE names/object cache
retains `t` after a remote drop while its independent Paimon table entry has
expired, fence acquisition throws `Catalog.TableNotExistException` through the
cache loader before any `withMissingTableRepair` call is entered. The failed
query therefore leaves `SHOW TABLES` and cached object/ID lookup state stale,
and repeated OPTIONS queries bypass the repair that a plain SELECT now
performs. Wrap `loadLatestSnapshotFence()` with the same guarded repair and add
an OPTIONS-first missing-table test.
--
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]
