Fix infinite loop OOM bug caused by rebalancing shards from inactive ingesters #6078
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
…ingesters Signed-off-by: Nadav Gov-Ari <[email protected]>
Signed-off-by: Nadav Gov-Ari <[email protected]>
Signed-off-by: Nadav Gov-Ari <[email protected]>
Signed-off-by: Nadav Gov-Ari <[email protected]>
fulmicoton
reviewed
Jan 8, 2026
| shards | ||
| }) | ||
| .collect(); | ||
| let target_max = total_open_shards.div_ceil(num_available_ingesters); |
Collaborator
There was a problem hiding this comment.
I think the point of using 0.9/1.1 was to add some tolerance to make sure rebalance do not happen too often.
fulmicoton
reviewed
Jan 8, 2026
| let mut per_leader_open_shards: HashMap<&str, Vec<&Shard>> = HashMap::new(); | ||
|
|
||
| let mut total_open_shards: usize = 0; | ||
| let mut orphaned_shards: Vec<Shard> = Vec::new(); |
Collaborator
There was a problem hiding this comment.
counting orphaned shards as needing to be moved is dangerous.
Right now moving a shard is done by opening a shard, and closing the shard moved.
Closing a shard requires contacting the node hosting it, so it can fail. In this exact case, it WILL fail because the ingester is not even in the pool.
Shards on inactive indexers cannot be moved, because they cannot be closed. For this reason, we remove them from the equation. Also, this PR reintroduce some leniency on our definition of what balanced means. We allow for a variation of 10% between the indexer with the highest number of shards and the indexer with the lowest number of shards. Signed-off-by: Nadav Gov-Ari <[email protected]>
fulmicoton-dd
force-pushed
the
nadav/dailymotion
branch
from
3602106 to
243d34e
Compare
Signed-off-by: Nadav Gov-Ari <[email protected]>
fulmicoton-dd
force-pushed
the
nadav/dailymotion
branch
2 times, most recently
from
fac5569 to
640b9f1
Compare
guilload
approved these changes
Jan 8, 2026
Comment on lines
3542
to
3543
| /// - `active_ingester_shards`: shards per active ingester (these are in the pool) | ||
| /// - `inactive_ingester_shards`: shards per inactive ingester (NOT in the pool - orphaned) |
Member
There was a problem hiding this comment.
Suggested change
| /// - `active_ingester_shards`: shards per active ingester (these are in the pool) | |
| /// - `inactive_ingester_shards`: shards per inactive ingester (NOT in the pool - orphaned) | |
| /// - `available_ingester_shards`: shards per available ingester (shards whose leader is in the pool) | |
| /// - `unavailable_ingester_shards`: shards per unavailable ingester (NOT in the pool - orphaned) |
Member
There was a problem hiding this comment.
I don't understand the notion of orphaned. Aren't the shards whose leader is in NOT the pool the orphans already?
Collaborator
Author
There was a problem hiding this comment.
Yeah, the comments got jumbled in between revisions. Let me fix.
Comment on lines
3545
to
3546
| active_ingester_shards: &[usize], | ||
| inactive_ingester_shards: &[usize], |
Member
There was a problem hiding this comment.
s/active/available/ everywhere in this function.
Signed-off-by: Nadav Gov-Ari <[email protected]>
Signed-off-by: Nadav Gov-Ari <[email protected]>
Signed-off-by: Nadav Gov-Ari <[email protected]>
nadav-govari
force-pushed
the
nadav/dailymotion
branch
from
05b563d to
90f24ea
Compare
nadav-govari
force-pushed
the
nadav/dailymotion
branch
from
90f24ea to
dd46e2d
Compare
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.
Rebalances all shards from unavailable ingesters. Moves to only look at ingesters with more shards than allowed as candidates for rebalancing. Also moves to integer division to simplify target calculation.