From e59e25e1a1d7c53a2a868308e68413719eb8a8fe Mon Sep 17 00:00:00 2001 From: Razvan Dimescu Date: Wed, 1 Apr 2026 12:12:57 +0300 Subject: [PATCH] fix: SRTT decay tests use binary search for max Instant age Replace age() helper with set_age_secs() on SrttCache that binary-searches for the maximum subtractable duration. Prevents panic on Windows (Instant starts at boot) while still producing the oldest representable instant for correct decay calculations. Also removes ephemeral test-ubuntu.sh from git. Co-Authored-By: Claude Opus 4.6 (1M context) --- scripts/test-ubuntu.sh | 132 ----------------------------------------- src/srtt.rs | 52 +++++++++------- 2 files changed, 32 insertions(+), 152 deletions(-) delete mode 100644 scripts/test-ubuntu.sh diff --git a/scripts/test-ubuntu.sh b/scripts/test-ubuntu.sh deleted file mode 100644 index 1e6df73..0000000 --- a/scripts/test-ubuntu.sh +++ /dev/null @@ -1,132 +0,0 @@ -#!/bin/bash -# Ubuntu integration test for Numa PR #27 -# Usage: scp target/release/numa scripts/test-ubuntu.sh EC2:~ && ssh EC2 'sudo bash test-ubuntu.sh' -set -euo pipefail - -BIN="./numa" -PASS=0 -FAIL=0 - -check() { - local desc="$1"; shift - if "$@" > /dev/null 2>&1; then - echo " ✓ $desc" - PASS=$((PASS + 1)) - else - echo " ✗ $desc" - FAIL=$((FAIL + 1)) - fi -} - -cleanup() { - $BIN uninstall 2>/dev/null || true - killall numa 2>/dev/null || true - sleep 1 -} - -echo "=== Numa Ubuntu Integration Tests ===" -echo "" -chmod +x "$BIN" - -# --- Test 1: Forward mode (default, no config) --- -echo "--- Test 1: Forward mode (default) ---" -cleanup -$BIN 2>&1 & -NUMA_PID=$! -sleep 3 - -check "API responds" curl -sf http://127.0.0.1:5380/health -check "mode is forward" bash -c 'curl -sf http://127.0.0.1:5380/stats | grep -q "\"mode\":\"forward\""' -check "DNS resolves" bash -c 'dig @127.0.0.1 example.com A +short +time=5 | grep -q "[0-9]"' -check "dashboard returns 200" bash -c 'curl -sf -o /dev/null -w "%{http_code}" http://127.0.0.1:5380/ | grep -q 200' -kill $NUMA_PID 2>/dev/null; sleep 1 -echo "" - -# --- Test 2: Recursive mode (explicit opt-in) --- -echo "--- Test 2: Recursive mode ---" -cleanup -mkdir -p /tmp/numa-test -cat > /tmp/numa-test/numa.toml << 'TOML' -[upstream] -mode = "recursive" -[dnssec] -enabled = true -TOML -$BIN /tmp/numa-test/numa.toml 2>&1 & -NUMA_PID=$! -sleep 5 - -check "API responds" curl -sf http://127.0.0.1:5380/health -check "mode is recursive" bash -c 'curl -sf http://127.0.0.1:5380/stats | grep -q "\"mode\":\"recursive\""' -check "dnssec enabled" bash -c 'curl -sf http://127.0.0.1:5380/stats | grep -q "\"dnssec\":true"' -check "DNS resolves recursively" bash -c 'dig @127.0.0.1 example.com A +short +time=10 | grep -q "[0-9]"' -check "AD flag set (DNSSEC)" bash -c 'dig @127.0.0.1 example.com A +dnssec +time=10 | grep "flags:" | grep -q "ad"' -kill $NUMA_PID 2>/dev/null; sleep 1 -echo "" - -# --- Test 3: Auto mode --- -echo "--- Test 3: Auto mode ---" -cleanup -cat > /tmp/numa-test/numa.toml << 'TOML' -[upstream] -mode = "auto" -TOML -$BIN /tmp/numa-test/numa.toml 2>&1 & -NUMA_PID=$! -sleep 10 - -check "API responds" curl -sf http://127.0.0.1:5380/health -MODE=$(curl -sf http://127.0.0.1:5380/stats | python3 -c "import sys,json; print(json.load(sys.stdin)['mode'])" 2>/dev/null || echo "unknown") -echo " → auto resolved to: $MODE" -check "mode is recursive or forward" bash -c "echo '$MODE' | grep -qE '^(recursive|forward)$'" -check "DNS resolves" bash -c 'dig @127.0.0.1 example.com A +short +time=10 | grep -q "[0-9]"' -kill $NUMA_PID 2>/dev/null; sleep 1 -echo "" - -# --- Test 4: Install / Uninstall --- -echo "--- Test 4: Install / Uninstall ---" -cleanup -cp "$BIN" /usr/local/bin/numa - -echo " Installing..." -INSTALL_OUTPUT=$($BIN install 2>&1) || true -echo "$INSTALL_OUTPUT" -check "post-install mentions recursive" bash -c "echo '$INSTALL_OUTPUT' | grep -q 'recursive'" -sleep 3 - -check "service is running" systemctl is-active numa -check "API responds after install" curl -sf http://127.0.0.1:5380/health -check "DNS resolves after install" bash -c 'dig @127.0.0.1 example.com A +short +time=5 | grep -q "[0-9]"' - -echo "" -echo " Uninstalling..." -$BIN uninstall 2>&1 || true -sleep 2 - -check "service stopped" bash -c '! systemctl is-active numa' -echo "" - -# --- Test 5: Port 53 conflict --- -echo "--- Test 5: Port 53 conflict ---" -cleanup -# Start a dummy listener on port 53 -python3 -c "import socket; s=socket.socket(socket.AF_INET,socket.SOCK_DGRAM); s.bind(('0.0.0.0',53)); input()" & -BLOCKER_PID=$! -sleep 1 - -$BIN 2>&1 & -NUMA_PID=$! -sleep 3 -# numa should fail to bind -check "numa fails when port 53 taken" bash -c '! kill -0 $NUMA_PID 2>/dev/null' -kill $BLOCKER_PID 2>/dev/null -kill $NUMA_PID 2>/dev/null -echo "" - -# --- Cleanup --- -cleanup -rm -rf /tmp/numa-test - -echo "=== Results: $PASS passed, $FAIL failed ===" -[ $FAIL -eq 0 ] && echo "All tests passed!" || echo "Some tests failed." -exit $FAIL diff --git a/src/srtt.rs b/src/srtt.rs index a832f99..710efc0 100644 --- a/src/srtt.rs +++ b/src/srtt.rs @@ -117,9 +117,31 @@ impl SrttCache { } #[cfg(test)] - fn set_updated_at(&mut self, ip: IpAddr, at: Instant) { + fn set_age_secs(&mut self, ip: IpAddr, age_secs: u64) { if let Some(entry) = self.entries.get_mut(&ip) { - entry.updated_at = at; + // 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) + }); } } @@ -218,16 +240,6 @@ mod tests { assert_eq!(addrs, original); } - // On Windows, Instant starts near boot time — large subtractions overflow. - // Fall back to a fixed reference point created at process start. - static EPOCH: std::sync::OnceLock = std::sync::OnceLock::new(); - - fn age(secs: u64) -> Instant { - Instant::now() - .checked_sub(std::time::Duration::from_secs(secs)) - .unwrap_or(*EPOCH.get_or_init(Instant::now)) - } - /// Cache with ip(1) saturated at FAILURE_PENALTY_MS fn saturated_penalty_cache() -> SrttCache { let mut cache = SrttCache::new(true); @@ -241,7 +253,7 @@ mod tests { fn no_decay_within_threshold() { let mut cache = SrttCache::new(true); cache.record_rtt(ip(1), 5000, false); - cache.set_updated_at(ip(1), age(DECAY_AFTER_SECS)); + cache.set_age_secs(ip(1), DECAY_AFTER_SECS); assert_eq!(cache.get(ip(1)), cache.entries[&ip(1)].srtt_ms); } @@ -249,7 +261,7 @@ mod tests { fn one_decay_period() { let mut cache = saturated_penalty_cache(); let raw = cache.entries[&ip(1)].srtt_ms; - cache.set_updated_at(ip(1), age(DECAY_AFTER_SECS + 1)); + cache.set_age_secs(ip(1), DECAY_AFTER_SECS + 1); let expected = (raw + INITIAL_SRTT_MS) / 2; assert_eq!(cache.get(ip(1)), expected); } @@ -258,7 +270,7 @@ mod tests { fn multiple_decay_periods() { let mut cache = saturated_penalty_cache(); let raw = cache.entries[&ip(1)].srtt_ms; - cache.set_updated_at(ip(1), age(DECAY_AFTER_SECS * 4 + 1)); + cache.set_age_secs(ip(1), DECAY_AFTER_SECS * 4 + 1); let mut expected = raw; for _ in 0..4 { expected = (expected + INITIAL_SRTT_MS) / 2; @@ -271,15 +283,15 @@ mod tests { // 9 periods and 100 periods should produce the same result (capped at 8) let mut cache_a = saturated_penalty_cache(); let mut cache_b = saturated_penalty_cache(); - cache_a.set_updated_at(ip(1), age(DECAY_AFTER_SECS * 9 + 1)); - cache_b.set_updated_at(ip(1), age(DECAY_AFTER_SECS * 100)); + cache_a.set_age_secs(ip(1), DECAY_AFTER_SECS * 9 + 1); + cache_b.set_age_secs(ip(1), DECAY_AFTER_SECS * 100); assert_eq!(cache_a.get(ip(1)), cache_b.get(ip(1))); } #[test] fn decay_converges_toward_initial() { let mut cache = saturated_penalty_cache(); - cache.set_updated_at(ip(1), age(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); assert!( @@ -293,7 +305,7 @@ mod tests { #[test] fn record_rtt_applies_decay_before_ewma() { let mut cache = saturated_penalty_cache(); - cache.set_updated_at(ip(1), age(DECAY_AFTER_SECS * 8)); + cache.set_age_secs(ip(1), DECAY_AFTER_SECS * 8); cache.record_rtt(ip(1), 50, false); let srtt = cache.get(ip(1)); // Without decay-before-EWMA, result would be ~(5000*7+50)/8 ≈ 4381 @@ -311,7 +323,7 @@ mod tests { assert_eq!(addrs, vec![sock(2), sock(1)]); // Age server 1 so it decays toward INITIAL (200ms) — below server 2's 300ms - cache.set_updated_at(ip(1), age(DECAY_AFTER_SECS * 100)); + 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)]);