From cdb21839894ab6bd3e73d737c496b0dcae9be850 Mon Sep 17 00:00:00 2001 From: Andreas Wrede Date: Mon, 17 Aug 2026 08:07:19 -0400 Subject: [PATCH] fix: stop synthetic rtt_* keys from masking real plugin-data checks host.plugin_data now always holds rtt_ipv4/rtt_ipv6 history after the first heartbeat, which broke two pre-existing behaviors that assumed plugin_data reflects only real client-collected data: - The request_update ACK gate in handle_datagram() never fired again after a host's first heartbeat, so stale OS/agent-version info was never refreshed after a reconnect (only a client restart fixed it). - plugin_data.clear() on every UP transition wiped rtt_* history too, so a flaky host could never accumulate a useful RTT graph. Add _is_rtt_key()/_has_real_plugin_data() helpers so both spots treat rtt_* keys as synthetic: the gate now looks only at real plugin data, and the recovery clear only drops real plugin keys, preserving RTT history across reconnects. Also fixes two pre-existing E127 continuation-indent flake8 issues in tests/test_udp_rtt_history.py. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01DEimzMv4Q5EjFg3hoiZ69T --- hbd/server/udp.py | 15 +++++++-- tests/test_udp_rtt_history.py | 63 +++++++++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/hbd/server/udp.py b/hbd/server/udp.py index f02e71f..b901a9e 100644 --- a/hbd/server/udp.py +++ b/hbd/server/udp.py @@ -319,6 +319,16 @@ def restore_connection_timers(hbdclass, ctx): logger.info("Restored timers for %d connection(s)", restored) +def _is_rtt_key(plugin_name: str) -> bool: + """True for synthetic RTT-history keys (rtt_ipv4/rtt_ipv6), not real plugin data.""" + return plugin_name.startswith("rtt_") + + +def _has_real_plugin_data(plugin_data: dict) -> bool: + """True if plugin_data holds any real (non-RTT) client-collected plugin data.""" + return any(not _is_rtt_key(k) for k in plugin_data) + + def handle_datagram(msg: dict, addr, transport, ctx: dict): """Handle a parsed datagram message. @@ -385,7 +395,7 @@ def handle_datagram(msg: dict, addr, transport, ctx: dict): host.doesack = msg.get("acks", -1) # send ACK back; ask client to resend plugin info when we have none yet rmsg = {"time": time.time()} - if not host.plugin_data: + if not _has_real_plugin_data(host.plugin_data): rmsg["request_update"] = 1 opkt = dicttos("ACK", rmsg) try: @@ -516,7 +526,8 @@ def handle_datagram(msg: dict, addr, transport, ctx: dict): # cleanly from the first two post-boot samples. for pname in list(host.plugin_timers): host.cancel_plugin_timer(pname) - host.plugin_data.clear() + for pname in [k for k in host.plugin_data if not _is_rtt_key(k)]: + del host.plugin_data[pname] stale_plugin_keys = [ k for k in host.alert_states if k not in ("rtt",) and not k.startswith("connectivity.") diff --git a/tests/test_udp_rtt_history.py b/tests/test_udp_rtt_history.py index a61d02a..7f2de63 100644 --- a/tests/test_udp_rtt_history.py +++ b/tests/test_udp_rtt_history.py @@ -1,4 +1,6 @@ """Tests for RTT history capture in udp.py's handle_datagram.""" +import time + from hbd.common.proto import dicttos from hbd.server import hbdclass from hbd.server.udp import handle_datagram, parse_message @@ -32,7 +34,7 @@ def _base_ctx(): def test_handle_datagram_records_rtt_history_for_new_connection(): hbdclass.Host.hosts.pop("rtt-hist-host", None) handle_datagram(_htb("rtt-hist-host", rtt=42.5), ("127.0.0.1", 50000), - _FakeTransport(), _base_ctx()) + _FakeTransport(), _base_ctx()) host = hbdclass.Host.hosts["rtt-hist-host"] samples = host.plugin_data.get("rtt_ipv4") @@ -58,7 +60,64 @@ def test_handle_datagram_appends_rtt_history_across_heartbeats(): def test_handle_datagram_skips_rtt_history_when_rtt_missing(): hbdclass.Host.hosts.pop("rtt-hist-host3", None) handle_datagram(_htb("rtt-hist-host3", rtt=None), ("127.0.0.1", 50000), - _FakeTransport(), _base_ctx()) + _FakeTransport(), _base_ctx()) host = hbdclass.Host.hosts["rtt-hist-host3"] assert "rtt_ipv4" not in host.plugin_data + + +def test_request_update_fires_on_recovery_even_with_rtt_history(): + """Regression for Finding 1: rtt_* keys must not permanently disable the + request_update gate. A connection recovering from a non-UP state must + still be asked to resend real plugin data, even though rtt_ipv4 already + holds samples from before the drop. + """ + hbdclass.Host.hosts.pop("rtt-hist-host4", None) + transport = _FakeTransport() + ctx = _base_ctx() + + # First heartbeat: brand-new host, no plugin data at all yet. + handle_datagram(_htb("rtt-hist-host4", rtt=15.0), ("127.0.0.1", 50000), transport, ctx) + host = hbdclass.Host.hosts["rtt-hist-host4"] + assert host.plugin_data.get("rtt_ipv4") # rtt history now non-empty + + # Simulate a recovery: connection was dropped (e.g. OVERDUE->UP after a + # missed heartbeat) and is about to come back UP on the next heartbeat. + conn = host.connections["IPv4"] + conn.state = hbdclass.Connection.DOWN + + transport.sent.clear() + handle_datagram(_htb("rtt-hist-host4", rtt=16.0), ("127.0.0.1", 50000), transport, ctx) + + ack_data, _ = transport.sent[0] + ack = parse_message(ack_data) + assert ack.get("request_update") + + +def test_recovery_clears_real_plugin_data_but_preserves_rtt_history(): + """Regression for Finding 2: host.plugin_data.clear() on recovery must + wipe real client-collected plugin data while leaving rtt_* history + samples intact. + """ + hbdclass.Host.hosts.pop("rtt-hist-host5", None) + transport = _FakeTransport() + ctx = _base_ctx() + + for rtt in (10.0, 11.0, 12.0): + handle_datagram(_htb("rtt-hist-host5", rtt=rtt), ("127.0.0.1", 50000), transport, ctx) + + host = hbdclass.Host.hosts["rtt-hist-host5"] + assert len(host.plugin_data["rtt_ipv4"]) == 3 + + # Simulate a drop, and pretend the client had previously sent real + # plugin data (collected before the connection went down). + conn = host.connections["IPv4"] + conn.state = hbdclass.Connection.DOWN + host.add_plugin_data("os_info", {"os": "linux"}, timestamp=time.time()) + assert "os_info" in host.plugin_data + + # Recovery heartbeat. + handle_datagram(_htb("rtt-hist-host5", rtt=13.0), ("127.0.0.1", 50000), transport, ctx) + + assert "os_info" not in host.plugin_data + assert len(host.plugin_data["rtt_ipv4"]) == 4