feat(evpp): implement LB_IpHash load balance strategy - #879
Merged
Merged
Conversation
Wire up the LB_IpHash strategy that was declared in load_balance_e but left unimplemented in EventLoopThreadPool::nextLoop(). - add sockaddr_ip_hash() in base (C layer), FNV-1a over the ip bytes only (port excluded) so the same client ip maps to the same worker loop - nextLoop() gains an optional hash arg; LB_IpHash selects idx = hash % n, and the previously empty else branch now falls back to RoundRobin - TcpServer::onAccept computes the peer ip hash when LB_IpHash is set - docs/cn/TcpServer.md: document the load balance options Also fix two pre-existing bugs in base/hsocket.c spotted along the way: - sockaddr_compare: IPv6 branch compared only 4 bytes (sizeof in_addr) instead of the full 16-byte in6_addr, so IPv6 addrs differing beyond the first 4 bytes were treated as equal - ResolveAddr: IPv6 literal branch was missing a return, causing a redundant getaddrinfo() call that could overwrite the parsed result Co-authored-by: TRAE CLI <traecli@bytedance.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
An automated behavioral test for LB_IpHash is still needed.
Pull request overview
Implements client-IP-based load balancing for TcpServer, with socket hashing, IPv6 fixes, and documentation updates.
Changes:
- Adds FNV-1a hashing for IPv4 and IPv6 addresses.
- Implements
LB_IpHashrouting with RoundRobin fallback. - Fixes IPv6 comparison and literal address resolution.
- Documents load-balance strategies.
File summaries
| File | Summary |
|---|---|
evpp/TcpServer.h |
Computes peer-IP hashes during accept. |
evpp/EventLoopThreadPool.h |
Adds hash-based dispatch and fallback behavior. |
docs/cn/TcpServer.md |
Documents load-balance strategies. |
base/hsocket.h |
Declares the IP hash helper. |
base/hsocket.c |
Implements hashing and socket-address fixes. |
Review details
Suppressed comments (2)
evpp/EventLoopThreadPool.h:32
- The public API reference in
docs/cn/EventLoop.md:92still declaresnextLoop(load_balance_e lb = LB_RoundRobin)and omits both the new hash parameter andLB_IpHash. A caller following that documentation cannot provide the key and will silently use the default hash 0, routing everyLB_IpHashcall to worker 0; update that reference and its strategy list with this API change.
EventLoopPtr nextLoop(load_balance_e lb = LB_RoundRobin, uint32_t hash = 0) {
evpp/EventLoopThreadPool.h:48
- The new
LB_IpHashbranch has no automated behavioral test:EventLoopThreadPool_test.cpponly prints selected loop IDs, and CI does not execute that binary. Add a deterministic test covering equal IP hashes (including different ports) and hashes that map to different workers before relying on manual verification.
idx = hash % numLoops;
- Files reviewed: 5/5 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Implements the
LB_IpHashload balance strategy, which was already declared inload_balance_e(event/hloop.h) but left as// Not ImplementedinEventLoopThreadPool::nextLoop().With
LB_IpHash, all connections from the same client IP are consistently dispatched to the same worker loop.Changes
base/hsocket.{h,c}: addsockaddr_ip_hash()(C layer, so C users can also use it). FNV-1a over the IP bytes only — port excluded — so the same client IP maps to a stable value. IPv4 hashes 4 bytes, IPv6 hashes 16 bytes.evpp/EventLoopThreadPool.h:nextLoop()gains an optionaluint32_t hash = 0arg (backward compatible).LB_IpHashselectsidx = hash % numLoops; the previously emptyelsenow falls back to RoundRobin instead of always returning loop 0.evpp/TcpServer.h:onAccept()computes the peer IP hash viahio_peeraddr()when the strategy isLB_IpHash.docs/cn/TcpServer.md: document the available load balance options.Drive-by bug fixes in
base/hsocket.cTwo pre-existing bugs spotted while working in this file:
sockaddr_compare: the IPv6 branch compared only 4 bytes (sizeof(struct in_addr)) instead of the full 16-bytein6_addr, so IPv6 addresses differing beyond the first 4 bytes were incorrectly treated as equal.ResolveAddr: the IPv6-literal branch was missing areturn 0;, causing a redundant (potentially blocking)getaddrinfo()call that then overwrote the already-parsed literal result.Notes
LB_UrlHashis still unimplemented: at accept time there is no application-layer data, so the URL is unavailable. It belongs to the HTTP layer, notTcpServer. Theelsefallback keeps behavior safe (RoundRobin) if it is ever selected.Testing
make libhv✅make evpp✅ (all C++ test binaries link)