github-actions[bot] commented on code in PR #66913: URL: https://github.com/apache/doris/pull/66913#discussion_r4056147996
########## fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java: ########## @@ -37,7 +37,11 @@ import org.apache.doris.datasource.metacache.MetaCacheSizeEstimate; import org.apache.doris.datasource.metacache.MetaCacheSizeEstimator; import org.apache.doris.mtmv.MTMVRelatedTableIf; +import org.apache.doris.nereids.StatementContext; +import org.apache.doris.qe.ConnectContext; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.iceberg.ManifestContent; Review Comment: [P2] Drain closed-generation manifest loads before dropping SDK caches This new `invalidateCatalog(catalogId)` route closes the manifest entry while retained scans are explicitly allowed to load through it. A load that starts or finishes after the one-time `ManifestFiles.dropCache` reaches `ManifestFiles.read`/`readDeleteManifest` and can recreate the per-FileIO content cache for the retired generation, so invalidating entries does not guarantee cleanup after reset. Please add a barrier/repeated drop or bypass SDK content caching for closed-generation loads, with a deterministic race test. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java: ########## @@ -37,7 +37,11 @@ import org.apache.doris.datasource.metacache.MetaCacheSizeEstimate; import org.apache.doris.datasource.metacache.MetaCacheSizeEstimator; import org.apache.doris.mtmv.MTMVRelatedTableIf; +import org.apache.doris.nereids.StatementContext; +import org.apache.doris.qe.ConnectContext; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import org.apache.commons.lang3.exception.ExceptionUtils; Review Comment: [P1] Preserve scoped access for display-schema callers `getIcebergTable` now throws whenever there is no `StatementContext`, but `IcebergExternalTable.getBaseSchemaForDisplay()` still calls it directly. `IndexInfoProcDir.lookup` therefore fails, while `FrontendServiceImpl.describeTables` catches the exception and returns an empty schema for Iceberg tables. Please route display-schema loading through `withIcebergTable` (or another short-lived borrower) so no-context metadata callers retain a valid generation. -- 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]
