FrankChen021 commented on code in PR #19548:
URL: https://github.com/apache/druid/pull/19548#discussion_r4182587339
##########
processing/src/main/java/org/apache/druid/query/dimension/LookupDimensionSpec.java:
##########
@@ -136,18 +140,60 @@ public String getName()
@Override
public ExtractionFn getExtractionFn()
{
- final LookupExtractor lookupExtractor;
+ final LookupExtractor lookupExtractor = getLookupExtractor(null);
+ return makeLookupExtractionFn(lookupExtractor);
+ }
+
+ @Override
+ public ExtractionFn getExtractionFn(final Closer closer)
Review Comment:
[P1] Give retained extraction functions a deterministic owner
**Finding:** This new overload establishes the correct lifetime contract,
but the no-closer path still acquires a RetainedLookupExtractor and relies on
Cleaner/GC. Runtime callers still capture or apply the resulting function
without an owner: TopNQueryQueryToolChest and GroupByQueryQueryToolChest retain
it in result post-processing functions, while UseIndexesStrategy and
IndexedTableColumnSelectorFactory already have closers available but do not
pass them here. A long-running or queued query can therefore keep a retired
namespace cache referenced; with the default maxRetiredCacheEntries=1,
subsequent lookup refreshes are skipped until GC or the retired-cache timeout.
**Suggestion:** Thread the query/cursor closer through every runtime path
that applies the extraction function, including the existing closers in search
and indexed-table joins, and attach a closer to result post-processing
sequences; reserve the no-closer accessor for deliberately Cleaner-managed
compatibility paths.
--
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]
