jtuglu-netflix commented on code in PR #17652:
URL: https://github.com/apache/druid/pull/17652#discussion_r1925881332
##########
server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java:
##########
@@ -152,6 +153,8 @@ public void after(boolean isDone, Throwable thrown)
// The resultset identifier and its length is cached along
with the resultset
resultLevelCachePopulator.populateResults();
log.debug("Cache population complete for query %s",
query.getId());
+ } else { // thrown == null &&
!resultLevelCachePopulator.isShouldPopulate()
+ log.error("Failed (and recovered) to populate result level
cache for query %s", query.getId());
Review Comment:
The block will be hit when `thrown == null and
!resultLevelCachePopulator.isShouldPopulate()`. In this case, `thrown`
represents when an irrecoverable exception was found
[here](https://github.netflix.net/corp/bdp-druid/blob/netflix/31.0.x/server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java#L311),
where we re-throw the exception.
`resultLevelCachePopulator.isShouldPopulate()` references the case when we hit
an exception that we can definitively recover from and that we know how to
handle properly (e.g `IOException`). That is where `stopPopulating()` is called.
The distinction is errors we can (and should) effectively recover from and
those where we should re-throw (fail the query).
I can switch to `Failed (gracefully) ...`.
--
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]
