Use LRU in connection-cache #24109
Use LRU in connection-cache #24109
Conversation
…ur home-grown least-recently-used cache works properly. Now that we've switched to using the LRU package, it's better to rely on the tests in the package itself
client/src/connection_cache.rs
Outdated
| @@ -162,82 +125,3 @@ pub fn par_serialize_and_send_transaction_batch( | |||
| Connection::Quic(conn) => conn.par_serialize_and_send_transaction_batch(transactions), | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(test)] | |||
| mod tests { | |||
There was a problem hiding this comment.
I guess we still need some tests even using lru
There was a problem hiding this comment.
Agreed, though I haven't been able to think of an appropriate test here, since before the tests basically tested whether our home-grown LRU cache did what it was supposed to. Now that we're using the LRU crate, it's more appropriate to rely on the crate's tests for that.
There was a problem hiding this comment.
I would just add some simple get calls make sure we are using that correctly.
client/src/connection_cache.rs
Outdated
|
|
||
| match map.map.get(addr) { | ||
| Some(connection) => { | ||
| inc_new_counter_info!("get_connection-cache-hits", 1); |
There was a problem hiding this comment.
let's save the metrics for a separate PR
| let (_, send_socket) = solana_net_utils::bind_in_range( | ||
| IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), | ||
| VALIDATOR_PORT_RANGE, | ||
| ) | ||
| .unwrap(); | ||
| let conn = if use_quic { | ||
| let connection = if map.use_quic { |
There was a problem hiding this comment.
try to refrain from superfluous renames in logical change commits (in the future)
client/src/connection_cache.rs
Outdated
| Connection::Quic(Arc::new(QuicTpuConnection::new(send_socket, *addr))) | ||
| } else { | ||
| Connection::Udp(Arc::new(UdpTpuConnection::new(send_socket, *addr))) | ||
| }; | ||
|
|
||
| entry.insert((conn.clone(), ticks)); | ||
| (conn, ticks) | ||
| map.map.put(addr.clone(), connection.clone()); |
There was a problem hiding this comment.
| map.map.put(addr.clone(), connection.clone()); | |
| map.map.put(*addr, connection.clone()); |
SockAddr is Copy. Odd that clippy didn't complain 🤔
Codecov Report
@@ Coverage Diff @@
## master #24109 +/- ##
=========================================
- Coverage 81.8% 81.7% -0.1%
=========================================
Files 581 590 +9
Lines 158312 161002 +2690
=========================================
+ Hits 129518 131558 +2040
- Misses 28794 29444 +650 |
Switch to using LRU for connection-cache (cherry picked from commit a38bd4a) # Conflicts: # gossip/src/cluster_info_metrics.rs
Switch to using LRU for connection-cache
Switch to using LRU for connection-cache
Problem
connection-cache currently uses a home-grown least-recently-used cache, which is not preferred, since there's an LRU package.
Summary of Changes
Switches connection-cache to use LRU
Fixes #