github-actions[bot] commented on code in PR #69806:
URL: https://github.com/apache/doris/pull/69806#discussion_r4426864286
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergExceptionUtils.java:
##########
@@ -31,12 +33,25 @@ private IcebergExceptionUtils() {
static RuntimeException wrapTableLoadFailure(
IcebergTableHandle handle, Exception failure, String
fallbackPrefix) {
+ if (isTableAbsent(failure)) {
Review Comment:
[P1] Preserve typed table absence through Iceberg row-count statistics.
`wrapTableLoadFailure` now turns a concurrent `NoSuchTableException` into
`ConnectorTableNotFoundException`, but both `getTableStatistics` overloads
catch that exception and return `Optional.empty()`.
`PluginDrivenExternalTable.fetchRowCount` calls statistics after resolving the
handle, so a drop observed only by this row-count read never reaches
`MetadataRead` and leaves the stale table ID/name and connector caches
resident. Route the statistics read through the retained observation (while
still degrading ordinary statistics failures) and cover latest plus pinned
row-count paths.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java:
##########
@@ -345,18 +351,19 @@ private static List<PartitionValue>
toPartitionValues(List<String> bound) {
* Both an opted-in latest pin and explicit time travel must use their
retained schema here.
* Unparseable partitions remain logged and skipped so the snapshot can
fall back to scan-all.
*/
- private PluginDrivenPartitionView listPartitions(ConnectorMetadata
metadata, ConnectorSession session,
+ private PluginDrivenPartitionView listPartitions(MetadataRead read,
ConnectorMetadata metadata,
+ ConnectorSession session,
ConnectorTableHandle handle, List<Column> partitionColumns) {
List<Type> types =
partitionColumns.stream().map(Column::getType).collect(Collectors.toList());
- Optional<String> version = metadata.getPartitionViewVersion(session,
handle);
+ Optional<String> version = read.execute(() ->
metadata.getPartitionViewVersion(session, handle));
if (version.isPresent()) {
PluginDrivenPartitionView.Key key =
PluginDrivenPartitionView.Key.versioned(
getId(), getDbName(), getName(), version.get(), types,
false);
return cachedPartitionView(key, k -> buildListPartitionView(
Review Comment:
[P1] Preserve typed absence when listing Iceberg partitions. `MetadataRead`
now wraps this `listPartitions` call, but Iceberg catches
`NoSuchTableException` inside both listing branches and returns an empty list
before `ConnectorTableNotFoundException` reaches the FE. A drop during MVCC
partition materialization or its deferred loader is therefore treated as an
empty partition view and leaves the stale table ID/name and connector caches.
Preserve the typed absence (while retaining ordinary listing degradation) and
add a drop/recreate regression for non-RANGE and deferred partition paths.
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -227,32 +227,41 @@ private void applyTableRefresh(ExternalCatalog catalog,
String localDbName, Stri
localDbName, localTableName, catalogId);
return;
}
- Optional<? extends ExternalTable> table =
db.get().getTableForReplay(localTableName);
- if (!table.isPresent()) {
- try {
- if (catalog instanceof PluginDrivenExternalCatalog) {
- // A cold table object cannot provide the mapped remote
table name.
- ((PluginDrivenExternalCatalog)
catalog).getConnector().invalidateDb(db.get().getRemoteName());
- }
- } finally {
+ if (invalidateTableLookup) {
Review Comment:
[P1] Fence cold-database refreshes before publishing replacements.
`applyTableRefresh` calls cache-only `getDbForReplay` before
`beginTableLookupRefresh`; when the database object is cold, the branch
invalidates connector/engine caches and returns without any database/catalog
generation. A concurrent `getDbNullable` can publish a replacement
database/table, then a delayed typed absence can pass the new table fence and
evict that replacement’s ID route and metadata after refresh. Extend the
catalog-level fence across this cold branch and the equivalent replay/rename
cold-database invalidation paths, and cover publication during the invalidation
window.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/scan/PluginDrivenScanNode.java:
##########
@@ -301,6 +312,10 @@ private ConnectorMetadata metadata() {
return m;
}
Review Comment:
[P1] Route batch and streaming split reads through the retained observation.
`readMetadata` currently protects synchronous planning, properties, and column
handles, but `computeBatchMode` invokes `streamingSplitEstimate` directly and
deferred `BatchSource` callbacks invoke `streamSplits` directly. Iceberg table
absence is typed in both provider paths, so EXPLAIN or an async batch producer
can leave stale shared identity and connector caches. Wrap these calls through
`MetadataRead` while preserving each TCCL boundary and producer lifetime, and
add drop-before-EXPLAIN/streaming regressions.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -119,11 +121,24 @@ public PluginDrivenExternalTable(long id, String name,
String remoteName,
*/
public Optional<ConnectorTableHandle> resolveConnectorTableHandle(
Review Comment:
[P1] Retain the table observation through `partition_values` metadata
listing. `getNameToPartitionValues` resolves a handle and calls
`metadata.listPartitions` directly, so the `partition_values` metadata path
bypasses `MetadataRead`. Iceberg converts a concurrent `NoSuchTableException`
to an empty list in that call, leaving the stale table ID/name and connector
caches resident. Execute this listing through the retained observation and add
a drop/recreate metadata-TVF regression.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalTable.java:
##########
@@ -139,14 +154,63 @@ public Optional<ConnectorTableHandle>
resolveConnectorTableHandle(
}
}
});
- String dbName = db != null ? db.getRemoteName() : "";
- Optional<ConnectorTableHandle> handle =
metadata.getTableHandle(session, dbName, getRemoteName());
- // A view has no table handle. Check live view existence before
treating an empty handle as a
- // dropped table; the cached isView flag can outlive a remote view
deletion.
- if (!handle.isPresent() && !metadata.viewExists(session, dbName,
getRemoteName())) {
- observation.ifPresent(Runnable::run);
+ }
+
+ /** A synthetic system relation repairs the shared source identity rather
than its transient name. */
+ protected PluginDrivenExternalTable lookupRepairSource() {
+ return this;
+ }
Review Comment:
[P1] Retain the table observation through row-level DML validation.
`validateConnectorRowLevelDml` resolves a handle and then calls
`metadata.validateRowLevelDmlMode` directly, bypassing the new `MetadataRead`
seam. Iceberg loads the table in that validator and now reports typed absence,
so a drop after handle resolution fails DML analysis while leaving the stale
table ID/name and connector caches. Capture a `MetadataRead` before resolving
the handle and execute the validator through it, then cover a concurrent-drop
DML case.
--
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]
