mirror of
https://github.com/jmcorgan/fips.git
synced 2026-08-09 16:24:45 +00:00
Cache architecture: identity fix, cache merge, parent-change flush
Identity cache: remove TTL-based expiry (60s TTL broke active sessions after expiry since handle_tun_outbound checks identity_cache before session table). Replace with LRU-only eviction bounded by configurable identity_size (default 10K). Lookup now touches timestamp for LRU freshness. Cache merge: unify coord_cache and route_cache into single coordinate cache. Both stored NodeAddr→TreeCoordinate; the layer distinction was conceptual, not functional. Discovery-sourced entries now get the same TTL+refresh treatment as session-sourced entries. Simplifies find_next_hop() to single cache lookup. Parent-change flush: clear coord_cache after recompute_coords() in both parent-switch paths of handle_tree_announce(). Stale coordinates after tree reconvergence cause dead-end routing that's more expensive than re-discovery. Tested: 493 unit tests passed, clippy clean, Docker mesh 20/20, Docker chain 6/6.
This commit is contained in:
+35
-16
@@ -110,7 +110,7 @@ async fn test_response_decode_error() {
|
||||
let from = make_node_addr(0xAA);
|
||||
node.handle_lookup_response(&from, &[0x00; 10]).await;
|
||||
// No panic, no route cached
|
||||
assert!(node.route_cache.is_empty());
|
||||
assert!(node.coord_cache().is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -134,10 +134,13 @@ async fn test_response_originator_caches_route() {
|
||||
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
// Route should be cached
|
||||
assert!(node.route_cache.contains(&target));
|
||||
let cached = node.route_cache.get(&target).unwrap();
|
||||
assert_eq!(cached.coords(), &coords);
|
||||
// Route should be cached in coord_cache
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(node.coord_cache().contains(&target, now_ms));
|
||||
assert_eq!(node.coord_cache().get(&target, now_ms).unwrap(), &coords);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -169,8 +172,12 @@ async fn test_response_transit_needs_recent_request() {
|
||||
// (will fail silently since 0xDD is not an actual peer)
|
||||
node.handle_lookup_response(&from, payload).await;
|
||||
|
||||
// Should NOT cache in route_cache (we're transit, not originator)
|
||||
assert!(!node.route_cache.contains(&target));
|
||||
// Should NOT cache in coord_cache (we're transit, not originator)
|
||||
let now_ms2 = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(!node.coord_cache().contains(&target, now_ms2));
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
@@ -275,8 +282,12 @@ async fn test_request_target_found_generates_response() {
|
||||
}
|
||||
|
||||
// Node0 should have cached node1's route (it originated the request)
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(
|
||||
nodes[0].node.route_cache.contains(&node1_addr),
|
||||
nodes[0].node.coord_cache().contains(&node1_addr, now_ms),
|
||||
"Node 0 should have cached node 1's route from LookupResponse"
|
||||
);
|
||||
|
||||
@@ -317,8 +328,12 @@ async fn test_request_three_node_chain() {
|
||||
);
|
||||
|
||||
// Node0 should have cached node2's route
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
assert!(
|
||||
nodes[0].node.route_cache.contains(&node2_addr),
|
||||
nodes[0].node.coord_cache().contains(&node2_addr, now_ms),
|
||||
"Node 0 should have cached node 2's route through 3-node chain"
|
||||
);
|
||||
|
||||
@@ -436,13 +451,17 @@ async fn test_discovery_100_nodes() {
|
||||
}
|
||||
}
|
||||
|
||||
// Verify: each originator should have the target's coords in route_cache
|
||||
// Verify: each originator should have the target's coords in coord_cache
|
||||
let now_ms = std::time::SystemTime::now()
|
||||
.duration_since(std::time::UNIX_EPOCH)
|
||||
.map(|d| d.as_millis() as u64)
|
||||
.unwrap_or(0);
|
||||
let mut resolved = 0usize;
|
||||
let mut failed = 0usize;
|
||||
let mut failed_pairs: Vec<(usize, usize)> = Vec::new();
|
||||
|
||||
for &(src, dst) in &lookup_pairs {
|
||||
if nodes[src].node.route_cache.contains(&all_addrs[dst]) {
|
||||
if nodes[src].node.coord_cache().contains(&all_addrs[dst], now_ms) {
|
||||
resolved += 1;
|
||||
} else {
|
||||
failed += 1;
|
||||
@@ -463,12 +482,12 @@ async fn test_discovery_100_nodes() {
|
||||
resolved as f64 / total_lookups as f64 * 100.0
|
||||
);
|
||||
|
||||
// Report route_cache stats across all nodes
|
||||
let total_cached: usize = nodes.iter().map(|tn| tn.node.route_cache.len()).sum();
|
||||
let min_cached = nodes.iter().map(|tn| tn.node.route_cache.len()).min().unwrap();
|
||||
let max_cached = nodes.iter().map(|tn| tn.node.route_cache.len()).max().unwrap();
|
||||
// Report coord_cache stats across all nodes
|
||||
let total_cached: usize = nodes.iter().map(|tn| tn.node.coord_cache().len()).sum();
|
||||
let min_cached = nodes.iter().map(|tn| tn.node.coord_cache().len()).min().unwrap();
|
||||
let max_cached = nodes.iter().map(|tn| tn.node.coord_cache().len()).max().unwrap();
|
||||
eprintln!(
|
||||
" Route cache entries: total={} min={} max={} avg={:.1}",
|
||||
" Coord cache entries: total={} min={} max={} avg={:.1}",
|
||||
total_cached,
|
||||
min_cached,
|
||||
max_cached,
|
||||
|
||||
+10
-11
@@ -303,14 +303,13 @@ fn test_routing_bloom_hit_without_coords_returns_none() {
|
||||
assert!(node.find_next_hop(&dest).is_none());
|
||||
}
|
||||
|
||||
// === Route cache fallback ===
|
||||
// === Discovery-populated coord_cache ===
|
||||
|
||||
#[test]
|
||||
fn test_routing_route_cache_fallback() {
|
||||
// Verify that find_next_hop() falls back to route_cache when
|
||||
// coord_cache has no entry. This is the key change that enables
|
||||
// discovery-based routing: initiate_lookup() populates route_cache,
|
||||
// and find_next_hop() now consults it.
|
||||
fn test_routing_discovery_coord_cache() {
|
||||
// Verify that find_next_hop() uses coord_cache entries populated by
|
||||
// discovery. initiate_lookup() populates coord_cache, and
|
||||
// find_next_hop() consults it.
|
||||
let mut node = make_node();
|
||||
let transport_id = TransportId::new(1);
|
||||
let my_addr = *node.node_addr();
|
||||
@@ -346,15 +345,15 @@ fn test_routing_route_cache_fallback() {
|
||||
.unwrap_or(0);
|
||||
assert!(node.coord_cache().get(&dest, now_ms).is_none());
|
||||
|
||||
// Without route_cache entry, should return None (same as before)
|
||||
// Without coord_cache entry, should return None
|
||||
assert!(node.find_next_hop(&dest).is_none());
|
||||
|
||||
// Now populate route_cache (as discovery would do)
|
||||
node.route_cache_mut().insert(dest, dest_coords, now_ms);
|
||||
// Now populate coord_cache (as discovery would do)
|
||||
node.coord_cache_mut().insert(dest, dest_coords, now_ms);
|
||||
|
||||
// find_next_hop should succeed via route_cache fallback
|
||||
// find_next_hop should succeed via coord_cache
|
||||
let result = node.find_next_hop(&dest);
|
||||
assert!(result.is_some(), "Should route via route_cache fallback");
|
||||
assert!(result.is_some(), "Should route via coord_cache");
|
||||
assert_eq!(
|
||||
result.unwrap().node_addr(),
|
||||
&peer_addr,
|
||||
|
||||
+27
-26
@@ -732,12 +732,6 @@ async fn test_session_100_nodes() {
|
||||
let min_coord = *coord_cache_sizes.iter().min().unwrap();
|
||||
let max_coord = *coord_cache_sizes.iter().max().unwrap();
|
||||
|
||||
let route_cache_sizes: Vec<usize> = nodes
|
||||
.iter()
|
||||
.map(|tn| tn.node.route_cache().len())
|
||||
.collect();
|
||||
let total_route_entries: usize = route_cache_sizes.iter().sum();
|
||||
|
||||
// === Report ===
|
||||
|
||||
eprintln!("\n === Session 100-Node Test ===");
|
||||
@@ -813,7 +807,6 @@ async fn test_session_100_nodes() {
|
||||
max_coord,
|
||||
total_coord_entries as f64 / NUM_NODES as f64
|
||||
);
|
||||
eprintln!(" Route cache: total={}", total_route_entries);
|
||||
|
||||
eprintln!("\n --- Timing ---");
|
||||
eprintln!(
|
||||
@@ -1307,37 +1300,45 @@ fn test_purge_idle_sessions_disabled_when_zero() {
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Unit tests: Identity cache expiry
|
||||
// Unit tests: Identity cache
|
||||
// ============================================================================
|
||||
|
||||
#[test]
|
||||
fn test_identity_cache_expiry() {
|
||||
fn test_identity_cache_lru_eviction() {
|
||||
let mut node = make_node();
|
||||
// Use a short TTL (1s) and insert with an old timestamp
|
||||
node.config.node.cache.identity_ttl_secs = 1;
|
||||
node.config.node.cache.identity_size = 2;
|
||||
|
||||
let remote = Identity::generate();
|
||||
let remote_addr = *remote.node_addr();
|
||||
let id1 = Identity::generate();
|
||||
let id2 = Identity::generate();
|
||||
let id3 = Identity::generate();
|
||||
|
||||
let mut prefix = [0u8; 15];
|
||||
prefix.copy_from_slice(&remote_addr.as_bytes()[0..15]);
|
||||
// Insert first two with explicit timestamps to ensure deterministic ordering
|
||||
let mut prefix1 = [0u8; 15];
|
||||
prefix1.copy_from_slice(&id1.node_addr().as_bytes()[0..15]);
|
||||
node.identity_cache.insert(prefix1, (*id1.node_addr(), id1.pubkey_full(), 1000));
|
||||
|
||||
// Insert directly with a timestamp far in the past (time 0)
|
||||
node.identity_cache.insert(prefix, (remote_addr, remote.pubkey_full(), 0));
|
||||
let mut prefix2 = [0u8; 15];
|
||||
prefix2.copy_from_slice(&id2.node_addr().as_bytes()[0..15]);
|
||||
node.identity_cache.insert(prefix2, (*id2.node_addr(), id2.pubkey_full(), 2000));
|
||||
|
||||
// Lookup should find the entry expired (registered_at=0, TTL=1s, now >> 1s)
|
||||
let result = node.lookup_by_fips_prefix(&prefix);
|
||||
assert!(result.is_none(), "Expired identity should return None");
|
||||
assert_eq!(node.identity_cache_len(), 2);
|
||||
|
||||
// Entry should have been removed from cache
|
||||
assert!(!node.identity_cache.contains_key(&prefix),
|
||||
"Expired entry should be removed from cache");
|
||||
// Adding a third should evict the oldest (id1, timestamp 1000)
|
||||
node.register_identity(*id3.node_addr(), id3.pubkey_full());
|
||||
assert_eq!(node.identity_cache_len(), 2);
|
||||
|
||||
assert!(node.lookup_by_fips_prefix(&prefix1).is_none(),
|
||||
"Oldest entry should have been evicted");
|
||||
|
||||
let mut prefix3 = [0u8; 15];
|
||||
prefix3.copy_from_slice(&id3.node_addr().as_bytes()[0..15]);
|
||||
assert!(node.lookup_by_fips_prefix(&prefix3).is_some(),
|
||||
"Newest entry should be present");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_identity_cache_survives_before_ttl() {
|
||||
fn test_identity_cache_lookup() {
|
||||
let mut node = make_node();
|
||||
// Default 60s TTL — just registered, should be available
|
||||
|
||||
let remote = Identity::generate();
|
||||
let remote_addr = *remote.node_addr();
|
||||
@@ -1348,7 +1349,7 @@ fn test_identity_cache_survives_before_ttl() {
|
||||
prefix.copy_from_slice(&remote_addr.as_bytes()[0..15]);
|
||||
|
||||
let result = node.lookup_by_fips_prefix(&prefix);
|
||||
assert!(result.is_some(), "Fresh identity should be available");
|
||||
assert!(result.is_some(), "Registered identity should be available");
|
||||
|
||||
let (addr, pk) = result.unwrap();
|
||||
assert_eq!(addr, remote_addr);
|
||||
|
||||
Reference in New Issue
Block a user