Repository navigation
Add bounded-load consistent hashing load balancer (c_murmurhash_bl) - #3502
Merged
Merged
Conversation
Classic consistent hashing routes a hot key to one server with no relief valve: that server saturates while its ring neighbors idle. c_murmurhash_bl implements "Consistent Hashing with Bounded Loads" (Mirrokni et al., CACM 2017) on the same murmurhash ring: each server accepts at most ceil(load_factor * average in-flight) requests, and an at-capacity server overflows clockwise to the next server with spare capacity, so spilled requests always land on the same ring successors. load_factor defaults to -chash_bounded_load_factor(1.25, validated > 1) and is overridable per channel: c_murmurhash_bl:load_factor=1.5. In-flight accounting uses relaxed per-server counters shared by both DoublyBufferedData buffers, incremented at selection and decremented in Feedback(); the total is restored even if the server was removed in between, and the counter map is resynced from the ring on membership changes. The ring code is reused by subclassing ConsistentHashingLoadBalancer via a per-key SetParameter() hook. The existing `replicas' parameter is now documented for all CH schemes, settling the old "TODO: or 160?" on the replica count. Includes unit tests (cap under hot-key load, overflow to ring successor, factor validation, feedback decrement, removal consistency, replicas parameter) and docs in cn/en client.md.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new client-side load balancing policy c_murmurhash_bl implementing “Consistent Hashing with Bounded Loads” on top of the existing murmurhash consistent-hash ring, aiming to cap hot-key overload by overflowing deterministically to ring successors. This extends brpc’s load balancer set with bounded-load CH, plus documentation and a dedicated unit test suite.
Changes:
- Introduce
ConsistentHashingBoundedLoadBalancerwith per-server in-flight accounting and overflow-to-successor selection. - Register the new policy (
c_murmurhash_bl) and add a new gflag-chash_bounded_load_factorwith validation. - Add unit tests and update client documentation (EN/CN) for new parameters (
load_factor,replicas) and behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/brpc_ch_bounded_load_balancer_unittest.cpp | New unit tests covering validation, overflow behavior, feedback accounting, and membership changes. |
| src/brpc/policy/consistent_hashing_load_balancer.h | Exposes ring members to subclasses and declares the new bounded-load CH load balancer. |
| src/brpc/policy/consistent_hashing_load_balancer.cpp | Implements bounded-load selection/feedback, parameter parsing hooks, and adds the new gflag + validator. |
| src/brpc/global.cpp | Registers c_murmurhash_bl in the global load balancer extension registry. |
| docs/en/client.md | Documents c_murmurhash_bl, load_factor, and replicas usage. |
| docs/cn/client.md | Chinese documentation updates for c_murmurhash_bl, load_factor, and replicas. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+423
to
+424
| LOG(ERROR) << "Failed to set this unknown parameters " << key << '=' << value; | ||
| return true; |
Comment on lines
+78
to
+85
| valid->Destroy(); | ||
|
|
||
| ASSERT_EQ("", GFLAGS_NAMESPACE::SetCommandLineOption( | ||
| "chash_bounded_load_factor", "0.9")); | ||
| ASSERT_EQ("", GFLAGS_NAMESPACE::SetCommandLineOption( | ||
| "chash_bounded_load_factor", "1.0")); | ||
| ASSERT_NE("", GFLAGS_NAMESPACE::SetCommandLineOption( | ||
| "chash_bounded_load_factor", "1.25")); |
|
|
||
| Do distinguish "key" and "attributes" of the request. Don't compute request_code by full content of the request just for quick. Minor change in attributes may result in totally different hash code and change destination dramatically. Another cause is padding, for example: `struct Foo { int32_t a; int64_t b; }` has a 4-byte undefined gap between `a` and `b` on 64-bit machines, result of `hash(&foo, sizeof(foo))` is undefined. Fields need to be packed or serialized before hashing. | ||
|
|
||
| Number of virtual nodes per server defaults to -chash_num_replicas(default 100) and can be overridden per channel: `c_murmurhash:replicas=300`. |
|
|
||
| ### c_murmurhash_bl | ||
|
|
||
| which is consistent hashing with bounded loads("Consistent Hashing with Bounded Loads", Mirrokni et al., CACM 2017). The hash ring is identical to `c_murmurhash`, but each server additionally has a capacity of `ceil(load_factor * average in-flight requests)`. When the hashed-to server is at capacity, the request overflows clockwise to the next server on the ring with spare capacity, so a hot key no longer saturates a single server while overflowed requests always land on the same ring successors, which keeps caches effective. The default factor comes from -chash_bounded_load_factor(default 1.25, must be > 1) and can be overridden per channel: `c_murmurhash_bl:load_factor=1.5`. The `replicas` parameter is supported as in `c_murmurhash`. |
Contributor
Author
|
@chenBright @wwbmmm gentle ping when you have a moment — CI has been fully green for a week and the Copilot pass is addressed. This is the bounded-load follow-up to the consistent-hashing family (same ring, only |
Contributor
|
LGTM |
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
Classic consistent hashing has no relief valve for hot keys: one server saturates while its ring neighbors sit idle. This PR adds
c_murmurhash_bl, an implementation of "Consistent Hashing with Bounded Loads" (Mirrokni et al., CACM 2017) on the existing murmurhash ring, so hot-key overflow spills to deterministic ring successors instead of overloading a single server.Usage
Design
ConsistentHashingLoadBalancer; onlySelectServerand load accounting differ, ring construction is shared.ceil(load_factor * (total_inflight + 1) / server_count); an at-capacity server overflows clockwise to the next server with spare capacity, with first-seen fallback if all are at capacity.Feedback()(total stays consistent even if the server was removed in between), and resynced on membership change.Tests & Docs
7 new cases in
test/brpc_ch_bounded_load_balancer_unittest.cpp(factor/replicas validation, hot-key capping, overflow to ring successor, feedback decrement, removal consistency, no accounting without changable_weights); existing consistent-hashing tests unchanged and passing. Documented indocs/cn/client.mdanddocs/en/client.md.cc @chenBright (reviewer of the sibling p2c load balancer, #3367)