maytasm commented on code in PR #20815:
URL: https://github.com/apache/druid/pull/20815#discussion_r4456352068
##########
server/src/main/java/org/apache/druid/query/ResultLevelCachingQueryRunner.java:
##########
@@ -206,6 +209,27 @@ private byte[] fetchResultsFromResultLevelCache(
return null;
}
+ private byte[] computeResultLevelCacheKey()
+ {
+ final byte[] queryCacheKey = strategy.computeResultLevelCacheKey(query);
+ if (!cacheConfig.isIncludeDatasourceAndIntervalsInResultLevelCacheKey()) {
+ return queryCacheKey;
+ }
+ // The cached value still starts with the ETag, which is checked before
the entry is used. Adding the datasource
+ // and intervals to the key gives queries that differ only by interval or
datasource their own entries, instead of
+ // one shared entry that each of them overwrites.
+ final CacheKeyBuilder builder = new
CacheKeyBuilder(RESULT_LEVEL_CACHE_KEY_WITH_DATASOURCE_AND_INTERVALS)
+ .appendByteArray(queryCacheKey);
Review Comment:
This shouldn't be a problem:
- Integer overflow: CacheKeyBuilder tracks the key size as an int, so
overflowing it would take a key over 2 GiB. This is unlikely.
- Backend key-length limits: Distributed cache like memcached store the
SHA-1 hex of the key (i.e. MemcachedCache.computeKeyHash), so their
250-character limit never applies. Caffeine and the map cache have no limit.
- Added size from this change: the flag adds the datasource key (its name
plus about 3 bytes) and 18 bytes per interval. The existing query key already
holds the filters, dimensions and aggregators, which are usually much bigger.
--
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]
