maytasm commented on code in PR #17823:
URL: https://github.com/apache/druid/pull/17823#discussion_r2008313011
##########
server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java:
##########
@@ -106,6 +106,8 @@ public Sequence<T> run(QueryPlus queryPlus, ResponseContext
responseContext)
if (useResultCache && newResultSetId != null &&
newResultSetId.equals(existingResultSetId)) {
log.debug("Return cached result set as there is no change in
identifiers for query %s ", query.getId());
+ // Call accumulate on the sequence to ensure that all
Wrapper/Closer/Baggage/etc. get called
+ resultFromClient.accumulate(null, (accumulated, in) -> accumulated);
Review Comment:
Yeah, I know.
The newResultSetId (ETag) is computed in CachingClusteredClient, which is
many many layers down from ResultLevelCachingQueryRunner. It's not simple to
compute that at this layer due to needing timeline, segmentServers, etc.
Also, the resultFromClient (Sequence) doesn't have a simple close() method
so I end up having to call the accumulate. The Sequence is Mapped, Wrapped,
etc. making it hard to access the resources/closers
But the Sequence retruend from CachingClusteredClient when the ETag matches
(ResultLevelCache hits) is just an empty Sequence so this call should not be
expensive.
--
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]
