maytasm commented on code in PR #9280:
URL: https://github.com/apache/druid/pull/9280#discussion_r4318256510
##########
processing/src/main/java/org/apache/druid/query/CacheStrategy.java:
##########
@@ -48,21 +50,27 @@
boolean isCacheable(QueryType query, boolean willMergeRunners);
/**
- * Computes the cache key for the given query
+ * Computes the per-segment cache key for the given query. Because this is a
per-segment cache key, it should only
+ * include parts of the query that affect the results for a specific segment
(i.e., the results returned from
+ * {@link QueryRunnerFactory#createRunner}).
*
- * @param query the query to compute a cache key for
+ * @param query the query to be cached
*
- * @return the cache key
+ * @return the per-segment cache key
*/
byte[] computeCacheKey(QueryType query);
/**
- * Computes the result level cache key for the given query.
- * Some implementations may include query parameters that might not be used
in {@code computeCacheKey} for same query
+ * Computes the result-level cache key for the given query. The result-level
cache will tack on datasource and
Review Comment:
@gianm
> Fwiw, I think it would be good to modify ResultLevelCachingQueryRunner to
make this part of the key, not part of the value, so it can cache results for
queries that differ in interval but are the same otherwise. I don't have plans
to do this personally though, at least not right now.
Is there a reason the datasource and intervals were left out of the key, or
does something depend on one entry per query shape? Adding the query interval
to the ETag rather than the key, stopped different intervals from returning
wrong results, but they still share one entry and overwrite each other. The one
trade-off I can see: a query with a rolling "now" interval would add a new
entry on every refresh instead of overwriting one. Those never hit today
either, since the interval is in the ETag, so no hits would be lost.
Size-bounded caches would see more churn, though.
Some issues with datasource and interval not in the key:
- **Metadata queries share one entry.** A metadata service polls datasources
with an identical query shape, so they all land on one key and overwrite each
other.
- **Alternating intervals never hit.** The same query run over intervals a,
b, a, b gets zero hits today, because each run overwrites the other's entry.
With intervals in the key, every repeat would hit as long as the segments don't
change.
--
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]
