[balance-overrides] replace cached with moka for caching

[ashleychandy]

Description

Replaces cached::SizedCache with moka::sync::Cache in balance-overrides.

cached::SizedCache required external synchronization via Mutex, while moka::sync::Cache is internally concurrent and allows direct cache access without locking boilerplate.

Changes

• Added a small cache.rs wrapper around moka::sync::Cache<K, V>
• Replaced Mutex<SizedCache<K, V>> with Cache<K, V>
• Removed lock().unwrap() and migrated cache_get/cache_set to cache.get()/insert()
• Removed explicit clones on cache reads
• Removed cached and cleaned up related transitive dependencies
• Simplified test cache setup with direct .insert() calls

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[gemini-code-assist]

Code Review

This pull request replaces the cached crate with moka for strategy caching within the balance-overrides crate. It introduces a thread-safe Cache wrapper around moka::sync::Cache, eliminating the need for manual Mutex management in the approval and balance detectors. No critical issues found.

[jmg-duarte]

As this is a performance improvement (which we've discussed before) what would be your testing plan for this?

Where would you put metrics before and after so that I could ship both to prod and measure between the two?

[ashleychandy]

Good point. I put together a separate branch for benchmarking/instrumentation work so I didn’t have to bloat the main PR with experimental code.

Branch:
ashley/balance-overrides-moka-benchmarking

It includes:

• cache hit/miss metrics
• cached vs uncached latency histograms
• cache entry/eviction metrics
• release labels so different deployments can be compared side-by-side in prod
• a local contention benchmark comparing moka::sync::Cache against the previous Mutex<cached::SizedCache> setup

The synthetic concurrent-read benchmark came out around:

• moka: ~21ms
• Mutex<cached::SizedCache>: ~72ms

Benchmark command:

cargo test -p balance-overrides compare_moka_vs_mutex_cache -- --ignored --nocapture

Though I understand the real validation still needs to come from production metrics rather than the local benchmark itself.

Do these sound like the right signals to monitor?

[github-actions]

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

[jmg-duarte]

I bring good news!

I tried this out on staging, performance improvement wasn't certainly amazing, however, it's much more stable

In red — cached, in blue moka

The number of traces is 100, is not amazing but its what our Tempo setup allowed me to collect

Raw Data

cached
moka

[metalurgical]

Should ensure the same admission and eviction policy is used for cache replacement (at least initially or otherwise indicated) so that they are still handling what gets cached the same way.

I think the equivalent here is:

let data = moka::sync::Cache::builder()
            .max_capacity(max_capacity)
            .eviction_policy(EvictionPolicy::lru())
            .build();

[squadgazzz]

LGTM, assuming all the pending comments are addressed.

[ashleychandy]

@jmg-duarte @squadgazzz Thanks for the review. I've addressed the pending comments and applied the suggested changes

[MartinquaXD]

Change makes sense to me. Just a minor nit. 👍

[ashleychandy]

@MartinquaXD Thanks for the suggestion. Propagated cache_size as u64 up to the config boundary