chore(spanner): add bounded EWMA latency registry for location-aware routing - #6649
chore(spanner): add bounded EWMA latency registry for location-aware routing#6649olavloite wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces bounded memory capacity, time-to-idle expiration, and periodic cleanup sweeps to the Spanner LatencyRegistry to prevent unbounded memory growth, alongside lock-free score reads in EwmaLatencyTracker. The review feedback correctly identifies a correctness bug in the EWMA decay calculation where concurrent samples could wipe out historical averages, and a performance bottleneck in the eviction logic where using .skip() on a HashMap iterator introduces O(N) overhead under an exclusive write lock. Refactoring the eviction sampling to run in O(1) time allows the removal of the eviction_cursor field and its associated logic.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6649 +/- ##
==========================================
+ Coverage 96.66% 96.69% +0.02%
==========================================
Files 308 308
Lines 97778 98516 +738
==========================================
+ Hits 94519 95261 +742
+ Misses 3259 3255 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
4da93b5 to
428a227
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces memory bounding, time-to-idle expiration, and periodic cleanup sweeps to the LatencyRegistry to prevent unbounded memory growth. It also optimizes EwmaLatencyTracker to use atomic float bits for lock-free score reads, reducing lock contention on the hot path. The review feedback correctly identifies a redundant clamping of initial_capacity in with_initial_capacity that is already performed in with_all_options.
…routing Add an Exponentially Weighted Moving Average (EWMA) latency registry to track replica responsiveness and support latency-aware routing in Cloud Spanner. Highlights: - Bounded memory: Prevents unbounded memory growth by capping trackers at 100,000 entries, evicting the oldest entries when full, and pruning idle endpoints after 10 minutes. - Fast, zero-allocation routing: Candidate selection runs without heap allocations or write locks, loading cached scores atomically so request routing stays fast. - Lower lock contention: Recording latency updates uses fine-grained per-tracker locks instead of locking the entire registry, allowing concurrent updates to different endpoints. - Thundering herd protection: Directs initial traffic to idle endpoints to probe their latency, while penalizing busy unmeasured endpoints to prevent traffic stampedes. - Numerical accuracy: Uses exp_m1 for smooth EWMA decay even with rapid back-to-back samples, avoiding floating-point precision loss.
428a227 to
2ea4aae
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces memory bounding, idle expiration, and lock-free read optimizations to the LatencyRegistry and EwmaLatencyTracker in the Spanner router. Specifically, it limits the registry capacity (defaulting to 100,000 trackers) and implements a sample-based eviction policy alongside periodic background cleanup for idle entries (defaulting to a 10-minute expiration). It also optimizes EwmaLatencyTracker by storing the EWMA score in an AtomicU64 to allow lock-free reads for score queries, and improves floating-point precision for small time deltas using exp_m1. A comprehensive suite of unit tests has been added to verify these behaviors. I have no additional feedback to provide as the implementation is robust and conforms to the repository's style guidelines.
Add an Exponentially Weighted Moving Average (EWMA) latency registry to track replica responsiveness and support latency-aware routing in Cloud Spanner.
Highlights: