Made storage::readers_cache size limitted
#16846
Merged
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
Signed-off-by: Michal Maslanka <michal@redpanda.com>
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45609#018e08ea-b646-4c8d-89b6-61fb399f6e11 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45609#018e08ea-b63f-4736-91cc-b69eb8f5783e ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45609#018e08fc-d372-4e56-91f4-d3bd574d0973 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45787#018e1859-f37c-4ccb-b528-d73d8615c537 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/45787#018e1859-f382-465a-b816-16fdc216a2d3 |
andrwng
reviewed
Mar 6, 2024
Added property which allows controlling maximum number of readers kept in cache per ntp. The value was previously unbounded which may lead to a situation in which readers cache grew and caused out of memory error. Signed-off-by: Michal Maslanka <michal@redpanda.com>
Added size based eviction to `storage::readers_cache`. Previously the eviction was only time based which may lead to a problem when multiple readers were created for the same NTP. Now with the size based eviction the readers cache size is bounded and will not grow indefinitely. Note on backward compatibility: Previously the size of cache was unbounded. In this commit we change the policy to keep up to 200 readers per ntp. This number is still large enough not cause any issues as it would mean that there are 200 concurrent readers reading a single partition at the same time. Signed-off-by: Michal Maslanka <michal@redpanda.com>
Added jitter to readers cache eviction timer to prevent all eviction timers from firing at the same time. Signed-off-by: Michal Maslanka <michal@redpanda.com>
mmaslankaprv
force-pushed
the
limit-readers-cache-size
branch
from
aa3965f
to
d3b07de
Compare
andrwng
approved these changes
Mar 7, 2024
| *this, | ||
| "readers_cache_target_max_size", | ||
| "Maximum desired number of readers cached per ntp. This a soft limit, a " | ||
| "number of readers in cache may temporary increase as cleanup is done in " |
|
LGTM! Thanks for changing the approach |
|
/backport v23.3.x |
|
/backport v23.2.x |
|
Failed to create a backport PR to v23.3.x branch. I tried: |
|
Failed to create a backport PR to v23.2.x branch. I tried: |
dotnwat
reviewed
Mar 8, 2024
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.
Added size based eviction to
storage::readers_cache. Previously theeviction was only time based which may lead to a problem when multiple
readers were created for the same NTP. Now with the size based eviction
the readers cache size is bounded and will not grow indefinitely.
Note on backward compatibility:
Previously the size of cache was unbounded. In this commit we change the
policy to keep up to 200 readers per ntp. This number is still large
enough not cause any issues as it would mean that there are 200
concurrent readers reading a single partition at the same time.
Backports Required
Release Notes
Improvements