github-actions[bot] commented on code in PR #69629:
URL: https://github.com/apache/doris/pull/69629#discussion_r4377031912
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalTable.java:
##########
@@ -1246,14 +1258,17 @@ public void beforeMTMVRefresh(MTMV mtmv) throws
DdlException {
@Override
public MvccSnapshot loadSnapshot(Optional<TableSnapshot> tableSnapshot,
Optional<TableScanParams> scanParams) {
- if (getDlaType() == DLAType.HUDI) {
- return HudiUtils.getHudiMvccSnapshot(tableSnapshot, this);
- } else if (getDlaType() == DLAType.ICEBERG) {
- return new IcebergMvccSnapshot(
- IcebergUtils.getSnapshotCacheValue(tableSnapshot, this,
scanParams));
- } else {
- return new EmptyMvccSnapshot();
- }
+ return withMissingTableRepair(() -> {
Review Comment:
[P2] Guard HMS capability discovery before metadata preload
With `enable_preload_external_metadata=true`, warm the names and an
uninitialized HMS object using `SHOW TABLES` plus `SHOW CREATE TABLE`, then
drop the remote table. A mixed internal/HMS query reaches this reduced
collection-stage plan:
```text
Project(*)
Join(a.k = b.k)
UnboundRelation(internal.db.local AS a)
UnboundRelation(ctl.db.t AS b)
```
`PreloadExternalMetadata.preloadExternalTable()` first calls
`supportsLatestSnapshotPreload()`, which invokes `getDlaType()` ->
`makeSureInitialized()` -> live HMS `loadHiveTable()`. The missing-table
exception therefore escapes before preload reaches this guarded
`loadSnapshot()` or `getBaseSchema()`. The internal OLAP relation enables the
preload gate, so repeated mixed queries leave the stale names/object/ID state
unrepaired. Guard the outer capability/init probe after the table monitor
unwinds, or make capability discovery avoid remote initialization, and cover
this SELECT-first path with preload enabled.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:
##########
@@ -393,6 +394,31 @@ public Optional<ColumnStatistic> getColumnStatistic(String
colName) {
return Optional.empty();
}
+ /** Only table-level remote reads use this seam; partition misses and
arbitrary failures do not repair names. */
+ protected <R> R withMissingTableRepair(Supplier<R> read) {
+ // Metadata validators can enter a schema read under this monitor.
Repair only after
+ // an outer unlocked entry point has unwound it, preserving catalog ->
table lock order.
+ if (Thread.holdsLock(this)) {
+ return read.get();
+ }
+ Optional<Runnable> observation = db.observeTableLookup(this);
Review Comment:
[P2] Cover the live HMS SHOW CREATE read with this guard
After warming `SHOW TABLES` and the cached HMS table object, drop `t`
through another catalog/engine and run `SHOW CREATE TABLE ctl.db.t` first.
`ShowCreateTableCommand.doRun()` calls
`HiveMetaStoreClientHelper.showCreateTable()` directly, which always invokes
the live HMS `getTable()`. Its `HMSClientException` retains the definite
`NoSuchObjectException`, but neither command validation nor this read enters
any of the newly wrapped schema/view/snapshot methods. Consequently this failed
table-level read leaves the names, object and ID caches intact: `SHOW TABLES`
still lists `t`, and repeated `SHOW CREATE` calls keep bypassing repair. Route
the live HMS DDL read through this guarded seam and add a SHOW CREATE-first
remote-drop test.
##########
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();
Review Comment:
[P2] Observe pinned reads without reopening a retired catalog
For `EXPLAIN ANALYZED PLAN SELECT 1 FROM ctl.db.t@branch(b)` on a Paimon
REST catalog (with `enable_materialized_view_rewrite=false`), binding has
already retained the branch table when
`LogicalFileScan.captureRelationSchema()` calls `getFullSchema(snapshot)`. Let
this observer pass the `initialized` check, then let another session change
`schema.cache.ttl-second` through `ALTER CATALOG`, resetting the catalog and
retiring this database. Resuming here calls `acquireTableMetadataLoadEpoch()`
-> `ExternalCatalog.acquireMetadataLoadEpoch()` -> `makeSureInitialized()`,
which reopens the REST catalog before attempting the retained schema read. If
its `/v1/config` endpoint is unavailable, the EXPLAIN now fails during
observation setup. Previously, the branch's `schemaFromSnapshotTable` path
projected the retained table's local schema; analyzed EXPLAIN returns before
descriptor serialization or scan block-rule checks, so neither required this
live initialization. Capture an already initi
alized generation without reopening the catalog, and abandon repair
observation when its parent/database has retired; add a
reset-versus-pinned-read regression.
--
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]
