924060929 commented on code in PR #69806:
URL: https://github.com/apache/doris/pull/69806#discussion_r4421205234
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -212,10 +214,19 @@ public PluginDrivenScanNode(PlanNodeId id,
TupleDescriptor desc,
Review Comment:
Fixed in 781100cd8f08. The synchronous planScan call now executes through
the scan node's existing pre-handle MetadataRead, preserving source identity
and the original refresh/recreation guards while keeping plugin TCCL and
profile-finalization boundaries intact. Iceberg's system-base-table loader uses
the existing confirmed-absence classification for direct and wrapped
NoSuchTableException; every non-absence failure is returned unchanged,
preserving the V1 system-scan error contract.
The SDK regression warms a real shared table cache, obtains a REST labels
handle and fixed schema without loading the table into the statement,
invalidates the shared cache and injects a concurrent drop before planScan. It
now returns typed source absence; a missing-file error remains the identical
original exception. FE tests exercise real getSplits for source cleanup and
delayed same-name recreation. The SDK regression and two FE regressions fail
before the repair.
Validation: 1003 focused unit cases passed across 83 classes; FE build and
Checkstyle passed. Controlled providers, not a live external-service
integration run.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalDatabase.java:
##########
@@ -92,6 +93,9 @@ public abstract class ExternalDatabase<T extends
ExternalTable>
private FeMetaCacheEntry<String, NameCacheValue> tableNames;
private FeMetaCacheEntry<String, T> tables;
private transient IdNameIndex tableIdNameIndex = new IdNameIndex("external
table");
+ // Guarded by extCatalog. Lookup refresh can replace table objects before
invalidation completes.
+ private transient long tableLookupRefreshGeneration;
+ private transient int activeTableLookupRefreshes;
private volatile boolean isInitializing = false;
Review Comment:
Fixed in 781100cd8f08. Removed the raw-remote folded-name test from this
suppression guard. It now compares retained mapped LOCAL identities using the
same ROOT fold as cleanup; a surviving remote FOO mapped to foo_new cannot
suppress cleanup of dropped Foo mapped to foo_old. Existing same-local-alias
replacement guards remain. The mode-1 test uses the real FE name cache and
metadata mapping seam, drops only Foo, refreshes names while retaining the old
object, and verifies the old numeric route is retired while foo_new remains
reloadable by ID. It fails on the previous behavior.
Validation: 1003 focused unit cases passed across 83 classes; FE build and
Checkstyle passed. Controlled providers, not a live external-service
integration run.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -894,7 +932,8 @@ public Optional<String> getShowCreateTableDdl() {
if (!handleOpt.isPresent()) {
return Optional.empty();
}
- return metadata.renderShowCreateTableDdl(session, handleOpt.get());
+ return readConnectorMetadata(observeConnectorTableLookup(connector),
session, metadata,
+ () -> metadata.renderShowCreateTableDdl(session,
handleOpt.get()));
}
Review Comment:
The latest review re-carries this previously resolved suggestion. The scope
decision remains unchanged: enabled comment caches intentionally memoize reads
until their configured TTL or explicit invalidation. A valid cache hit does not
confirm provider absence. Probing before every comment display would change
that freshness/performance contract, which is outside this repair. Actual
confirmed absence still reaches guarded cleanup. The new REST labels regression
also explicitly preserves cache-hit behavior before invalidation while testing
absence discovered by a later real provider read.
Validation: 1003 focused unit cases passed across 83 classes; FE build and
Checkstyle passed. Controlled providers, not a live external-service
integration run.
--
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]
