github-actions[bot] commented on code in PR #66913:
URL: https://github.com/apache/doris/pull/66913#discussion_r3870564425
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -110,7 +114,13 @@ public IcebergExternalMetaCache(ExecutorService
refreshExecutor, ExternalMetaCac
this::loadTableCacheValue, defaultEntryCacheSpec(),
MetaCacheEntryInvalidation.forNameMapping(nameMapping ->
nameMapping))
.withSizeEstimator(this::prepareTableForCachePublication)
- .withReplacementListener(this::retireTableGeneration));
+ .withReplacementListener(this::retireTableGeneration)
+ .withUnpublishedValueRetirer(IcebergTableCacheValue::retire)
+ .withRemovalListener(value -> value, (key, value) -> {
Review Comment:
[P1] Preserve cleanup ownership when soft values are collected
Weight-bounded entries use Caffeine soft values, and `MetaCacheEntry`
explicitly handles `COLLECTED` callbacks whose value is null. This extractor
therefore produces no token and the listener skips
`retireRemovedTableGeneration()`. The collected `IcebergTableCacheValue` was
the only owner of both its table-FileIO cleanup and catalog `ResourceLease`, so
memory-pressure collection can permanently pin a retired catalog/executor.
Unlike the existing unpublished-refresh case, this value was published but GC
erased it before the callback. Keep an independent strong cleanup token or
avoid soft references for resource-owning values, and cover a `COLLECTED`
notification with observable table and catalog cleanup.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalMetaCache.java:
##########
@@ -204,8 +236,7 @@ private Table createQueryTable(
public IcebergSnapshotCacheValue getSnapshotCache(ExternalTable
dorisTable) {
NameMapping nameMapping = dorisTable.getOrBuildNameMapping();
- IcebergTableCacheValue tableValue =
- tableEntry.get(nameMapping.getCtlId()).get(nameMapping);
+ IcebergTableCacheValue tableValue = statementValue(nameMapping);
Review Comment:
[P1] Keep partition projection on the captured authenticator
The caller has already entered this table generation's captured
authenticator, but `loadPartitionInfo()` immediately re-reads
`dorisTable.getCatalog().getExecutionAuthenticator()`. A concurrent reset can
make that nested value null or G2 while the frozen table/FileIO is still the
leased G1 generation, causing avoidable failure or mixed-generation credential
use before scan construction. This is distinct from the existing scan/sink
thread because the mismatch occurs inside snapshot projection before any scan
node exists. Pass the captured authenticator through (or make the outer
`executeForGeneration` the only authentication boundary), and add a partitioned
reset-barrier test.
--
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]
