jtuglu1 commented on code in PR #20815:
URL: https://github.com/apache/druid/pull/20815#discussion_r4504360086


##########
server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java:
##########
@@ -95,7 +98,7 @@ public Sequence<T> run(QueryPlus queryPlus, ResponseContext 
responseContext)
   {
     if (useResultCache || populateResultCache) {
 
-      final byte[] queryCacheKey = strategy.computeResultLevelCacheKey(query);
+      final byte[] queryCacheKey = computeResultLevelCacheKey();
       final Cache.NamedKey cacheKey = 
CacheUtil.computeResultLevelCacheKey(queryCacheKey);
       final byte[] cachedResultSet = 
fetchResultsFromResultLevelCache(cacheKey);

Review Comment:
   something I noticed while looking over 
[this](https://github.com/maytasm/druid/blob/f577780e1c7d044cddda8e629c3e8a1dadbae678/server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java#L366)
 code is that looks like there might? be a further way to increase hit rate. 
Consider 3 query instances that map to the same query cache key:
   
   ```
   q1: read cache, calculate E1 from old segments, start execution
   q2: read cache, calculate E2 from new segments, start execution
   q2: finish, write (key, {E2, results2})
   q1: finish, write (key, {E1, results1})
   q3: read cached {E1, results1}
   q3: calculate E2 from current segments
   q3: reject E1, execute query, write (key, {E2, results3})
   ```
   In order to prevent q3 from re-executing, we might be able to either:
   
   - Do some sort of cache.putIfAbsentOrOlder() which sets the result if it is 
not there yet or the etag represents a segment set that is "fresher" than the 
prior cached result.
   - Alternatively, can maybe think about storing the etag in the cache key 
itself which would blow up the cache size potentially (and you'd need eviction 
policy which is complicated)



-- 
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]

Reply via email to