github-actions[bot] commented on code in PR #69632:
URL: https://github.com/apache/doris/pull/69632#discussion_r4384700585
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -479,15 +488,21 @@ public boolean supportsLatestSnapshotPreload() {
@Override
public List<Column> getFullSchema() {
- // Descriptor serialization is table-only and may follow multiple
OPTIONS aliases. Keep it
- // on a validated statement projection rather than falling back to the
neutral cache.
- return getPaimonSchemaCacheValue(
-
MvccUtil.getSnapshotForTableMetadataFromContext(this)).getSchema();
+ return withMissingTableRepair(() -> {
Review Comment:
[P2] Guard Paimon comment reads as well as schema reads
The schema wrappers still leave getComment() -> getBasePaimonTable() ->
PaimonUtils.getPaimonTable() outside repair. With
meta.cache.paimon.table.ttl-second=0, warm SHOW TABLES and DESC t, drop t
externally, then request TABLE_COMMENT through ctl.information_schema.tables.
FrontendServiceImpl.listTableStatus() calls getComment() before any schema
read; the base-table loader confirms absence, but status enumeration logs and
swallows its exception and subsequent SHOW TABLES still lists t. Enclose the
actual Paimon comment/base-table read in the repair boundary and add a
status-first negative test. This is a separate engine/accessor from the fixed
Iceberg properties/status thread, and neither existing Paimon OPTIONS test
enters this path.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalTable.java:
##########
@@ -403,12 +405,17 @@ public boolean isPartitionColumnAllowNull() {
@Override
public MvccSnapshot loadSnapshot(Optional<TableSnapshot> tableSnapshot,
Optional<TableScanParams> scanParams) {
- return new
PaimonMvccSnapshot(getPaimonSnapshotCacheValue(tableSnapshot, scanParams));
+ return withMissingTableRepair(() -> {
Review Comment:
[P2] Do not classify a missing Paimon branch as a missing base table
This new boundary also encloses the branch arm of
getPaimonSnapshotCacheValue(). If an external drop removes a branch after
resolvePaimonBranch() checks branchExists() but before the cold
getPaimonTable(..., branch, null), [Paimon 1.4.2 loads that branch's schema and
throws
TableNotExistException](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/catalog/FileSystemCatalog.java#L103-L106).
The main table still exists, but withMissingTableRepair() matches only the
exception type and retires its lookup. With lower_case_table_names != 0 this
clears every table object and idToName route in the DB. Background numeric
lookups, including ExternalRowCountCache -> StatisticsUtil.findTable(), then
cannot reload even surviving siblings until a named object load/registration
restores their IDs; SHOW TABLES only restores names. Scope repair to a
confirmed base/main-table failure, and cover a branch drop between validation
and loading with a retained
sibling ID. This differs from the prior transient-enumeration finding: the
missing object here is a branch after a successful base-table read.
##########
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);
+ try {
+ return read.get();
+ } catch (RuntimeException failure) {
+ Throwable cause = failure;
+ while (cause != null) {
+ if (cause instanceof
org.apache.hadoop.hive.metastore.api.NoSuchObjectException
+ || cause instanceof
org.apache.iceberg.exceptions.NoSuchTableException
Review Comment:
[P2] Recognize the missing-object exceptions used by guarded reads
Two guarded readers confirm absence using exception types this whitelist
never matches:
- With the default HMS get_schema_from_table=false and
schema.cache.ttl-second=0, let initSchemaAndUpdateTime() finish getTable(),
then drop t externally before initHiveSchema() calls getSchema(). [Hive 3.1.3
converts the missing-table NoSuchObjectException into a new
UnknownTableException](https://github.com/apache/hive/blob/rel/release-3.1.3/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java#L4887-L4895)
without its cause. ThriftHMSCachedClient preserves UnknownTableException, so
the NoSuchObjectException check cannot repair this ordinary DESC/schema failure.
- IcebergExternalCatalog includes native views in FE table membership.
Configure meta.cache.iceberg.view.ttl-second=0 before warming a view's names
and status/comment, then drop it externally. The warm isView=true object enters
guarded properties()->getIcebergView(); the native loader's
[NoSuchViewException is a direct
RuntimeException](https://github.com/apache/iceberg/blob/apache-iceberg-1.11.0/api/src/main/java/org/apache/iceberg/exceptions/NoSuchViewException.java#L21-L22),
independently of NoSuchTableException.
Both paths propagate confirmed absence without running the retained
observation, leaving the deleted object in SHOW TABLES. Recognize these
missing-object types with the appropriate observed-object scope and add
negative tests for both. The Iceberg case differs from the fixed table/property
thread: its complete read is guarded, but its view exception is unrecognized.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalTable.java:
##########
@@ -276,12 +281,18 @@ protected boolean needInternalHiddenColumns() {
@Override
public List<Column> getFullSchema() {
- return getFullSchema(MvccUtil.getSnapshotFromContext(this));
+ return withMissingTableRepair(() -> {
+ return getFullSchema(MvccUtil.getSnapshotFromContext(this));
+
+ });
}
@Override
public List<Column> getFullSchema(Optional<MvccSnapshot> snapshot) {
- return IcebergUtils.withSnapshotCacheValue(snapshot, this,
this::projectFullSchema);
+ return withMissingTableRepair(() -> {
Review Comment:
[P2] Guard source loads during native system-table DESC
These source-table wrappers do not cover DescribeCommand's native
system-table path: DESC ctl.db.`t$snapshots` constructs IcebergSysExternalTable
and calls its getFullSchema() directly, which uses
IcebergUtils.withSnapshotCacheValue(empty, sourceTable, ...) without entering a
source repair boundary. A concrete interleaving is: with a warm source object
and meta.cache.iceberg.table.ttl-second=0, the initial synthetic-name miss
refreshes names while t still exists; then t is dropped externally after source
resolution but before the synthetic schema loads the base table. That load
reports NoSuchTableException, yet the refreshed names/object lookup survives
and SHOW TABLES still lists t. Normal system-table SELECT loads the source
snapshot through BindRelation and is guarded. Wrap native display loads using
the real source table's observation and add a controlled interleaving test.
This is distinct from the fixed ordinary getBaseSchemaForDisplay() thread
because a separate synthetic c
lass performs the read.
--
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]
