Limit the maximum amount of snapshots that may be applied per shard #27456
Conversation
This file contains hidden or 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
Seastar's underlying semaphore backs its count with ssize_t, but allows initialization from size_t. A naiive default for "unlimited capacity" would be size_t::max. This commit adds a limit check to adjustable_semaphore to ensure this signed integer is not overflowed
adds raft_max_load_local_snapshots_per_shard to configuration. This parameter will allow a user to limit the number of snapshots that can be loaded from disk concurrently per shard. This parameters is bounded between [1, uint32_t::max()]
This commit implements the limiting of the loading of snapshots per shard by adding a threadlocal static semaphore which will track the numeber of in-flight snapshot load operations against the configured limit. Configuration will be applied at the beginning of each snapshot load.
|
This is a draft for the moment while I figure out how to test this. |
joe-redpanda
commented
Sep 3, 2025
| @@ -514,6 +514,14 @@ configuration::configuration() | |||
| {.needs_restart = needs_restart::no, .visibility = visibility::tunable}, | |||
| 0, | |||
| {.min = 0, .max = 100_MiB}) | |||
| , raft_max_load_local_snapshots_per_shard( | |||
There was a problem hiding this comment.
we already have a "maximum recoveries per shard" config
Are we double dipping on our configuration options?
joe-redpanda
commented
Sep 3, 2025
| @@ -514,6 +514,14 @@ configuration::configuration() | |||
| {.needs_restart = needs_restart::no, .visibility = visibility::tunable}, | |||
| 0, | |||
| {.min = 0, .max = 100_MiB}) | |||
| , raft_max_load_local_snapshots_per_shard( | |||
| *this, | |||
| "raft_max_load_local_snapshots_per_shard", | |||
There was a problem hiding this comment.
fix description
|
/dt |
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 goal of this PR is to add limits on the maximum number of local snapshots that may be loaded at a given time to a configurable value.
This is accomplished by adding a new configuration value for this, alongside a theadlocal static semaphore to persisted_stm which will gate local snapshot loads.
There are additionally some boundary check fixes for adjustable_semaphore.
Backports Required
Release Notes