feat(tonic-xds): implement gRFC A50 outlier detection success-rate algorithm by LYZJU2019 · Pull Request #2673 · grpc/grpc-rust

LYZJU2019 · 2026-06-04T17:32:50Z

Motivation

#2619 landed the failure-percentage algorithm and the shared sweep plumbing. This PR fills in the second gRFC A50 ejection algorithm: success-rate (cross-host mean/stdev).

Solution

In the housekeeping sweep, the success-rate algorithm now runs before the failure-percentage algorithm (A50 lists them in that order, and Envoy's implementation follows the same order; the two algorithms are otherwise independent and gated separately).

Tests

Seven new unit tests covering:

success_rate_ejects_outlier_below_threshold — 4× 100% + 1× 0%, mean=80, stdev=40, threshold@1900 = 4 → outlier ejected.
success_rate_uniform_population_does_not_eject — stdev=0 → threshold=mean, nothing strictly below.
success_rate_minimum_hosts_gates_ejection — qualifying<minimum_hosts → algorithm skipped.
success_rate_request_volume_filters_low_traffic — low-volume outlier excluded from population and candidates.
success_rate_enforcement_zero_never_ejects — enforcing_success_rate = 0 short-circuits the roll.
success_rate_stdev_factor_zero_ejects_below_mean — factor=0 collapses threshold to the mean.
success_rate_max_ejection_percent_caps_concurrent_ejections — cap (with floor) holds.
success_rate_and_failure_percentage_compose — both algorithms configured: success-rate ejects, failure-percentage skips already-ejected host (no double-count).

All 33 tests in client::loadbalance::outlier_detection pass, including the 26 from #2619.

…gorithm grpc#2619 landed the failure-percentage algorithm and the shared sweep plumbing. This change fills in the second A50 ejection algorithm: success-rate (cross-host mean/stdev). Algorithm (run before failure-percentage in the same sweep, gated by SuccessRateConfig being present on the cluster config): 1. For each host with total >= request_volume, compute its success rate as a percentage in 0.0..=100.0. 2. If fewer than minimum_hosts qualify, skip the algorithm. 3. Compute mean and population stdev of the success rates. 4. threshold = mean - stdev * stdev_factor / 1000 5. For each qualifying host whose success rate is strictly below the threshold, attempt ejection — subject to max_ejection_percent (with A50's at-least-one floor) and the enforcing_success_rate roll. Hosts already ejected (e.g., by a previous algorithm in this sweep) are skipped, and ejections feed into ejected_count so the subsequent failure-percentage pass respects the cap. Success-rate runs first because A50 lists it first and Envoy's implementation runs the algorithms in the same order; the two algorithms are otherwise independent. Seven new unit tests cover the outlier-below-threshold happy path, uniform-population no-eject (stdev = 0), minimum_hosts gating, request_volume filtering, enforcement = 0 no-op, stdev_factor = 0 collapsing to the mean, max_ejection_percent interaction, and combined success-rate + failure-percentage composition.

madhurishgupta · 2026-06-14T21:05:59Z

+                for (state, s, f) in &snapshots {
+                    let total = s + f;
+                    if total < request_volume || state.is_ejected() {
+                        continue;
+                    }
+                    if self.ejected_count.load(Ordering::Relaxed) >= max_ejections {
+                        break;
+                    }
+                    let rate = 100.0 * (*s as f64) / (total as f64);
+                    if rate >= threshold {
+                        continue;
+                    }
+                    if !roll(enforcing) {
+                        continue;
+                    }
+                    if state.try_eject(now) {
+                        self.ejected_count.fetch_add(1, Ordering::Relaxed);
+                        self.ejected_set_version.fetch_add(1, Ordering::Relaxed);
+                    }


This code is duplicated both for success_rate and failure_percentage logic except one line. Can you pleas create a new function for the common logic so that no drift is introduced in future between the two impls

addressed in 9438358

madhurishgupta · 2026-06-14T21:12:49Z

+                    if self.ejected_count.load(Ordering::Relaxed) >= max_ejections {
+                        break;
+                    }
+                    let rate = 100.0 * (*s as f64) / (total as f64);


If total and request_volume are 0, then rate will (100*0)/0 which is NaN. Can this might lead to ejection of all the hosts

addressed in 9438358

madhurishgupta · 2026-06-14T21:25:00Z

+        let mut config = sr_config(1900, 10, 3);
+        config.max_ejection_percent = pct(20);
+        let registry = make_registry_only(config);
+        // 4 hosts at 100%, 1 at 0%. The outlier is the only candidate


There is only 1 bad host and max cap is also 1. So this test will never breach the cap. Maybe keep number of bad hosts > 1.

addressed in 9438358

Extract the per-host ejection walk shared by the success-rate and failure-percentage algorithms into `eject_outliers`, parameterized by an `is_outlier` predicate, so the two paths can no longer drift apart. Guard against zero-traffic hosts: the shared loop skips `total == 0` and the success-rate statistics exclude zero-total hosts, so neither the floating-point success rate goes `NaN` nor the integer failure-percentage ratio divides by zero when `request_volume` is 0. Strengthen the max-ejection-cap test to use two below-threshold hosts against a cap of one, so it actually exercises the cap instead of matching a lone outlier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

madhurishgupta

🚢

ankurmittal · 2026-09-02T19:55:54Z

+            if total == 0 || total < request_volume || state.is_ejected() {
+                continue;
+            }
+            if self.ejected_count.load(Ordering::Relaxed) >= max_ejections {


This is the comparison, against a cap computed in max_ejections:

let cap = len * u64::from(config.max_ejection_percent.get()) / 100; if len > 0 { cap.max(1) } else { 0 }

A50 does it differently:

If the percentage of ejected addresses is greater than or equal to
max_ejection_percent, stop.

addressed in 26923d9

ankurmittal · 2026-09-02T19:57:16Z

+        }
+    }
+
+    /// minimum_hosts gate: only 2 hosts meet request_volume but


success_rate_minimum_hosts_gates_ejection sets minimum_hosts = 5 but only
registers 2 hosts. Both 2 >= 5 and 2 > 5 are false, so you could flip that
comparison and the test would still pass.

addressed in c0fac85

ankurmittal · 2026-09-02T19:58:27Z

+            if !roll(enforcing) {
+                continue;
+            }
+            if state.try_eject(now) {


If a host is removed while the sweep is running, we can still eject it, and
nothing ever un-ejects it.

The sweep snapshots host A. Not ejected yet.

An EDS update removes A. It wasn't ejected, so nothing decrements.

The sweep reaches it anyway, ejects it, and bumps ejected_count.

The load balancer drives un-ejection, and it no longer knows about A. The
broadcast rebuilds from channels, where A is gone. So the count stays up for
good.

addressed in 3c6272f

ankurmittal · 2026-09-02T20:00:29Z

We read the counters at the start of the sweep and reset them at the end. Any
request that finishes in between is dropped

A50 resets the counters first and runs the algorithms afterwards, so there's
nothing to lose. The window is small next to a 10s interval, but the
success-rate pass added here makes it longer.

addressed in f011584

A50 step 2 swaps (resets) each address's call-counter buckets before the success-rate and failure-percentage algorithms run in steps 3-4. The sweep instead read the counters at the start and reset them at the end, so any outcome recorded while the sweep was in flight was discarded by the trailing reset. The window widened with the success-rate pass. Snapshot-and-reset in a single step when the snapshot is taken, matching A50 and the internal implementation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

A50's success-rate and failure-percentage algorithms both re-check "if the percentage of ejected addresses is >= max_ejection_percent, stop" before each ejection. The sweep instead precomputed `floor(count * pct / 100)` and capped against that, which truncates: a 4-endpoint cluster at 30% should eject 2 (0% and 25% are below 30%), but `floor(1.2) = 1` under-ejected. Replace the precomputed cap with a per-ejection `can_eject_more` check that matches the spec, still allowing the first ejection regardless of the percentage. Adds a regression test for the 4-endpoint / 30% boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The sweep snapshots hosts, then may eject them later in the pass. If an EDS update removes a host after the snapshot but before it is ejected, the old code still ejected the stale snapshot and bumped `ejected_count`. Because the host was not ejected when `remove_channel` ran, nothing decremented it, and the un-ejection path (which works off `channels`) no longer knows the host — so the count stayed inflated and throttled future ejections. Eject through the DashMap entry and verify identity with `Arc::ptr_eq`, skipping a host that was removed (or removed and re-added) mid-sweep. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

`success_rate_minimum_hosts_gates_ejection` registered 2 hosts against `minimum_hosts = 5`; both `2 >= 5` and `2 > 5` are false, so the gate's comparison could be flipped without failing the test. Replace it with a boundary case that registers exactly `minimum_hosts` qualifying hosts and asserts the outlier is ejected, which only holds for `>=`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

madhurishgupta reviewed Jun 14, 2026

View reviewed changes

LYZJU2019 and others added 2 commits September 1, 2026 11:37

Merge branch 'master' into lyzju2019/a50-outlier-detector-success-rate

c05e828

madhurishgupta approved these changes Sep 1, 2026

View reviewed changes

ankurmittal reviewed Sep 2, 2026

View reviewed changes

LYZJU2019 and others added 4 commits September 3, 2026 14:42

Uh oh!

Conversation

LYZJU2019 commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

Tests

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

madhurishgupta left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

LYZJU2019 commented Jun 4, 2026 •

edited

Loading