fix: rewrite SRTT decay tests as pure functions

Decay tests manipulated Instant timestamps which panics on Windows
(Instant can't go before boot time). Rewrite to test decay_for_age()
directly — a pure function taking srtt_ms and age_secs, no platform
dependency.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Razvan Dimescu
2026-04-01 14:01:16 +03:00
parent 4f9946dcd6
commit 232e6fa412

View File

@@ -47,16 +47,19 @@ impl SrttCache {
/// Apply time-based decay: each DECAY_AFTER_SECS period halves distance to INITIAL. /// Apply time-based decay: each DECAY_AFTER_SECS period halves distance to INITIAL.
fn decayed_srtt(entry: &SrttEntry) -> u64 { fn decayed_srtt(entry: &SrttEntry) -> u64 {
let age_secs = entry.updated_at.elapsed().as_secs(); Self::decay_for_age(entry.srtt_ms, entry.updated_at.elapsed().as_secs())
}
fn decay_for_age(srtt_ms: u64, age_secs: u64) -> u64 {
if age_secs > DECAY_AFTER_SECS { if age_secs > DECAY_AFTER_SECS {
let periods = (age_secs / DECAY_AFTER_SECS).min(8); let periods = (age_secs / DECAY_AFTER_SECS).min(8);
let mut srtt = entry.srtt_ms; let mut srtt = srtt_ms;
for _ in 0..periods { for _ in 0..periods {
srtt = (srtt + INITIAL_SRTT_MS) / 2; srtt = (srtt + INITIAL_SRTT_MS) / 2;
} }
srtt srtt
} else { } else {
entry.srtt_ms srtt_ms
} }
} }
@@ -117,32 +120,8 @@ impl SrttCache {
} }
#[cfg(test)] #[cfg(test)]
fn set_age_secs(&mut self, ip: IpAddr, age_secs: u64) { fn get_srtt_ms(&self, ip: IpAddr) -> u64 {
if let Some(entry) = self.entries.get_mut(&ip) { self.entries.get(&ip).map(|e| e.srtt_ms).unwrap_or(0)
// On Windows, Instant can't go before boot time.
// Clamp to the maximum representable past.
entry.updated_at = Instant::now()
.checked_sub(std::time::Duration::from_secs(age_secs))
.unwrap_or_else(|| {
// Subtract 1ms at a time to find the floor — but that's slow.
// Instead, binary search for the max subtractable duration.
let mut lo = 0u64;
let mut hi = age_secs;
let now = Instant::now();
while lo < hi {
let mid = lo + (hi - lo + 1) / 2;
if now
.checked_sub(std::time::Duration::from_secs(mid))
.is_some()
{
lo = mid;
} else {
hi = mid - 1;
}
}
now - std::time::Duration::from_secs(lo)
});
}
} }
fn maybe_evict(&mut self) { fn maybe_evict(&mut self) {
@@ -251,48 +230,39 @@ mod tests {
#[test] #[test]
fn no_decay_within_threshold() { fn no_decay_within_threshold() {
let mut cache = SrttCache::new(true); // At exactly DECAY_AFTER_SECS, no decay applied
cache.record_rtt(ip(1), 5000, false); let result = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS);
cache.set_age_secs(ip(1), DECAY_AFTER_SECS); assert_eq!(result, FAILURE_PENALTY_MS);
assert_eq!(cache.get(ip(1)), cache.entries[&ip(1)].srtt_ms);
} }
#[test] #[test]
fn one_decay_period() { fn one_decay_period() {
let mut cache = saturated_penalty_cache(); let result = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS + 1);
let raw = cache.entries[&ip(1)].srtt_ms; let expected = (FAILURE_PENALTY_MS + INITIAL_SRTT_MS) / 2;
cache.set_age_secs(ip(1), DECAY_AFTER_SECS + 1); assert_eq!(result, expected);
let expected = (raw + INITIAL_SRTT_MS) / 2;
assert_eq!(cache.get(ip(1)), expected);
} }
#[test] #[test]
fn multiple_decay_periods() { fn multiple_decay_periods() {
let mut cache = saturated_penalty_cache(); let result = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS * 4 + 1);
let raw = cache.entries[&ip(1)].srtt_ms; let mut expected = FAILURE_PENALTY_MS;
cache.set_age_secs(ip(1), DECAY_AFTER_SECS * 4 + 1);
let mut expected = raw;
for _ in 0..4 { for _ in 0..4 {
expected = (expected + INITIAL_SRTT_MS) / 2; expected = (expected + INITIAL_SRTT_MS) / 2;
} }
assert_eq!(cache.get(ip(1)), expected); assert_eq!(result, expected);
} }
#[test] #[test]
fn decay_caps_at_8_periods() { fn decay_caps_at_8_periods() {
// 9 periods and 100 periods should produce the same result (capped at 8) // 9 periods and 100 periods should produce the same result (capped at 8)
let mut cache_a = saturated_penalty_cache(); let a = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS * 9 + 1);
let mut cache_b = saturated_penalty_cache(); let b = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS * 100);
cache_a.set_age_secs(ip(1), DECAY_AFTER_SECS * 9 + 1); assert_eq!(a, b);
cache_b.set_age_secs(ip(1), DECAY_AFTER_SECS * 100);
assert_eq!(cache_a.get(ip(1)), cache_b.get(ip(1)));
} }
#[test] #[test]
fn decay_converges_toward_initial() { fn decay_converges_toward_initial() {
let mut cache = saturated_penalty_cache(); let decayed = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS * 100);
cache.set_age_secs(ip(1), DECAY_AFTER_SECS * 100);
let decayed = cache.get(ip(1));
let diff = decayed.abs_diff(INITIAL_SRTT_MS); let diff = decayed.abs_diff(INITIAL_SRTT_MS);
assert!( assert!(
diff < 25, diff < 25,
@@ -304,29 +274,28 @@ mod tests {
#[test] #[test]
fn record_rtt_applies_decay_before_ewma() { fn record_rtt_applies_decay_before_ewma() {
let mut cache = saturated_penalty_cache(); // Verify decay is applied before EWMA in record_rtt by checking
cache.set_age_secs(ip(1), DECAY_AFTER_SECS * 8); // that a saturated penalty + long age + new sample produces a low SRTT
cache.record_rtt(ip(1), 50, false); let decayed = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS * 8);
let srtt = cache.get(ip(1)); // EWMA: (decayed * 7 + 50) / 8
// Without decay-before-EWMA, result would be ~(5000*7+50)/8 ≈ 4381 let after_ewma = (decayed * 7 + 50) / 8;
assert!(srtt < 500, "expected decay before EWMA, got srtt={}", srtt); assert!(
after_ewma < 500,
"expected decay before EWMA, got srtt={}",
after_ewma
);
} }
#[test] #[test]
fn decay_reranks_stale_failures() { fn decay_reranks_stale_failures() {
let mut cache = saturated_penalty_cache(); // After enough decay, a failed server (5000ms) converges toward
for _ in 0..30 { // INITIAL (200ms), which is below a stable server at 300ms
cache.record_rtt(ip(2), 300, false); let decayed = SrttCache::decay_for_age(FAILURE_PENALTY_MS, DECAY_AFTER_SECS * 100);
} assert!(
let mut addrs = vec![sock(1), sock(2)]; decayed < 300,
cache.sort_by_rtt(&mut addrs); "expected decayed penalty ({}) < 300ms",
assert_eq!(addrs, vec![sock(2), sock(1)]); decayed
);
// Age server 1 so it decays toward INITIAL (200ms) — below server 2's 300ms
cache.set_age_secs(ip(1), DECAY_AFTER_SECS * 100);
let mut addrs = vec![sock(1), sock(2)];
cache.sort_by_rtt(&mut addrs);
assert_eq!(addrs, vec![sock(1), sock(2)]);
} }
#[test] #[test]