[Palaiologos1453]

Which Issue(s) This PR Fixes

Fixes #10982

Brief Description

getRouteInfoByTopic currently copies route data and encodes it for every query. Add an opt-in cache of encoded responses so repeated queries of stable routes reuse the response body.

• routeInfoCacheMaxBytes defaults to 0 (disabled), is startup-only, and bounds retained encoded bodies, keys and estimated entry overhead. Oversized responses remain uncached. Cache misses populate only the requested JSON variant; both variants share the weight budget.
• Effective route mutations advance a global revision. Unchanged registrations and heartbeats preserve the cached response. Registration performs no cache traversal/rebuild. Misses detach mutable data under the existing read lock and serialize outside it. A fill racing a completed mutation cannot be reused under the newer revision.
• Read orderTopicConf from KV on each query and validate its value and the acting-master setting on hits. Zone filtering transforms a decoded copy and does not alter the shared cache body.
• FIFO capacity maintenance runs only on fills; hits do not acquire an eviction lock or enqueue access records. Entries refresh on lookup after five minutes. The bound is a cache weight budget, not an exact JVM heap cap.

This takes a deliberately conservative approach to #4246: no eager route cache rebuilding in broker registration, default off, and a fixed memory budget. The global revision can lower hit rates when unrelated routes change; an undersized cache can churn and be slower than uncached lookup. The docs describe these limits. This is a first implementation for design review, not a claim that all large-cluster cases are solved.

How Did You Test This Change?

• JDK 8: mvn -B -f namesrv/pom.xml verify -Djacoco.skip=true — 136 tests, 0 failures/errors/skips; Checkstyle and SpotBugs passed. No SpotBugs suppression was added. JaCoCo was disabled for these local runs.
• JDK 17: 27 final route/cache/request/zone tests passed. The full suite has an existing RequestProcessorTest reflective access to Field.modifiers that fails on this JDK; the full successful verification above used JDK 8.
• License check: mvn -B -f namesrv/pom.xml apache-rat:check passed (0 unapproved/unknown files).
• Tests cover repeated-response reuse, unchanged registration/heartbeat, queue/address/zone/filter changes, topic deletion/recreation, permission changes, static mappings, acting-master transitions, KV ordering changes, JSON selection, aggregate capacity/oversized responses, lazy variant merging, zone hook isolation, interrupted misses, and a fill deliberately paused across completed topic deletion. That last case also checks serialization does not hold the route read lock.

Local JMH (Windows x86_64, Ryzen 9 7945HX, OpenJDK 17.0.17, JMH 1.19, fixed 512 MiB heap, 8 brokers / 10,000 queried topics; 2 forks, 3 × 1s warmup, 3 × 1s measurement, 1 thread, -prof gc):

Method	Cache	ns/op	JMH 99.9% error	B/op
Query	disabled	1,982.5	136.8	5,816.5
Query	64 MiB	32.8	8.9	< 0.001
Registration	original upstream class	1,049,852.9	697,579.0	320,540.3
Registration	disabled	828,471.8	319,719.4	320,497.4
Registration	64 MiB	689,833.4	316,458.7	320,498.9

The registration uncertainty intervals overlap; no registration speedup is claimed. The upstream control loads RouteInfoManager compiled from base commit 57c2dc04 ahead of the benchmark classpath. Registration payload versions change while effective routes remain equivalent. A 4-reader / 1-registration group also completed; its subgroup results, raw query iteration means and reproduction instructions are in docs/en/namesrv-route-cache.md. The cache fits the working set in the reported hit-path results. These are method-level measurements, not end-to-end throughput or a 300-broker-scale validation.

[RockteMQ-AI]

Review by github-manager-bot

Summary

This PR adds an opt-in cache for serialized topic route responses in the NameServer, addressing #10982. The cache is disabled by default (routeInfoCacheMaxBytes=0), startup-only, and uses FIFO eviction with a weight budget. Effective route mutations advance a global revision for invalidation. The implementation is deliberately conservative — no eager cache rebuilding on broker registration, and the docs clearly describe limitations.

Findings

• [Info] NamesrvConfig.java:35-38 — Good documentation on the new config field. Adding routeInfoCacheMaxBytes to the config blacklist (configBlackList) is correct — prevents runtime changes to a startup-only option.

• [Info] RouteInfoManager.java — The global routeDataRevision approach is clean. Only effective mutations increment the revision; unchanged registrations and heartbeats preserve cached responses. This avoids unnecessary invalidations during normal broker heartbeat cycles.

• [Warning] TopicRouteDataCache.java — The insertionOrder LinkedHashMap is used for FIFO eviction, but I see it is accessed in put() and evict() methods. Please confirm that all accesses to insertionOrder are properly synchronized (either under the same lock or the map is otherwise thread-safe). LinkedHashMap is not thread-safe, and concurrent modifications could cause issues.

