maytasm opened a new pull request, #20815:
URL: https://github.com/apache/druid/pull/20815
### Description
The Broker's result-level cache key comes only from the query's shape
(`CacheStrategy#computeResultLevelCacheKey`). It doesn't include the datasource
or the query intervals. Those only feed the ETag stored at the start of the
cached value (intervals were added there in #7325). So queries that differ only
by interval or datasource map to one cache entry. Each reads the other's entry,
rejects it because the ETag doesn't match, and overwrites it, so alternating
queries (a, b, a, b) never hit.
#### Added
`druid.broker.cache.includeDatasourceAndIntervalsInResultLevelCacheKey`
The default is `false`, which leaves the key unchanged. When `true`,
`ResultLevelCachingQueryRunner` builds the key from:
- a version byte,
- the strategy's result-level key,
- the datasource cache key (`DataSource#getCacheKey`), when it isn't null,
- each query interval's start and end millis, encoded the same way as
`CacheUtil#computeSegmentCacheKey`.
The ETag in the cached value, and the check on read, are unchanged. The flag
only decides which entry a query reads and writes.
Design notes:
- The flag is off by default because enabling it means existing result-level
entries are no longer read, and the cache stores more entries.
- When the datasource cache key is null, only the intervals are added. The
ETag is also null for those datasources, so nothing is cached for them either
way.
- Intervals come from the query's segment spec, which already condenses
them, so equivalent interval lists produce the same key.
- Trade-offs, also listed in the docs:
- The cache holds more entries.
- A query whose interval changes on every run, such as a rolling time
window, adds a new entry each run that is never read again.
- Queries whose intervals cover the same data but differ elsewhere no
longer reuse each other's entry.
- Alternative: always include the datasource and intervals, with no flag.
That changes the key for every deployment and adds the trade-offs above, so
this PR makes it opt-in.
#### Release note
Added
`druid.broker.cache.includeDatasourceAndIntervalsInResultLevelCacheKey`
(default `false`). When enabled, the Broker's result-level cache key includes
the query's datasource and intervals, so queries that differ only by interval
or datasource get their own cache entries instead of overwriting one shared
entry. Enabling it means existing result-level cache entries are no longer read.
<hr>
##### Key changed/added classes in this PR
* `CacheConfig`
* `ResultLevelCachingQueryRunner`
* `ResultLevelCachingQueryRunnerTest`
<hr>
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency
checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md)
(Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] a release note entry in the PR description.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked
related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in
[licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [ ] added comments explaining the "why" and the intent of the code
wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths,
ensuring the threshold for [code
coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md)
is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
--
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]
