Add cross-scheme slow-start warm-up ramp for newly added servers - #3526
Open
rajvarun77 wants to merge 3 commits into
Open
Add cross-scheme slow-start warm-up ramp for newly added servers#3526rajvarun77 wants to merge 3 commits into
rajvarun77 wants to merge 3 commits into
Conversation
When a server joins a LoadBalancer (scale-up, restart, redeploy),
every policy immediately sends it a full traffic share while its
caches, JIT and connection pools are still cold, spiking tail
latency; latency-feedback policies (la/p2c) then punish the cold
server and oscillate between starving and slamming it.
-lb_warmup_ms (default 0, disabled) ramps a newly added server from
about 10% of its normal share to 100% over the window and
-lb_warmup_curve (default 1, linear) shapes the ramp, similar to
Envoy slow_start's aggression parameter.
The ramp math lives once in load_balancer.{h,cpp} and works off a
per-server join timestamp recorded when the server is added: la and
p2c multiply the ramp into their weights so it composes with latency
scoring, while rr/wrr/random/consistent-hashing divert selections
probabilistically to the next candidate. Re-adding a removed server
restarts the ramp; transient disconnections do not change LB
membership and keep it; servers added together at channel init ramp
together with unchanged relative shares. When disabled the only
per-selection cost is one gflag branch per candidate.
Includes unit tests (ramp math, disabled-by-default, reduced-share
integration for rr/wrr/chash/la/p2c, re-join restart) and docs in
cn/en client.md.
Promote the hardcoded 0.1 warm-up floor to a validated gflag in (0, 1],
mirroring Envoy slow_start's min_weight_percent, and document it in
docs/{cn,en}/client.md.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Flag validation and test flag-isolation should be tightened to match documented behavior and established repository test practices.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an opt-in slow-start (“warm-up”) ramp that applies consistently across multiple load balancer policies, reducing cold-start tail latency and oscillations when servers newly join or rejoin membership. The ramp logic is centralized in load_balancer.{h,cpp} and then composed into selection/weighting in each policy; docs and a dedicated unit test suite are included.
Changes:
- Introduce
-lb_warmup_ms,-lb_warmup_curve,-lb_warmup_min_weightplus centralizedWarmupMultiplier/WarmupAccepthelpers insrc/brpc/load_balancer.{h,cpp}. - Apply warm-up behavior across rr/wrr/random/consistent-hash (probabilistic diversion) and la/p2c (weight-based composition).
- Add
test/brpc_lb_warmup_unittest.cppand document the feature indocs/{en,cn}/client.md.
File summaries
| File | Description |
|---|---|
| test/brpc_lb_warmup_unittest.cpp | Adds unit coverage for ramp math and per-policy behavior (rr/wrr/random/la/p2c/chash). |
| src/brpc/load_balancer.h | Declares warm-up gflag + exposes warm-up helper APIs for policies. |
| src/brpc/load_balancer.cpp | Implements warm-up math/acceptance and defines new gflags. |
| src/brpc/policy/round_robin_load_balancer.h | Tracks per-server join timestamps for warm-up. |
| src/brpc/policy/round_robin_load_balancer.cpp | Applies warm-up acceptance during rr selection. |
| src/brpc/policy/randomized_load_balancer.h | Tracks per-server join timestamps for warm-up. |
| src/brpc/policy/randomized_load_balancer.cpp | Applies warm-up acceptance during random selection. |
| src/brpc/policy/weighted_round_robin_load_balancer.h | Adds join timestamp to server entries for warm-up. |
| src/brpc/policy/weighted_round_robin_load_balancer.cpp | Applies warm-up acceptance in wrr selection path. |
| src/brpc/policy/consistent_hashing_load_balancer.h | Adds per-node join timestamp carried on the ring. |
| src/brpc/policy/consistent_hashing_load_balancer.cpp | Assigns join timestamps to replicas and diverts along ring probabilistically during warm-up. |
| src/brpc/policy/locality_aware_load_balancer.h | Adds join timestamp into weight calculation state. |
| src/brpc/policy/locality_aware_load_balancer.cpp | Stamps join time and discounts weight while warming. |
| src/brpc/policy/p2c_ewma_load_balancer.h | Adds join timestamp to per-node stats. |
| src/brpc/policy/p2c_ewma_load_balancer.cpp | Composes warm-up multiplier into p2c score/weighting. |
| docs/en/client.md | Documents slow-start flags, behavior, and policy interactions. |
| docs/cn/client.md | Adds Chinese documentation for slow-start flags and semantics. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+52
to
+56
| static bool ValidateWarmupMinWeight(const char*, double v) { | ||
| return v > 0.0 && v <= 1.0; | ||
| } | ||
| BRPC_VALIDATE_GFLAG(lb_warmup_curve, PassValidate); | ||
| BRPC_VALIDATE_GFLAG(lb_warmup_min_weight, ValidateWarmupMinWeight); |
Contributor
Author
There was a problem hiding this comment.
Done in 251b7a9: ValidateWarmupCurve rejects non-positive values; help text updated; covered by the new flag_validation test.
Comment on lines
+85
to
+101
| class LbWarmupTest : public ::testing::Test { | ||
| protected: | ||
| void SetUp() override { | ||
| _saved_warmup_ms = brpc::FLAGS_lb_warmup_ms; | ||
| _saved_curve = brpc::FLAGS_lb_warmup_curve; | ||
| _saved_min_weight = brpc::FLAGS_lb_warmup_min_weight; | ||
| } | ||
| void TearDown() override { | ||
| brpc::FLAGS_lb_warmup_ms = _saved_warmup_ms; | ||
| brpc::FLAGS_lb_warmup_curve = _saved_curve; | ||
| brpc::FLAGS_lb_warmup_min_weight = _saved_min_weight; | ||
| } | ||
|
|
||
| int64_t _saved_warmup_ms; | ||
| double _saved_curve; | ||
| double _saved_min_weight; | ||
| }; |
Contributor
Author
There was a problem hiding this comment.
Done in 251b7a9: fixture now holds a GFLAGS_NAMESPACE::FlagSaver member instead of the manual save/restore.
The ramp formula progress^lb_warmup_curve is only meaningful for a positive exponent; reject other values at flag-parse time instead of silently falling back to a linear ramp. The test fixture now relies on GFLAGS_NAMESPACE::FlagSaver to restore flags, as other tests in the repo do, and a new test covers the validators of both flags.
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.
What & Why
A server that just joined the cluster or restarted is "cold" (empty caches, unwarmed JIT, unestablished connection pools). Every load balancer hands it a full traffic share immediately, causing cold-start tail-latency spikes — and latency-feedback schemes (la, p2c_ewma from #3367) can even oscillate: the cold server scores badly, gets starved, its stats decay, it gets slammed again. This PR adds an opt-in cross-scheme slow-start ramp, the same remedy Envoy ships as
slow_start.Usage
Design
AddServer(deliberately not onSocket— sockets are shared across channels, so a socket-level stamp would leak one channel's membership change into another's ramp).load_balancer.{h,cpp}(WarmupMultiplier/WarmupAccept), one implementation for all policies.laandp2c_ewmamultiply the ramp into their weight so it composes with latency scoring instead of fighting it;rr,wrr,random, and consistent hashing divert probabilistically (chash moves to the next ring node, temporarily diverting part of the hash affinity).Tests & Docs
8 cases in
test/brpc_lb_warmup_unittest.cpp(disabled-by-default, ramp math incl. curve shaping and configurable floor, per-policy share convergence for rr/wrr/random/la/p2c/chash, last-chance exemption, re-add restart); fullbrpc_load_balancer_unittest(17/17) passing. Documented indocs/cn/client.mdanddocs/en/client.md.cc @chenBright @wwbmmm