Cache: Add maxEntrySize config, make groupBy cacheable by default. #5108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The idea is this makes it more feasible to cache query types that can potentially generate large result sets, like groupBy and select, without fear of writing too much to the cache per query. Includes a refactor of cache population code in CachingQueryRunner and CachingClusteredClient, such that they now use the same CachePopulator interface with two implementations: one for foreground and one for background. The main reason for splitting the foreground / background impls is that the foreground impl can have a more effective implementation of maxEntrySize. It can stop retaining subvalues for the cache early.
|
Removed WIP tag -- I have added tests and resolved conflicts. |
gianm
changed the title
| @@ -52,10 +52,13 @@ | |||
| private int cacheBulkMergeLimit = Integer.MAX_VALUE; | |||
|
|
|||
| @JsonProperty | |||
| private int resultLevelCacheLimit = Integer.MAX_VALUE; | |||
| private int maxEntrySize = 1_000_000; | |||
There was a problem hiding this comment.
What is a typical cache entry size? (how did you pick this number)
There was a problem hiding this comment.
In our cluster the average size is about 1KB. But I am not sure how representative this is. I chose 1MB because I figured it was a large number that will block egregiously large cache entries.
clintropolis
approved these changes
Jun 27, 2018
|
@gianm please fix the licenses. |
|
Oops, missed those. I pushed with new license headers. |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The idea is a maxEntrySize makes it more feasible to cache query types that
can potentially generate large result sets, like groupBy and select,
without fear of writing too much to the cache per query.
Includes a refactor of cache population code in CachingQueryRunner and
CachingClusteredClient, such that they now use the same CachePopulator
interface with two implementations: one for foreground and one for
background.
The main reason for splitting the foreground / background impls is
that the foreground impl can have a more efficient implementation of
maxEntrySize. It can stop retaining subvalues for the cache early.
Also includes:
put/ok,put/errors,put/oversized.