github-actions[bot] commented on code in PR #69806:
URL: https://github.com/apache/doris/pull/69806#discussion_r4438439469


##########
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:
   [P2] Preserve guarded repair for partition-name reads too
   
   The new normalization still misses `SHOW PARTITIONS` and the `PARTITIONS` 
metadata path: `ShowPartitionsCommand.handleShowPluginDrivenTablePartitions` 
and `MetadataGenerator.dealPluginDrivenCatalog` call 
`metadata.getTableHandle`/`listPartitionNames` directly. With a warm FE 
identity and an expired or capacity-evicted Iceberg table-cache entry, a real 
source drop makes the handle lookup return empty, but neither caller retires 
the stale names, numeric ID route or metadata caches; `PARTITIONS` returns an 
OK empty batch. A handle cache hit followed by eviction also reaches 
`listPartitionNames`, whose remaining `NoSuchTableException` catch returns 
empty before this normalizer. Retain the source observation in those callers, 
use the guarded resolver, and preserve absence in `listPartitionNames`. This is 
a different SPI method and consumers from the already-fixed 
`partition_values`/MVCC `listPartitions` findings. Cover confirmed drop and 
delayed same-name recreation.



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java:
##########
@@ -212,47 +212,58 @@ private void applyTableRefresh(ExternalCatalog catalog, 
String localDbName, Stri
         boolean invalidateDatabase = invalidateTableLookup && 
catalog.getLowerCaseTableNames() != 0;
         ExternalMetaCacheMgr cacheMgr = 
Env.getCurrentEnv().getExtMetaCacheMgr();
         Optional<ExternalDatabase<? extends ExternalTable>> db = 
catalog.getDbForReplay(localDbName);
-        if (!db.isPresent()) {
-            try {
-                invalidateAllConnectorCachesIfPresent(catalog);
-            } finally {
-                if (invalidateDatabase) {
-                    cacheMgr.invalidateDb(catalogId, dbId, localDbName);
-                } else {
-                    cacheMgr.invalidateTable(catalogId, dbId, localDbName, 
tableId, localTableName);
-                }
-            }
-            LOG.info("database object cache is cold when refreshing table; "
-                            + "invalidated caches by local names {}.{} in 
catalog {}",
-                    localDbName, localTableName, catalogId);
-            return;
+        Optional<? extends ExternalTable> table = db.flatMap(database -> 
database.getTableForReplay(localTableName));
+        // Cold paths invalidate connector database/catalog caches without a 
retained table object.
+        boolean fenceLookup = invalidateTableLookup || !table.isPresent();

Review Comment:
   [P2] Fence metadata-only refreshes across normal object eviction
   
   For a hot metadata-only replay this condition skips the catalog fence; 
ordinary mutation/event refreshes also call `refreshTableInternal` directly. 
They only fence the retained table object. While `T_old` is refreshing, normal 
table-cache capacity/TTL eviction can admit `T_new` with the same name/ID and 
an idle generation. A `SHOW CREATE` read on `T_new` can capture valid 
observations, then delay a confirmed provider absence until after remote 
recreation, completion of `T_old`'s refresh and warming fresh metadata on 
`T_new`. Only `T_old`'s generation advances, so cleanup accepts the delayed 
failure and removes `T_new`'s fresh caches and numeric route. Keep the refresh 
interval in an owner that survives eviction for metadata-only refreshes too, 
without reloading names. This differs from the existing explicit wide-lookup 
invalidation finding: the separate public metadata-only path and normal cache 
eviction cause this replacement. Add that interleaving to the refresh tests.



-- 
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]

Reply via email to