Fix KetamaHashing panic on zero-weight backend killing load balancer background task - #1007
Open
sinhaparth5 wants to merge 1 commit into
Open
Fix KetamaHashing panic on zero-weight backend killing load balancer background task#1007sinhaparth5 wants to merge 1 commit into
sinhaparth5 wants to merge 1 commit into
Conversation
KetamaHashing::build_with_config passes Backend::weight straight into pingora_ketama::Bucket::new, which asserts weight != 0 and panics otherwise. Backend::weight is an unvalidated usize set by whatever ServiceDiscovery is in use (some treat 0 as a draining/disabled marker), so a single zero-weight backend from discovery reaches that assert. The panic happens inside LoadBalancer::update(), called from the background update loop in background.rs with no catch_unwind or spawn_blocking around it (unlike the newer LoadBalancerGroup rebuild path, which already guards against selector-build panics). The panicking task ends and is never restarted, so backend membership and health check state freeze permanently after the first such discovery response. Clamp the weight to a minimum of 1 at the point it crosses from Backend (untrusted, unvalidated) into Bucket (which requires a positive weight), instead of letting it panic.
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
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.
Fixes #1006.
Root cause
KetamaHashing::build_with_configpassesBackend::weight(an unvalidatedusize) straight intopingora_ketama::Bucket::new, which assertsweight != 0and panics otherwise. AServiceDiscoveryimplementation that uses weight 0 as a draining marker, or a config/DNS typo, is enough to trigger it.That call runs inside
LoadBalancer::update(), called from the background update loop inbackground.rswith nocatch_unwindorspawn_blockingaround it. The newerLoadBalancerGrouprebuild path already guards the equivalent call withspawn_blockingandJoinError::is_panic(), but the plainLoadBalancer<S>::run()path does not. Once the panic fires, the update task ends and is never restarted: backend membership and health check state freeze permanently (or the whole process aborts, underpanic = "abort"), from a single zero-weight entry in one discovery response.Fix
Clamp the weight to a minimum of 1 at the point it crosses from
BackendintoBucket, inKetamaHashing::build_with_config. This is the only place backend weights feed intopingora-ketamain this crate, so it is the right boundary to treat the value as untrusted input.Testing
test_ketama_zero_weight_backend_does_not_panic, which builds aKetamaHashingring with a zero-weight backend and checks it is still reachable.#[should_panic]version of the same test and confirmed it panics withweight must be at least one; the fixed commit does not panic and the same test passes.cargo test -p pingora-load-balancingpasses in full (79 tests) on the fixed commit.