From 9bdcf87247abc625fef450a6cee70102951453d5 Mon Sep 17 00:00:00 2001 From: rajvarun77 <287367605+rajvarun77@users.noreply.github.com> Date: Wed, 15 Jul 2026 01:40:24 -0400 Subject: [PATCH 1/3] Add cross-scheme slow-start warm-up for newly added servers 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. --- docs/cn/client.md | 8 + docs/en/client.md | 8 + src/brpc/load_balancer.cpp | 46 +++ src/brpc/load_balancer.h | 25 ++ .../consistent_hashing_load_balancer.cpp | 8 +- .../policy/consistent_hashing_load_balancer.h | 4 + .../policy/locality_aware_load_balancer.cpp | 1 + .../policy/locality_aware_load_balancer.h | 5 + src/brpc/policy/p2c_ewma_load_balancer.cpp | 6 +- src/brpc/policy/p2c_ewma_load_balancer.h | 4 +- src/brpc/policy/randomized_load_balancer.cpp | 7 +- src/brpc/policy/randomized_load_balancer.h | 2 + src/brpc/policy/round_robin_load_balancer.cpp | 7 +- src/brpc/policy/round_robin_load_balancer.h | 2 + .../weighted_round_robin_load_balancer.cpp | 12 +- .../weighted_round_robin_load_balancer.h | 5 +- test/brpc_lb_warmup_unittest.cpp | 283 ++++++++++++++++++ 17 files changed, 425 insertions(+), 8 deletions(-) create mode 100644 test/brpc_lb_warmup_unittest.cpp diff --git a/docs/cn/client.md b/docs/cn/client.md index e659403c24..dd667568d7 100755 --- a/docs/cn/client.md +++ b/docs/cn/client.md @@ -290,6 +290,14 @@ locality-aware,优先选择延时低的下游,直到其延时高于其他机 channel.Init("http://...", "random:min_working_instances=6 hold_seconds=10", &options); ``` +### 慢启动(预热) + +新加入集群或刚重启的server往往是“冷”的(缓存未命中、JIT未编译、连接池未建立),立即承担全量流量会推高其延时甚至过载。设置-lb_warmup_ms大于0(默认为0,即关闭)后,新加入负载均衡器的server先获得约10%的正常流量份额,并在该时间窗口内线性爬升到100%。该机制对rr、wrr、random、la、p2c和一致性哈希均生效:la和p2c把爬升系数乘入权重,与延时评分自然叠加而不会互相干扰;其余算法按该系数概率性地把请求转给其他server(一致性哈希转给环上的下一个节点,预热期间会有部分请求偏离原有的哈希亲和性)。 + +-lb_warmup_curve(默认1.0)控制爬升曲线:流量份额为max(0.1, progress^lb_warmup_curve),progress在窗口内从0线性升到1。大于1的值让新server冷得更久,小于1则更激进。 + +说明:预热的起点是server被加入负载均衡器的时刻。server被命名服务摘除后重新加入会重新预热;而短暂断连或健康检查失败不改变负载均衡器成员,不会重新预热。Channel初始化时所有server同时加入、一起爬升,相对流量比例不变,因此首次启动无需特殊处理。 + ## 健康检查 连接断开的server会被暂时隔离而不会被负载均衡算法选中,brpc会定期连接被隔离的server,以检查他们是否恢复正常,间隔由参数-health_check_interval控制: diff --git a/docs/en/client.md b/docs/en/client.md index 266eb90289..dda70d069d 100644 --- a/docs/en/client.md +++ b/docs/en/client.md @@ -291,6 +291,14 @@ This recovery mechanism requires the capabilities of downstream servers to be si channel.Init("http://...", "random:min_working_instances=6 hold_seconds=10", &options); ``` +### Slow start (warm-up) + +A server that just joined the cluster or restarted is often "cold" (empty caches, uncompiled JIT, unestablished connection pools); sending it a full traffic share immediately raises its latency or even overloads it. When -lb_warmup_ms is positive (default 0, disabled), a server newly added to a LoadBalancer gets about 10% of its normal traffic share at first and ramps up to 100% over the window. The mechanism works across rr, wrr, random, la, p2c and consistent hashing: la and p2c multiply the ramp into the weight so it composes with their latency scoring instead of fighting it; the other policies divert requests probabilistically to other servers (consistent hashing moves to the next node on the ring, so part of the hash affinity is temporarily diverted during warm-up). + +-lb_warmup_curve (default 1.0) shapes the ramp: the traffic share is max(0.1, progress^lb_warmup_curve) where progress rises linearly from 0 to 1 over the window. Values above 1 keep a new server colder for longer, values below 1 ramp more aggressively. + +Note: warm-up starts when the server is added to the LoadBalancer. A server removed by the naming service and added back restarts its ramp, while a transient disconnection or health-check failure does not change LB membership and keeps the ramp. At channel initialization all servers join and ramp together with unchanged relative shares, so initial startup needs no special casing. + ## Health checking Servers whose connections are lost are isolated temporarily to prevent them from being selected by LoadBalancer. brpc connects isolated servers periodically to test if they're healthy again. The interval is controlled by gflag -health_check_interval: diff --git a/src/brpc/load_balancer.cpp b/src/brpc/load_balancer.cpp index 0544a48367..6a51970969 100644 --- a/src/brpc/load_balancer.cpp +++ b/src/brpc/load_balancer.cpp @@ -16,7 +16,10 @@ // under the License. +#include // std::pow #include +#include "butil/fast_rand.h" // fast_rand_double +#include "butil/time.h" // gettimeofday_us #include "brpc/reloadable_flags.h" #include "brpc/load_balancer.h" #include "brpc/socket.h" @@ -30,7 +33,50 @@ DEFINE_int32(default_weight_of_wlb, 0, "Default weight value of Weighted LoadBal "problems when user is using wlb but forgot to set the weights of some of their " "downstream instances. Then these instances will be set default_weight_of_wlb as " "their weights. wlb policy degradation is not enabled by default."); +DEFINE_int64(lb_warmup_ms, 0, + "When positive, a server newly added to a LoadBalancer gets " + "about 10% of its normal traffic share at first and ramps up " + "to 100% over this period(ms). 0 disables the warm-up"); +DEFINE_double(lb_warmup_curve, 1.0, + "Shape of the warm-up ramp: the weight multiplier is " + "max(0.1, progress^lb_warmup_curve) where progress rises " + "linearly from 0 to 1 over lb_warmup_ms. 1 ramps linearly, " + "larger values keep a new server colder for longer"); BRPC_VALIDATE_GFLAG(show_lb_in_vars, PassValidate); +BRPC_VALIDATE_GFLAG(lb_warmup_ms, PassValidate); +BRPC_VALIDATE_GFLAG(lb_warmup_curve, PassValidate); + +// Floor of the warm-up multiplier so that a warming server still gets a +// trickle of traffic and latency-based policies keep observing it. +static const double WARMUP_MIN_RATIO = 0.1; + +double WarmupMultiplierImpl(int64_t join_time_us, int64_t now_us) { + const int64_t warmup_us = FLAGS_lb_warmup_ms * 1000L; + if (warmup_us <= 0 || join_time_us <= 0) { + return 1.0; + } + if (now_us <= 0) { + now_us = butil::gettimeofday_us(); + } + const int64_t elapsed_us = now_us - join_time_us; + if (elapsed_us >= warmup_us) { + return 1.0; + } + if (elapsed_us <= 0) { + // The clock went backwards, be conservative. + return WARMUP_MIN_RATIO; + } + double progress = (double)elapsed_us / (double)warmup_us; + if (FLAGS_lb_warmup_curve > 0 && FLAGS_lb_warmup_curve != 1.0) { + progress = std::pow(progress, FLAGS_lb_warmup_curve); + } + return std::max(progress, WARMUP_MIN_RATIO); +} + +bool WarmupAcceptImpl(int64_t join_time_us, int64_t now_us) { + const double m = WarmupMultiplierImpl(join_time_us, now_us); + return m >= 1.0 || butil::fast_rand_double() < m; +} // For assigning unique names for lb. static butil::static_atomic g_lb_counter = BUTIL_STATIC_ATOMIC_INIT(0); diff --git a/src/brpc/load_balancer.h b/src/brpc/load_balancer.h index 2a76fa4305..403d8561cf 100644 --- a/src/brpc/load_balancer.h +++ b/src/brpc/load_balancer.h @@ -113,6 +113,31 @@ class LoadBalancer : public NonConstDescribable, public Destroyable { DECLARE_bool(show_lb_in_vars); DECLARE_int32(default_weight_of_wlb); +DECLARE_int64(lb_warmup_ms); + +double WarmupMultiplierImpl(int64_t join_time_us, int64_t now_us); +bool WarmupAcceptImpl(int64_t join_time_us, int64_t now_us); + +// Slow start: while -lb_warmup_ms is positive, a server newly added to a +// LoadBalancer serves a ramping fraction of its normal traffic share, from +// about 10% right after joining to 100% at the end of the window. The ramp +// restarts when a removed server is added back(naming service flap); a +// transiently disconnected server does not change LB membership and keeps +// its ramp. Servers added together(e.g. at channel init) ramp together and +// keep their relative shares. +// Returns the weight multiplier in (0, 1] for a server that joined the +// LoadBalancer at `join_time_us'(gettimeofday_us). `now_us' <= 0 makes the +// function read the clock itself. +inline double WarmupMultiplier(int64_t join_time_us, int64_t now_us) { + return FLAGS_lb_warmup_ms <= 0 ? + 1.0 : WarmupMultiplierImpl(join_time_us, now_us); +} + +// Probabilistic form of WarmupMultiplier for policies without changable +// weights: returns true with probability WarmupMultiplier(...). +inline bool WarmupAccept(int64_t join_time_us, int64_t now_us) { + return FLAGS_lb_warmup_ms <= 0 || WarmupAcceptImpl(join_time_us, now_us); +} // A intrusively shareable load balancer created from name. class SharedLoadBalancer : public SharedObject, public NonConstDescribable { diff --git a/src/brpc/policy/consistent_hashing_load_balancer.cpp b/src/brpc/policy/consistent_hashing_load_balancer.cpp index 5ff1558f8b..996df84039 100644 --- a/src/brpc/policy/consistent_hashing_load_balancer.cpp +++ b/src/brpc/policy/consistent_hashing_load_balancer.cpp @@ -22,6 +22,7 @@ #include #include "butil/containers/flat_map.h" #include "butil/errno.h" +#include "butil/time.h" #include "butil/strings/string_number_conversions.h" #include "brpc/socket.h" #include "brpc/policy/consistent_hashing_load_balancer.h" @@ -71,6 +72,7 @@ bool DefaultReplicaPolicy::Build(ServerId server, return false; } replicas->clear(); + const int64_t join_time_us = butil::gettimeofday_us(); for (size_t i = 0; i < num_replicas; ++i) { char host[256]; int len = 0; @@ -85,6 +87,7 @@ bool DefaultReplicaPolicy::Build(ServerId server, node.hash = _hash_func(host, len); node.server_sock = server; node.server_addr = ptr->remote_side(); + node.join_time_us = join_time_us; replicas->push_back(node); } return true; @@ -107,6 +110,7 @@ bool KetamaReplicaPolicy::Build(ServerId server, return false; } replicas->clear(); + const int64_t join_time_us = butil::gettimeofday_us(); const size_t points_per_hash = 4; CHECK(num_replicas % points_per_hash == 0) << "Ketam hash replicas number(" << num_replicas << ") should be n*4"; @@ -126,6 +130,7 @@ bool KetamaReplicaPolicy::Build(ServerId server, ConsistentHashingLoadBalancer::Node node; node.server_sock = server; node.server_addr = ptr->remote_side(); + node.join_time_us = join_time_us; node.hash = ((uint32_t) (digest[3 + j * 4] & 0xFF) << 24) | ((uint32_t) (digest[2 + j * 4] & 0xFF) << 16) | ((uint32_t) (digest[1 + j * 4] & 0xFF) << 8) @@ -321,7 +326,8 @@ int ConsistentHashingLoadBalancer::SelectServer( } for (size_t i = 0; i < s->size(); ++i) { if (((i + 1) == s->size() // always take last chance - || !ExcludedServers::IsExcluded(in.excluded, choice->server_sock.id)) + || (!ExcludedServers::IsExcluded(in.excluded, choice->server_sock.id) + && WarmupAccept(choice->join_time_us, in.begin_time_us))) && IsServerAvailable(choice->server_sock.id, out->ptr)) { return 0; } else { diff --git a/src/brpc/policy/consistent_hashing_load_balancer.h b/src/brpc/policy/consistent_hashing_load_balancer.h index a4808c1a70..4a883bc7ff 100644 --- a/src/brpc/policy/consistent_hashing_load_balancer.h +++ b/src/brpc/policy/consistent_hashing_load_balancer.h @@ -47,6 +47,10 @@ class ConsistentHashingLoadBalancer : public LoadBalancer { uint32_t hash; ServerId server_sock; butil::EndPoint server_addr; // To make sorting stable among all clients + // Time when the server was added, for the warm-up ramp. Not part + // of ordering/equality so that re-adding an existing server keeps + // its original stamp. + int64_t join_time_us; bool operator<(const Node &rhs) const { if (hash < rhs.hash) { return true; } if (hash > rhs.hash) { return false; } diff --git a/src/brpc/policy/locality_aware_load_balancer.cpp b/src/brpc/policy/locality_aware_load_balancer.cpp index 81729d781e..da99f79702 100644 --- a/src/brpc/policy/locality_aware_load_balancer.cpp +++ b/src/brpc/policy/locality_aware_load_balancer.cpp @@ -555,6 +555,7 @@ LocalityAwareLoadBalancer::Weight::Weight(int64_t initial_weight) , _old_index((size_t)-1L) , _old_weight(0) , _avg_latency(0) + , _join_time_us(butil::gettimeofday_us()) , _time_q(_time_q_items, sizeof(_time_q_items), butil::NOT_OWN_STORAGE) { } diff --git a/src/brpc/policy/locality_aware_load_balancer.h b/src/brpc/policy/locality_aware_load_balancer.h index 82373a36dd..958eee469b 100644 --- a/src/brpc/policy/locality_aware_load_balancer.h +++ b/src/brpc/policy/locality_aware_load_balancer.h @@ -101,6 +101,7 @@ class LocalityAwareLoadBalancer : public LoadBalancer { size_t _old_index; int64_t _old_weight; int64_t _avg_latency; + int64_t _join_time_us; butil::BoundedQueue _time_q; // content of _time_q TimeInfo _time_q_items[RECV_QUEUE_SIZE]; @@ -176,6 +177,10 @@ inline int64_t LocalityAwareLoadBalancer::Weight::ResetWeight( new_weight = new_weight * punish_latency / inflight_delay; } } + const double wm = WarmupMultiplier(_join_time_us, now_us); + if (wm < 1.0) { + new_weight = (int64_t)(new_weight * wm); + } if (new_weight < FLAGS_min_weight) { new_weight = FLAGS_min_weight; } diff --git a/src/brpc/policy/p2c_ewma_load_balancer.cpp b/src/brpc/policy/p2c_ewma_load_balancer.cpp index bd71d34fdf..6eebf3dfb0 100644 --- a/src/brpc/policy/p2c_ewma_load_balancer.cpp +++ b/src/brpc/policy/p2c_ewma_load_balancer.cpp @@ -81,6 +81,7 @@ bool P2CEwmaLoadBalancer::Add(Servers& bg, const Servers& fg, // Both buffers do not have the server. Create the stat structure // which will be shared by both buffers. info.stat = std::make_shared(); + info.stat->join_time_us = butil::gettimeofday_us(); } else { // Already added to the other buffer, share its stat. info.stat = fg.server_list[*pindex].stat; @@ -164,7 +165,10 @@ double P2CEwmaLoadBalancer::Score( } // Clamp so that a transiently negative counter can not invert routing. const int32_t load = std::max(inflight + 1, 1); - return latency_term * (double)load / (double)info.weight; + // The warm-up multiplier discounts the effective weight, composing with + // (instead of fighting) the latency score of a cold server. + return latency_term * (double)load / + ((double)info.weight * WarmupMultiplier(info.stat->join_time_us, now_us)); } int P2CEwmaLoadBalancer::SelectServer(const SelectIn& in, SelectOut* out) { diff --git a/src/brpc/policy/p2c_ewma_load_balancer.h b/src/brpc/policy/p2c_ewma_load_balancer.h index 1594f15934..28359f8b82 100644 --- a/src/brpc/policy/p2c_ewma_load_balancer.h +++ b/src/brpc/policy/p2c_ewma_load_balancer.h @@ -54,7 +54,9 @@ class P2CEwmaLoadBalancer : public LoadBalancer { // added and shared by both buffers of _db_servers, so a stable pointer // can be used from SelectServer()/Feedback() without copying. struct NodeStat { - NodeStat() : inflight(0), ewma_us(0), stamp_us(0) {} + NodeStat() : join_time_us(0), inflight(0), ewma_us(0), stamp_us(0) {} + // Time when the server was added, for the warm-up ramp. + int64_t join_time_us; butil::atomic inflight; // Peak-sensitive EWMA of latency in us. 0 means no observation yet. butil::atomic ewma_us; diff --git a/src/brpc/policy/randomized_load_balancer.cpp b/src/brpc/policy/randomized_load_balancer.cpp index a76eaa91b6..88e939fd9a 100644 --- a/src/brpc/policy/randomized_load_balancer.cpp +++ b/src/brpc/policy/randomized_load_balancer.cpp @@ -18,6 +18,7 @@ #include "butil/macros.h" #include "butil/fast_rand.h" +#include "butil/time.h" #include "bthread/prime_offset.h" #include "brpc/socket.h" #include "brpc/policy/randomized_load_balancer.h" @@ -36,6 +37,7 @@ bool RandomizedLoadBalancer::Add(Servers& bg, const ServerId& id) { } bg.server_map[id] = bg.server_list.size(); bg.server_list.push_back(id); + bg.join_times.push_back(butil::gettimeofday_us()); return true; } @@ -44,8 +46,10 @@ bool RandomizedLoadBalancer::Remove(Servers& bg, const ServerId& id) { if (it != bg.server_map.end()) { size_t index = it->second; bg.server_list[index] = bg.server_list.back(); + bg.join_times[index] = bg.join_times.back(); bg.server_map[bg.server_list[index]] = index; bg.server_list.pop_back(); + bg.join_times.pop_back(); bg.server_map.erase(it); return true; } @@ -112,7 +116,8 @@ int RandomizedLoadBalancer::SelectServer(const SelectIn& in, SelectOut* out) { for (size_t i = 0; i < n; ++i) { const SocketId id = s->server_list[offset].id; if (((i + 1) == n // always take last chance - || !ExcludedServers::IsExcluded(in.excluded, id)) + || (!ExcludedServers::IsExcluded(in.excluded, id) + && WarmupAccept(s->join_times[offset], in.begin_time_us))) && IsServerAvailable(id, out->ptr)) { // We found an available server return 0; diff --git a/src/brpc/policy/randomized_load_balancer.h b/src/brpc/policy/randomized_load_balancer.h index 3787e45a5c..0511af4dd1 100644 --- a/src/brpc/policy/randomized_load_balancer.h +++ b/src/brpc/policy/randomized_load_balancer.h @@ -45,6 +45,8 @@ class RandomizedLoadBalancer : public LoadBalancer { private: struct Servers { std::vector server_list; + // Time when server_list[i] was added, for the warm-up ramp. + std::vector join_times; std::map server_map; }; bool SetParameters(const butil::StringPiece& params); diff --git a/src/brpc/policy/round_robin_load_balancer.cpp b/src/brpc/policy/round_robin_load_balancer.cpp index c219808b6a..efd84d6f19 100644 --- a/src/brpc/policy/round_robin_load_balancer.cpp +++ b/src/brpc/policy/round_robin_load_balancer.cpp @@ -18,6 +18,7 @@ #include "butil/macros.h" #include "butil/fast_rand.h" +#include "butil/time.h" #include "bthread/prime_offset.h" #include "brpc/socket.h" #include "brpc/policy/round_robin_load_balancer.h" @@ -36,6 +37,7 @@ bool RoundRobinLoadBalancer::Add(Servers& bg, const ServerId& id) { } bg.server_map[id] = bg.server_list.size(); bg.server_list.push_back(id); + bg.join_times.push_back(butil::gettimeofday_us()); return true; } @@ -44,8 +46,10 @@ bool RoundRobinLoadBalancer::Remove(Servers& bg, const ServerId& id) { if (it != bg.server_map.end()) { const size_t index = it->second; bg.server_list[index] = bg.server_list.back(); + bg.join_times[index] = bg.join_times.back(); bg.server_map[bg.server_list[index]] = index; bg.server_list.pop_back(); + bg.join_times.pop_back(); bg.server_map.erase(it); return true; } @@ -119,7 +123,8 @@ int RoundRobinLoadBalancer::SelectServer(const SelectIn& in, SelectOut* out) { tls.offset = (tls.offset + tls.stride) % n; const SocketId id = s->server_list[tls.offset].id; if (((i + 1) == n // always take last chance - || !ExcludedServers::IsExcluded(in.excluded, id)) + || (!ExcludedServers::IsExcluded(in.excluded, id) + && WarmupAccept(s->join_times[tls.offset], in.begin_time_us))) && IsServerAvailable(id, out->ptr)) { s.tls() = tls; return 0; diff --git a/src/brpc/policy/round_robin_load_balancer.h b/src/brpc/policy/round_robin_load_balancer.h index f087dcdc42..60fbeabdce 100644 --- a/src/brpc/policy/round_robin_load_balancer.h +++ b/src/brpc/policy/round_robin_load_balancer.h @@ -44,6 +44,8 @@ class RoundRobinLoadBalancer : public LoadBalancer { private: struct Servers { std::vector server_list; + // Time when server_list[i] was added, for the warm-up ramp. + std::vector join_times; std::map server_map; }; struct TLS { diff --git a/src/brpc/policy/weighted_round_robin_load_balancer.cpp b/src/brpc/policy/weighted_round_robin_load_balancer.cpp index f52bf2990b..e28ae8d067 100644 --- a/src/brpc/policy/weighted_round_robin_load_balancer.cpp +++ b/src/brpc/policy/weighted_round_robin_load_balancer.cpp @@ -19,6 +19,7 @@ #include #include "butil/fast_rand.h" +#include "butil/time.h" #include "brpc/socket.h" #include "brpc/policy/weighted_round_robin_load_balancer.h" #include "butil/strings/string_number_conversions.h" @@ -91,7 +92,7 @@ bool WeightedRoundRobinLoadBalancer::Add(Servers& bg, const ServerId& id) { bool insert_server = bg.server_map.emplace(id.id, bg.server_list.size()).second; if (insert_server) { - bg.server_list.emplace_back(id.id, weight); + bg.server_list.emplace_back(id.id, weight, butil::gettimeofday_us()); bg.weight_sum += weight; return true; } @@ -182,8 +183,15 @@ int WeightedRoundRobinLoadBalancer::SelectServer(const SelectIn& in, SelectOut* size_t remain_servers = s->server_list.size(); while (remain_servers > 0) { SocketId server_id = GetServerInNextStride(s->server_list, filter, tls_temp); + bool warmup_pass = true; + if (remain_servers > 1 && FLAGS_lb_warmup_ms > 0) { + warmup_pass = WarmupAccept( + s->server_list[s->server_map.at(server_id)].join_time_us, + in.begin_time_us); + } if ((remain_servers == 1 // always take last chance - || !ExcludedServers::IsExcluded(in.excluded, server_id)) + || (!ExcludedServers::IsExcluded(in.excluded, server_id) + && warmup_pass)) && Socket::Address(server_id, out->ptr) == 0 && (*out->ptr)->IsAvailable()) { // update tls. diff --git a/src/brpc/policy/weighted_round_robin_load_balancer.h b/src/brpc/policy/weighted_round_robin_load_balancer.h index 828de65965..4c02ce69ea 100644 --- a/src/brpc/policy/weighted_round_robin_load_balancer.h +++ b/src/brpc/policy/weighted_round_robin_load_balancer.h @@ -43,9 +43,12 @@ class WeightedRoundRobinLoadBalancer : public LoadBalancer { private: struct Server { - Server(SocketId s_id = 0, uint32_t s_w = 0): id(s_id), weight(s_w) {} + Server(SocketId s_id = 0, uint32_t s_w = 0, int64_t s_jt = 0) + : id(s_id), weight(s_w), join_time_us(s_jt) {} SocketId id; uint32_t weight; + // Time when the server was added, for the warm-up ramp. + int64_t join_time_us; }; struct Servers { // The value is configured weight for each server. diff --git a/test/brpc_lb_warmup_unittest.cpp b/test/brpc_lb_warmup_unittest.cpp new file mode 100644 index 0000000000..8ffdfbdb25 --- /dev/null +++ b/test/brpc_lb_warmup_unittest.cpp @@ -0,0 +1,283 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include +#include +#include +#include +#include +#include +#include "butil/fast_rand.h" +#include "butil/time.h" +#include "brpc/socket.h" +#include "brpc/load_balancer.h" +#include "brpc/policy/round_robin_load_balancer.h" +#include "brpc/policy/weighted_round_robin_load_balancer.h" +#include "brpc/policy/consistent_hashing_load_balancer.h" +#include "brpc/policy/locality_aware_load_balancer.h" +#include "brpc/policy/p2c_ewma_load_balancer.h" + +namespace brpc { +DECLARE_double(lb_warmup_curve); +} + +namespace { + +class SaveRecycle : public brpc::SocketUser { + void BeforeRecycle(brpc::Socket* s) { delete this; } +}; + +brpc::ServerId CreateServer(const char* addr, const char* tag = "") { + butil::EndPoint point; + EXPECT_EQ(0, str2endpoint(addr, &point)); + brpc::ServerId id(8888); + brpc::SocketOptions options; + options.remote_side = point; + options.user = new SaveRecycle; + EXPECT_EQ(0, brpc::Socket::Create(options, &id.id)); + id.tag = tag; + return id; +} + +// Select `count' times at `now_us' and return times each server was chosen. +// Feeds back immediately when the LB asks for it(la). +std::map CountShares( + brpc::LoadBalancer* lb, int count, int64_t now_us, + bool changable_weights = false, bool with_request_code = false) { + std::map shares; + for (int i = 0; i < count; ++i) { + brpc::LoadBalancer::SelectIn in = { + now_us, changable_weights, with_request_code, + with_request_code ? butil::fast_rand() % UINT_MAX : 0u, NULL }; + brpc::SocketUniquePtr ptr; + brpc::LoadBalancer::SelectOut out(&ptr); + if (lb->SelectServer(in, &out) != 0) { + continue; + } + ++shares[ptr->id()]; + if (out.need_feedback) { + brpc::LoadBalancer::CallInfo info; + info.begin_time_us = now_us; + info.server_id = ptr->id(); + info.error_code = 0; + info.controller = NULL; + lb->Feedback(info); + } + } + return shares; +} + +class LbWarmupTest : public ::testing::Test { +protected: + void SetUp() override { + _saved_warmup_ms = brpc::FLAGS_lb_warmup_ms; + _saved_curve = brpc::FLAGS_lb_warmup_curve; + } + void TearDown() override { + brpc::FLAGS_lb_warmup_ms = _saved_warmup_ms; + brpc::FLAGS_lb_warmup_curve = _saved_curve; + } + + int64_t _saved_warmup_ms; + double _saved_curve; +}; + +TEST_F(LbWarmupTest, disabled_by_default) { + ASSERT_EQ(0, brpc::FLAGS_lb_warmup_ms); + // Any stamp maps to full weight when disabled. + ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(butil::gettimeofday_us(), 0)); + + // A just-added server gets its full share right away. + brpc::policy::RoundRobinLoadBalancer lb; + const brpc::ServerId a = CreateServer("127.0.0.1:8101"); + const brpc::ServerId b = CreateServer("127.0.0.1:8102"); + ASSERT_TRUE(lb.AddServer(a)); + ASSERT_TRUE(lb.AddServer(b)); + std::map shares = + CountShares(&lb, 2000, butil::gettimeofday_us()); + ASSERT_GT(shares[a.id], 600); + ASSERT_GT(shares[b.id], 600); +} + +TEST_F(LbWarmupTest, multiplier_math) { + brpc::FLAGS_lb_warmup_ms = 10000; + const int64_t join_us = 1000000; + + // Unstamped server is never ramped. + ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(0, join_us)); + // Ramp floor right after joining and on backward clock jumps. + ASSERT_DOUBLE_EQ(0.1, brpc::WarmupMultiplier(join_us, join_us)); + ASSERT_DOUBLE_EQ(0.1, brpc::WarmupMultiplier(join_us + 5000000, join_us)); + // Linear ramp. + ASSERT_DOUBLE_EQ(0.1, brpc::WarmupMultiplier(join_us, join_us + 500000)); + ASSERT_DOUBLE_EQ(0.3, brpc::WarmupMultiplier(join_us, join_us + 3000000)); + ASSERT_DOUBLE_EQ(0.5, brpc::WarmupMultiplier(join_us, join_us + 5000000)); + ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(join_us, join_us + 10000000)); + ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(join_us, join_us + 60000000)); + + // Curve shaping: >1 is more conservative early, <1 more aggressive. + brpc::FLAGS_lb_warmup_curve = 2.0; + ASSERT_DOUBLE_EQ(0.25, brpc::WarmupMultiplier(join_us, join_us + 5000000)); + brpc::FLAGS_lb_warmup_curve = 0.5; + ASSERT_DOUBLE_EQ(0.5, brpc::WarmupMultiplier(join_us, join_us + 2500000)); + + brpc::FLAGS_lb_warmup_curve = 1.0; + brpc::FLAGS_lb_warmup_ms = 0; + ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(join_us, join_us)); +} + +TEST_F(LbWarmupTest, accept_probability_follows_multiplier) { + brpc::FLAGS_lb_warmup_ms = 10000; + const int64_t join_us = butil::gettimeofday_us(); + int accepted = 0; + const int N = 10000; + for (int i = 0; i < N; ++i) { + accepted += brpc::WarmupAccept(join_us, join_us + 5000000); + } + // ~N/2 accepts at multiplier 0.5. + ASSERT_GT(accepted, N * 4 / 10); + ASSERT_LT(accepted, N * 6 / 10); +} + +TEST_F(LbWarmupTest, rr_ramp_and_rejoin) { + brpc::FLAGS_lb_warmup_ms = 300; + brpc::policy::RoundRobinLoadBalancer lb; + const brpc::ServerId a = CreateServer("127.0.0.1:8111"); + ASSERT_TRUE(lb.AddServer(a)); + usleep(400 * 1000); + const brpc::ServerId b = CreateServer("127.0.0.1:8112"); + ASSERT_TRUE(lb.AddServer(b)); + + const int N = 4000; + // Server b is still cold, its share stays well below the even 50%. + std::map shares = + CountShares(&lb, N, butil::gettimeofday_us()); + ASSERT_LT(shares[b.id], N / 4) << shares[b.id]; + ASSERT_GT(shares[b.id], 0); + + // Past the window(simulated by a future timestamp) shares even out. + shares = CountShares(&lb, N, butil::gettimeofday_us() + 1000000); + ASSERT_GT(shares[b.id], N * 35 / 100); + ASSERT_LT(shares[b.id], N * 65 / 100); + + // Removing and re-adding restarts the ramp. + ASSERT_TRUE(lb.RemoveServer(b)); + ASSERT_TRUE(lb.AddServer(b)); + shares = CountShares(&lb, N, butil::gettimeofday_us()); + ASSERT_LT(shares[b.id], N / 4) << shares[b.id]; +} + +TEST_F(LbWarmupTest, wrr_ramp) { + brpc::FLAGS_lb_warmup_ms = 300; + brpc::policy::WeightedRoundRobinLoadBalancer lb; + const brpc::ServerId a = CreateServer("127.0.0.1:8121", "2"); + ASSERT_TRUE(lb.AddServer(a)); + usleep(400 * 1000); + const brpc::ServerId b = CreateServer("127.0.0.1:8122", "2"); + ASSERT_TRUE(lb.AddServer(b)); + + const int N = 4000; + std::map shares = + CountShares(&lb, N, butil::gettimeofday_us()); + ASSERT_LT(shares[b.id], N / 4) << shares[b.id]; + + shares = CountShares(&lb, N, butil::gettimeofday_us() + 1000000); + ASSERT_GT(shares[b.id], N * 35 / 100); + ASSERT_LT(shares[b.id], N * 65 / 100); +} + +TEST_F(LbWarmupTest, chash_ramp) { + brpc::FLAGS_lb_warmup_ms = 300; + brpc::policy::ConsistentHashingLoadBalancer lb( + brpc::policy::CONS_HASH_LB_MURMUR3); + const brpc::ServerId a = CreateServer("127.0.0.1:8131"); + ASSERT_TRUE(lb.AddServer(a)); + usleep(400 * 1000); + const brpc::ServerId b = CreateServer("127.0.0.1:8132"); + ASSERT_TRUE(lb.AddServer(b)); + + const int N = 4000; + // Requests hashed onto cold b are mostly diverted along the ring. + std::map shares = + CountShares(&lb, N, butil::gettimeofday_us(), false, true); + ASSERT_LT(shares[b.id], N * 35 / 100) << shares[b.id]; + + shares = CountShares(&lb, N, butil::gettimeofday_us() + 1000000, + false, true); + ASSERT_GT(shares[b.id], N * 25 / 100); + ASSERT_LT(shares[b.id], N * 75 / 100); +} + +TEST_F(LbWarmupTest, la_ramp) { + brpc::FLAGS_lb_warmup_ms = 300; + brpc::policy::LocalityAwareLoadBalancer lb; + const brpc::ServerId a = CreateServer("127.0.0.1:8141"); + ASSERT_TRUE(lb.AddServer(a)); + usleep(400 * 1000); + const brpc::ServerId b = CreateServer("127.0.0.1:8142"); + ASSERT_TRUE(lb.AddServer(b)); + + // A cold server gets a reduced share of the weight tree. + const int N = 4000; + std::map shares = + CountShares(&lb, N, butil::gettimeofday_us(), true); + ASSERT_LT(shares[b.id], N * 30 / 100) << shares[b.id]; + ASSERT_GT(shares[b.id], 0); + + // While warming, b's weight lags its base weight... + brpc::DescribeOptions opt; + opt.verbose = true; + std::ostringstream cold_desc; + lb.Describe(cold_desc, opt); + ASSERT_NE(std::string::npos, cold_desc.str().find("(base=")) + << cold_desc.str(); + + // ...and catches up with it once the window has passed. Remove a so + // that selections must touch b and refresh its weight; share-based + // assertions do not work here: with identical synthetic latencies + // LALB's qps/latency weights are degenerate. + usleep(400 * 1000); + ASSERT_TRUE(lb.RemoveServer(a)); + CountShares(&lb, 100, butil::gettimeofday_us(), true); + std::ostringstream warm_desc; + lb.Describe(warm_desc, opt); + ASSERT_EQ(std::string::npos, warm_desc.str().find("(base=")) + << warm_desc.str(); +} + +TEST_F(LbWarmupTest, p2c_ramp) { + brpc::FLAGS_lb_warmup_ms = 300; + brpc::policy::P2CEwmaLoadBalancer lb; + const brpc::ServerId a = CreateServer("127.0.0.1:8151"); + ASSERT_TRUE(lb.AddServer(a)); + usleep(400 * 1000); + const brpc::ServerId b = CreateServer("127.0.0.1:8152"); + ASSERT_TRUE(lb.AddServer(b)); + + const int N = 4000; + // The discounted weight lifts b's score, both sampled servers being + // otherwise equal, so b loses (nearly) every comparison while cold. + std::map shares = + CountShares(&lb, N, butil::gettimeofday_us()); + ASSERT_LT(shares[b.id], N * 5 / 100) << shares[b.id]; + + shares = CountShares(&lb, N, butil::gettimeofday_us() + 1000000); + ASSERT_GT(shares[b.id], N * 30 / 100); + ASSERT_LT(shares[b.id], N * 70 / 100); +} + +} // namespace From f66b1902bebd8cb2bddfe57308a7d6b3b095c3f8 Mon Sep 17 00:00:00 2001 From: rajvarun77 <287367605+rajvarun77@users.noreply.github.com> Date: Wed, 2 Sep 2026 20:43:40 -0400 Subject: [PATCH 2/3] Make warm-up floor configurable via -lb_warmup_min_weight 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. --- docs/cn/client.md | 4 ++-- docs/en/client.md | 4 ++-- src/brpc/load_balancer.cpp | 23 +++++++++++++++-------- test/brpc_lb_warmup_unittest.cpp | 17 +++++++++++++++-- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/docs/cn/client.md b/docs/cn/client.md index dd667568d7..1eebfed65f 100755 --- a/docs/cn/client.md +++ b/docs/cn/client.md @@ -292,9 +292,9 @@ channel.Init("http://...", "random:min_working_instances=6 hold_seconds=10", &op ### 慢启动(预热) -新加入集群或刚重启的server往往是“冷”的(缓存未命中、JIT未编译、连接池未建立),立即承担全量流量会推高其延时甚至过载。设置-lb_warmup_ms大于0(默认为0,即关闭)后,新加入负载均衡器的server先获得约10%的正常流量份额,并在该时间窗口内线性爬升到100%。该机制对rr、wrr、random、la、p2c和一致性哈希均生效:la和p2c把爬升系数乘入权重,与延时评分自然叠加而不会互相干扰;其余算法按该系数概率性地把请求转给其他server(一致性哈希转给环上的下一个节点,预热期间会有部分请求偏离原有的哈希亲和性)。 +新加入集群或刚重启的server往往是“冷”的(缓存未命中、JIT未编译、连接池未建立),立即承担全量流量会推高其延时甚至过载。设置-lb_warmup_ms大于0(默认为0,即关闭)后,新加入负载均衡器的server先获得一小部分正常流量份额(-lb_warmup_min_weight,默认0.1),并在该时间窗口内线性爬升到100%。该机制对rr、wrr、random、la、p2c和一致性哈希均生效:la和p2c把爬升系数乘入权重,与延时评分自然叠加而不会互相干扰;其余算法按该系数概率性地把请求转给其他server(一致性哈希转给环上的下一个节点,预热期间会有部分请求偏离原有的哈希亲和性)。 --lb_warmup_curve(默认1.0)控制爬升曲线:流量份额为max(0.1, progress^lb_warmup_curve),progress在窗口内从0线性升到1。大于1的值让新server冷得更久,小于1则更激进。 +-lb_warmup_curve(默认1.0)控制爬升曲线:流量份额为max(lb_warmup_min_weight, progress^lb_warmup_curve),progress在窗口内从0线性升到1。大于1的值让新server冷得更久,小于1则更激进。 说明:预热的起点是server被加入负载均衡器的时刻。server被命名服务摘除后重新加入会重新预热;而短暂断连或健康检查失败不改变负载均衡器成员,不会重新预热。Channel初始化时所有server同时加入、一起爬升,相对流量比例不变,因此首次启动无需特殊处理。 diff --git a/docs/en/client.md b/docs/en/client.md index dda70d069d..b8706e4e9f 100644 --- a/docs/en/client.md +++ b/docs/en/client.md @@ -293,9 +293,9 @@ channel.Init("http://...", "random:min_working_instances=6 hold_seconds=10", &op ### Slow start (warm-up) -A server that just joined the cluster or restarted is often "cold" (empty caches, uncompiled JIT, unestablished connection pools); sending it a full traffic share immediately raises its latency or even overloads it. When -lb_warmup_ms is positive (default 0, disabled), a server newly added to a LoadBalancer gets about 10% of its normal traffic share at first and ramps up to 100% over the window. The mechanism works across rr, wrr, random, la, p2c and consistent hashing: la and p2c multiply the ramp into the weight so it composes with their latency scoring instead of fighting it; the other policies divert requests probabilistically to other servers (consistent hashing moves to the next node on the ring, so part of the hash affinity is temporarily diverted during warm-up). +A server that just joined the cluster or restarted is often "cold" (empty caches, uncompiled JIT, unestablished connection pools); sending it a full traffic share immediately raises its latency or even overloads it. When -lb_warmup_ms is positive (default 0, disabled), a server newly added to a LoadBalancer gets a fraction of its normal traffic share at first (-lb_warmup_min_weight, default 0.1) and ramps up to 100% over the window. The mechanism works across rr, wrr, random, la, p2c and consistent hashing: la and p2c multiply the ramp into the weight so it composes with their latency scoring instead of fighting it; the other policies divert requests probabilistically to other servers (consistent hashing moves to the next node on the ring, so part of the hash affinity is temporarily diverted during warm-up). --lb_warmup_curve (default 1.0) shapes the ramp: the traffic share is max(0.1, progress^lb_warmup_curve) where progress rises linearly from 0 to 1 over the window. Values above 1 keep a new server colder for longer, values below 1 ramp more aggressively. +-lb_warmup_curve (default 1.0) shapes the ramp: the traffic share is max(lb_warmup_min_weight, progress^lb_warmup_curve) where progress rises linearly from 0 to 1 over the window. Values above 1 keep a new server colder for longer, values below 1 ramp more aggressively. Note: warm-up starts when the server is added to the LoadBalancer. A server removed by the naming service and added back restarts its ramp, while a transient disconnection or health-check failure does not change LB membership and keeps the ramp. At channel initialization all servers join and ramp together with unchanged relative shares, so initial startup needs no special casing. diff --git a/src/brpc/load_balancer.cpp b/src/brpc/load_balancer.cpp index 6a51970969..ff50381846 100644 --- a/src/brpc/load_balancer.cpp +++ b/src/brpc/load_balancer.cpp @@ -35,20 +35,26 @@ DEFINE_int32(default_weight_of_wlb, 0, "Default weight value of Weighted LoadBal "their weights. wlb policy degradation is not enabled by default."); DEFINE_int64(lb_warmup_ms, 0, "When positive, a server newly added to a LoadBalancer gets " - "about 10% of its normal traffic share at first and ramps up " - "to 100% over this period(ms). 0 disables the warm-up"); + "lb_warmup_min_weight of its normal traffic share at first and " + "ramps up to 100% over this period(ms). 0 disables the warm-up"); DEFINE_double(lb_warmup_curve, 1.0, "Shape of the warm-up ramp: the weight multiplier is " - "max(0.1, progress^lb_warmup_curve) where progress rises " + "max(lb_warmup_min_weight, progress^lb_warmup_curve) where progress rises " "linearly from 0 to 1 over lb_warmup_ms. 1 ramps linearly, " "larger values keep a new server colder for longer"); BRPC_VALIDATE_GFLAG(show_lb_in_vars, PassValidate); BRPC_VALIDATE_GFLAG(lb_warmup_ms, PassValidate); +DEFINE_double(lb_warmup_min_weight, 0.1, + "Floor of the warm-up multiplier, in (0, 1]: the share of " + "normal traffic a server gets right after joining, so that " + "it still receives a trickle and latency-based policies keep " + "observing it"); +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); -// Floor of the warm-up multiplier so that a warming server still gets a -// trickle of traffic and latency-based policies keep observing it. -static const double WARMUP_MIN_RATIO = 0.1; double WarmupMultiplierImpl(int64_t join_time_us, int64_t now_us) { const int64_t warmup_us = FLAGS_lb_warmup_ms * 1000L; @@ -62,15 +68,16 @@ double WarmupMultiplierImpl(int64_t join_time_us, int64_t now_us) { if (elapsed_us >= warmup_us) { return 1.0; } + const double min_weight = std::min(std::max(FLAGS_lb_warmup_min_weight, 1e-9), 1.0); if (elapsed_us <= 0) { // The clock went backwards, be conservative. - return WARMUP_MIN_RATIO; + return min_weight; } double progress = (double)elapsed_us / (double)warmup_us; if (FLAGS_lb_warmup_curve > 0 && FLAGS_lb_warmup_curve != 1.0) { progress = std::pow(progress, FLAGS_lb_warmup_curve); } - return std::max(progress, WARMUP_MIN_RATIO); + return std::max(progress, min_weight); } bool WarmupAcceptImpl(int64_t join_time_us, int64_t now_us) { diff --git a/test/brpc_lb_warmup_unittest.cpp b/test/brpc_lb_warmup_unittest.cpp index 8ffdfbdb25..e889293060 100644 --- a/test/brpc_lb_warmup_unittest.cpp +++ b/test/brpc_lb_warmup_unittest.cpp @@ -33,6 +33,7 @@ namespace brpc { DECLARE_double(lb_warmup_curve); +DECLARE_double(lb_warmup_min_weight); } namespace { @@ -62,7 +63,7 @@ std::map CountShares( for (int i = 0; i < count; ++i) { brpc::LoadBalancer::SelectIn in = { now_us, changable_weights, with_request_code, - with_request_code ? butil::fast_rand() % UINT_MAX : 0u, NULL }; + with_request_code ? butil::fast_rand() % UINT_MAX : 0u, nullptr }; brpc::SocketUniquePtr ptr; brpc::LoadBalancer::SelectOut out(&ptr); if (lb->SelectServer(in, &out) != 0) { @@ -74,7 +75,7 @@ std::map CountShares( info.begin_time_us = now_us; info.server_id = ptr->id(); info.error_code = 0; - info.controller = NULL; + info.controller = nullptr; lb->Feedback(info); } } @@ -86,14 +87,17 @@ class LbWarmupTest : public ::testing::Test { 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; }; TEST_F(LbWarmupTest, disabled_by_default) { @@ -134,6 +138,15 @@ TEST_F(LbWarmupTest, multiplier_math) { ASSERT_DOUBLE_EQ(0.25, brpc::WarmupMultiplier(join_us, join_us + 5000000)); brpc::FLAGS_lb_warmup_curve = 0.5; ASSERT_DOUBLE_EQ(0.5, brpc::WarmupMultiplier(join_us, join_us + 2500000)); + brpc::FLAGS_lb_warmup_curve = 1.0; + + // The floor is configurable. + brpc::FLAGS_lb_warmup_min_weight = 0.3; + ASSERT_DOUBLE_EQ(0.3, brpc::WarmupMultiplier(join_us, join_us)); + ASSERT_DOUBLE_EQ(0.3, brpc::WarmupMultiplier(join_us, join_us + 1000000)); + ASSERT_DOUBLE_EQ(0.5, brpc::WarmupMultiplier(join_us, join_us + 5000000)); + brpc::FLAGS_lb_warmup_min_weight = 1.0; + ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(join_us, join_us)); brpc::FLAGS_lb_warmup_curve = 1.0; brpc::FLAGS_lb_warmup_ms = 0; From 251b7a97ab1c4ced9ae9737a13a851f8ad1f3ef7 Mon Sep 17 00:00:00 2001 From: rajvarun77 <287367605+rajvarun77@users.noreply.github.com> Date: Sun, 6 Sep 2026 06:19:59 -0400 Subject: [PATCH 3/3] Validate lb_warmup_curve > 0 and use FlagSaver in warm-up tests 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. --- src/brpc/load_balancer.cpp | 9 ++++++--- test/brpc_lb_warmup_unittest.cpp | 28 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/brpc/load_balancer.cpp b/src/brpc/load_balancer.cpp index ff50381846..8e05f3d2ea 100644 --- a/src/brpc/load_balancer.cpp +++ b/src/brpc/load_balancer.cpp @@ -40,8 +40,8 @@ DEFINE_int64(lb_warmup_ms, 0, DEFINE_double(lb_warmup_curve, 1.0, "Shape of the warm-up ramp: the weight multiplier is " "max(lb_warmup_min_weight, progress^lb_warmup_curve) where progress rises " - "linearly from 0 to 1 over lb_warmup_ms. 1 ramps linearly, " - "larger values keep a new server colder for longer"); + "linearly from 0 to 1 over lb_warmup_ms. Must be positive: 1 ramps " + "linearly, larger values keep a new server colder for longer"); BRPC_VALIDATE_GFLAG(show_lb_in_vars, PassValidate); BRPC_VALIDATE_GFLAG(lb_warmup_ms, PassValidate); DEFINE_double(lb_warmup_min_weight, 0.1, @@ -49,10 +49,13 @@ DEFINE_double(lb_warmup_min_weight, 0.1, "normal traffic a server gets right after joining, so that " "it still receives a trickle and latency-based policies keep " "observing it"); +static bool ValidateWarmupCurve(const char*, double v) { + return v > 0.0; +} 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_curve, ValidateWarmupCurve); BRPC_VALIDATE_GFLAG(lb_warmup_min_weight, ValidateWarmupMinWeight); diff --git a/test/brpc_lb_warmup_unittest.cpp b/test/brpc_lb_warmup_unittest.cpp index e889293060..6cdba991e1 100644 --- a/test/brpc_lb_warmup_unittest.cpp +++ b/test/brpc_lb_warmup_unittest.cpp @@ -84,20 +84,8 @@ std::map CountShares( 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; + // Restores every flag the tests touch when the fixture is destroyed. + GFLAGS_NAMESPACE::FlagSaver _flag_saver; }; TEST_F(LbWarmupTest, disabled_by_default) { @@ -153,6 +141,18 @@ TEST_F(LbWarmupTest, multiplier_math) { ASSERT_DOUBLE_EQ(1.0, brpc::WarmupMultiplier(join_us, join_us)); } +TEST_F(LbWarmupTest, flag_validation) { + // Curve must be positive; the floor must be in (0, 1]. + ASSERT_FALSE(GFLAGS_NAMESPACE::SetCommandLineOption("lb_warmup_curve", "2").empty()); + ASSERT_TRUE(GFLAGS_NAMESPACE::SetCommandLineOption("lb_warmup_curve", "0").empty()); + ASSERT_TRUE(GFLAGS_NAMESPACE::SetCommandLineOption("lb_warmup_curve", "-1").empty()); + ASSERT_DOUBLE_EQ(2.0, brpc::FLAGS_lb_warmup_curve); + ASSERT_FALSE(GFLAGS_NAMESPACE::SetCommandLineOption("lb_warmup_min_weight", "0.5").empty()); + ASSERT_TRUE(GFLAGS_NAMESPACE::SetCommandLineOption("lb_warmup_min_weight", "0").empty()); + ASSERT_TRUE(GFLAGS_NAMESPACE::SetCommandLineOption("lb_warmup_min_weight", "1.5").empty()); + ASSERT_DOUBLE_EQ(0.5, brpc::FLAGS_lb_warmup_min_weight); +} + TEST_F(LbWarmupTest, accept_probability_follows_multiplier) { brpc::FLAGS_lb_warmup_ms = 10000; const int64_t join_us = butil::gettimeofday_us();