• [Info] TopicRouteDataCache.java:49-54 — The 5-minute TTL check (System.nanoTime() - entry.createdNanos >= TimeUnit.MINUTES.toNanos(5)) is a good safety mechanism. Entries that are not accessed for 5 minutes will be refreshed on next lookup, preventing stale data from persisting indefinitely.

• [Warning] The global revision means that any route change (even for an unrelated topic) invalidates the entire cache. In a large cluster with frequent route updates, this could significantly lower hit rates. The PR description acknowledges this tradeoff, but consider adding a metric (e.g., cache hit/miss ratio) so operators can tune routeInfoCacheMaxBytes based on actual workload patterns.

• [Info] RouteInfoManager.java — Releasing the read lock before serialization on cache miss is a good optimization. This prevents the cache fill from blocking other readers while JSON encoding is in progress.

• [Info] The test coverage is excellent — 27 tests covering reuse, invalidation, queue/address/zone/filter changes, topic deletion/recreation, permission changes, static mappings, acting-master transitions, KV ordering, JSON selection, capacity limits, zone hook isolation, and concurrent fill scenarios. The JMH benchmarks in the PR description show meaningful throughput improvements for repeated queries.

Suggestions

1. Thread safety of insertionOrder: If LinkedHashMap accesses are not all under a single lock, consider using ConcurrentLinkedHashMap or synchronizing the put/evict methods. Alternatively, document the synchronization contract clearly.

2. Configurable TTL: The 5-minute refresh interval is hardcoded. Consider making it configurable via NamesrvConfig (e.g., routeInfoCacheTtlMinutes) so operators can tune it based on their route change frequency.

3. Cache metrics: Add JMX metrics for cache hits, misses, and evictions. This would help operators understand whether the cache is providing value and whether routeInfoCacheMaxBytes is appropriately sized.

4. Documentation: The English docs (docs/en/namesrv-route-cache.md) are well-written. Consider adding a Chinese version (docs/cn/namesrv-route-cache.md) for consistency with other NameServer documentation.

Cross-repo Note

This PR only affects the NameServer. No changes are needed in apache/rocketmq-clients since the cache is transparent to clients.

Overall

This is a well-designed, conservative approach to route response caching. The default-off behavior and clear documentation of limitations make it safe to merge. The main concerns are thread safety of the LinkedHashMap and the potential for low hit rates in high-churn clusters. The test coverage is comprehensive and the JMH results are promising.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Adds an opt-in, startup-only cache for serialized topic route responses in NameServer. Uses a global revision counter for invalidation — effective route mutations advance the revision, while unchanged registrations and heartbeats preserve cached responses. Default disabled (0 bytes), FIFO eviction, bounded by weight budget.

Findings

• [Info] The global revision approach is clean and avoids walking/rebuilding the cache on registration. The revision check in pickupTopicRouteDataEncoded ensures a fill racing a mutation cannot be reused by later requests observing the newer revision.
• [Info] Serialization outside the route lock is correct and important — the read lock is held only to detach mutable data, then released before encoding. This prevents serialization from blocking registration.
• [Info] The locked flag in pickupTopicRouteData properly handles the case where lockInterruptibly() throws before acquiring the lock, preventing an illegal unlock. Good defensive fix.
• [Info] Thread interruption is properly propagated via Thread.currentThread().interrupt() in the catch block.
• [Info] FIFO eviction with hits not touching bookkeeping is a good trade-off — avoids contention on the hot path. The 5-minute refresh interval prevents stale entries from persisting indefinitely.
• [Info] Comprehensive test coverage including race conditions (fill paused across mutation), zone hook isolation, oversized responses, and all mutation types. JMH benchmarks with honest uncertainty analysis are appreciated.

Suggestions

• The TopicRouteDataCache.put method is synchronized, which serializes all fills. Under high fill rates (many cache misses), this could become a bottleneck. Consider using a ConcurrentLinkedDeque or similar for the insertion order, or accept the contention as acceptable given fills are rare compared to hits. The current design is safe and correct; this is a future optimization consideration.
• The documentation in docs/en/namesrv-route-cache.md is excellent — clear, honest about limitations, and includes benchmark methodology. Consider adding a Chinese version under docs/cn/ for consistency with other NameServer docs.
• The routeInfoCacheMaxBytes is added to the config blacklist, preventing runtime changes. This is correct for a startup-only option, but consider logging a warning if someone tries to change it at runtime via the admin API.

Overall this is a well-designed, conservative approach to route caching. The opt-in default, clear documentation, and comprehensive testing make this safe to merge.

---

Automated review by github-manager-bot

[Palaiologos1453]

The automated review has completed. @RongtongJin @lizhimins, could you please take a look at the design and invalidation approach when convenient? The cache is disabled by default, and the PR includes the JDK 8 NameServer test results and local query/registration benchmark details.