feat(router): add router chain snapshot cache#3305
Conversation
Aetherance
force-pushed
the
feat/router/chain-cache
branch
from
beb8384 to
cba446e
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #3305 +/- ##
===========================================
+ Coverage 46.76% 52.66% +5.90%
===========================================
Files 295 494 +199
Lines 17172 38037 +20865
===========================================
+ Hits 8031 20034 +12003
- Misses 8287 16393 +8106
- Partials 854 1610 +756 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
add a benchmark test to show the enhancement |
Benchmark Results环境：Linux amd64 / Intel i7-14650HX / Go 1.25.0 N 表示 invoker 数量 Static Tag Routing
Dynamic Tag Address Routing
Cache Hit
运行 benchmark: |
AlexStocks
left a comment
AlexStocks
left a comment
There was a problem hiding this comment.
Code Review — dubbo-go #3305
核心优化思路（位图索引 + 快照缓存）方向正确，但存在严重的语义正确性问题：
P0 × 4：
- 缓存路径使用全量 invokers 而非已过滤 invokers，路由链语义错误
- TagRouter.cache 字段并发读写无同步保护
- AddrMetadata 返回值被静默丢弃
- RouterCacheDisable 在特定场景下漏设
P1 × 3：正则热路径编译、空切片 panic 风险、位图展开双重分配
P2 × 3：benchmark 缺 ReportAllocs、位图 key 无常量定义、FindAddrPool 每次复制切片
建议在合并前重点修复 P0-1（缓存路径与路由链语义不一致）和 P0-2（并发安全），并对等价性做更严格的验证。
Aetherance
force-pushed
the
feat/router/chain-cache
branch
from
6784e99 to
2e50749
Compare
|
Introduce routerCache on RouterChain, rebuilt in SetInvokers. Add FindAddrPoolWithInvokers to atomically read snapshot and pool.
… with non-indexed keys
Aetherance
force-pushed
the
feat/router/chain-cache
branch
from
d780a06 to
8cd4d71
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces an invoker-snapshot cache at the RouterChain layer to avoid re-walking the full invoker list on every RPC, enabling Poolable routers (notably TagRouter) to precompute and reuse routing indices across Route calls. It adds a generation guard so cached bitmap indices are only used when they’re aligned with the chain’s per-call snapshot.
Changes:
- Add
RouterChainsnapshot generation tracking + arouterCacherebuilt on eachSetInvokers, and publish the generation onto the invocation for fast-path safety. - Implement TagRouter as
Poolable+CacheAccessor, building roaring bitmap indices (tag/address/port) for O(1)-style lookups, with fallback to existing traversal logic. - Add unit tests, concurrency/race coverage, and micro-benchmarks; also fix a TagRouter failover filtering bug in
requestTag.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| common/constant/key.go | Adds invocation attribute keys for cache control/generation + bitmap key prefixes/constants. |
| cluster/router/router.go | Extends the router.Cache interface and adds CacheAccessor for cache injection into routers. |
| cluster/router/chain/chain.go | Introduces chain-level cache + generation publishing and cache-disable behavior during routing. |
| cluster/router/chain/cache.go | New routerCache implementation storing pools + invoker snapshot + generation under a lock. |
| cluster/router/chain/chain_test.go | Adds tests for generation publish/increment and a concurrency race test for cache generation skew. |
| cluster/router/tag/router.go | Adds cache-aware fast path in TagRouter guarded by cache generation alignment. |
| cluster/router/tag/cache.go | Implements TagRouter pooling (bitmap indices) and bitmap-based routing path with fallback. |
| cluster/router/tag/match.go | Fixes failover address filtering to operate on the already-filtered result set. |
| cluster/router/tag/router_test.go | Adds extensive tests covering bitmap path, fallbacks, and generation guard behavior. |
| cluster/router/tag/cache_benchmarks_test.go | Adds benchmarks comparing cached vs non-cached routing for representative scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if c.cache == nil { | ||
| c.cache = newRouterCache() | ||
| for _, r := range c.routers { | ||
| if accessor, ok := r.(router.CacheAccessor); ok { | ||
| accessor.SetCache(c.cache) | ||
| } | ||
| } | ||
| } | ||
| c.cache.rebuild(c.generation, invokers, c.routers) |
| if len(match) != 0 { | ||
| return nil // not bitmap-cached; fall back to requestTag | ||
| } |
There was a problem hiding this comment.
缓存 ParamMatch 会引入过于高的复杂度 已在注释和 pr 描述中明确 ParamMatch 对应字段不为空直接fallback到无缓存路径
| // FindAddrPool returns the address pool, the invoker snapshot, and the generation of that | ||
| // snapshot in a single locked read. The generation lets callers verify the pool/invokers | ||
| // belong to the same generation the chain snapshotted for the current route, so the bitmap | ||
| // indices stay aligned with the invoker slice and the route is not served from a snapshot | ||
| // produced by a concurrent SetInvokers. | ||
| FindAddrPool(Poolable) (AddrPool, []base.Invoker, uint64) |
There was a problem hiding this comment.
这确实是个 breaking change，但是 AddrPool 里的 bitmap index 依赖同一代 invoker snapshot，单独返回 AddrPool 不能表达安全的使用语义。把 pool、invokers 和 generation 放在同一个 FindAddrPool 返回值里，可以保证调用方一次读取到一致快照，也避免后续误用旧 API。
|
Description
Every RPC call executes the full router chain. TagRouter performs O(n) invoker traversal for every call, even though invoker lists and routing rules change orders of magnitude less frequently than RPC calls. This PR adds an invoker-snapshot cache on RouterChain so that Poolable routers can pre-compute address pools once per SetInvokers and reuse them across subsequent Route calls.
Fixes #3166
Approach
New
routerCacheon RouterChain, rebuilt in full on each SetInvokers. Implements the existingrouter.Cacheinterface.CacheAccessorinterface lets Poolable routers receive the cache reference from RouterChain.TagRouter implements
Poolable+CacheAccessor.Pool()builds a three-dimensional bitmap index (tag, addr/port) stored in a single AddrPool.Static tag matching, dynamic address matching, and indexed param matching all resolve via O(1) bitmap lookup instead of O(n) traversal.
When a ParamMatch references a key not indexed, the bitmap path gracefully falls back to the original filterInvokers-based logic.
Checklist
develop