From 92ba1adf294a62f8c755669e42cd72e60d63ed3f Mon Sep 17 00:00:00 2001 From: mikael-lovqvists-claude-agent Date: Mon, 30 Mar 2026 04:39:52 +0000 Subject: [PATCH] docs: add discovery flow diagrams, document restart detection limitation - Four Mermaid sequence diagrams: startup, steady-state keepalive, node loss/timeout, and node restart - Explicitly document that the site_id-change restart heuristic does not work in practice (site_id is static config, not a runtime value) - Describe what needs to change: a boot nonce (random u32 at startup) - Add boot nonce as a deferred item in planning.md Co-Authored-By: Claude Sonnet 4.6 --- docs/discovery.md | 78 ++++++++++++++++++++++++++++++++++++++++++----- planning.md | 1 + 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/docs/discovery.md b/docs/discovery.md index 61faeb3..2ca922d 100644 --- a/docs/discovery.md +++ b/docs/discovery.md @@ -43,17 +43,81 @@ A node may set multiple bits — a relay that also archives sets both `RELAY` an ### Behaviour -- Nodes send announcements periodically (e.g. every 5 s) and immediately on startup via multicast +- Nodes send announcements periodically (default every 5 s) and immediately on startup via multicast - No daemon — the node process itself sends and listens; no background service required -- On receiving an announcement the control plane records the peer (address, port, name, function) and can initiate a transport connection if needed -- A node going silent for a configured number of announcement intervals is considered offline -- Announcements are informational only — the hub validates identity at connection time +- On receiving an announcement the node records the peer (address, port, name, capabilities) and can initiate a transport connection if needed +- A peer that goes silent for `timeout_intervals × interval_ms` is considered offline and removed from the peer table +- Announcements are informational only — identity is validated at TCP connection time -#### Targeted replies +#### Startup — new node joins the network -Multicast is only used for the periodic keep-alive broadcast. When a node receives an announcement from a peer it does not yet know, or detects that a known peer has restarted (its `site_id` changed for the same address and port), it sends an **immediate unicast reply** directly back to that peer's IP address. This ensures the new or restarted peer learns about this node quickly without waiting up to `interval_ms`, while avoiding a multicast blast that would unnecessarily wake every other node on the subnet. +```mermaid +sequenceDiagram + participant N as New Node + participant MC as Multicast group + participant A as Node A + participant B as Node B -Steady-state keepalive packets from already-known peers do not trigger any reply. + N->>MC: announce (multicast) + MC-->>A: receives announce + MC-->>B: receives announce + A->>N: announce (unicast reply) + B->>N: announce (unicast reply) + Note over N,B: All parties now know each other.
Subsequent keepalives are multicast only. +``` + +Each node that hears a new peer sends a **unicast reply** directly to that peer. This allows the new node to populate its peer table within one round-trip rather than waiting up to `interval_ms` for other nodes' next scheduled broadcast. + +#### Steady-state keepalive + +```mermaid +sequenceDiagram + participant A as Node A + participant MC as Multicast group + participant B as Node B + participant C as Node C + + loop every interval_ms + A->>MC: announce (multicast) + MC-->>B: receives — updates last_seen_ms, no reply + MC-->>C: receives — updates last_seen_ms, no reply + end +``` + +Known peers update their `last_seen_ms` timestamp and do nothing else. No reply is sent, so there is no amplification. + +#### Node loss — timeout + +```mermaid +sequenceDiagram + participant A as Node A + participant B as Node B (offline) + + Note over B: Node B stops sending + loop timeout_intervals × interval_ms elapses + A->>A: check_timeouts() — not yet expired + end + A->>A: check_timeouts() — expired, remove B + A->>A: on_peer_lost(B) callback +``` + +#### Node restart — known limitation + +The current implementation attempts to detect a restart by checking whether `site_id` changed for a known `(addr, port)` entry. In practice this **does not work**: `site_id` is a static configuration value and will be the same before and after a restart. A restarted node will therefore simply be treated as a continuing keepalive and will not receive an immediate unicast reply — it will have to wait up to `interval_ms` for the next scheduled multicast broadcast from its peers. + +```mermaid +sequenceDiagram + participant R as Restarted Node + participant MC as Multicast group + participant A as Node A + + Note over R: Node restarts — same addr, port, site_id + R->>MC: announce (multicast) + MC-->>A: receives — site_id unchanged, treated as keepalive + Note over A: No unicast reply sent. R waits up to interval_ms
to learn about A via A's next scheduled multicast. +``` + +**What needs to change:** a **boot nonce** (random `u32` generated at startup, not configured) should be added to the announcement payload. A change in boot nonce for a known peer unambiguously signals a restart and triggers an immediate unicast reply. This requires a wire format version bump and updates to the peer table struct, announcement builder, and receive logic. ### No Avahi/Bonjour Dependency diff --git a/planning.md b/planning.md index 9f34fd2..70cacf7 100644 --- a/planning.md +++ b/planning.md @@ -136,4 +136,5 @@ These are open questions tracked in `architecture.md` that do not need to be res - controller_cli is a temporary dev tool; the long-term replacement is a dedicated `controller` binary outside `dev/cli/` that maintains simultaneous connections to all discovered nodes (not switching between them). Commands address a specific node by peer index. This mirrors the web UI's model of administering the whole network rather than one node at a time. The `connect` / active-connection model in the current controller_cli is an interim design choice that should not be carried forward. - start-ingest peer addressing: the `dest_host` + `dest_port` in START_INGEST is awkward to type manually and requires the caller to know the target's TCP port. Should accept a peer ID (index from the discovered peer table on the node) so the node can resolve the address itself. Requires the node to run discovery and expose its peer table. - Connection multiplexing: currently each ingest stream opens its own outbound TCP connection to the destination. Multiple streams between the same two peers should share one connection, with stream_id used to demultiplex frames. This is the priority/encapsulation scheme described in the architecture — high-priority and low-latency frames from different streams travel over the same socket rather than competing across separate sockets. +- Discovery boot nonce: the announcement payload needs a `boot_nonce` field (random u32 generated at startup, not configured). The current restart detection uses `site_id` change as a proxy, but `site_id` is static config and does not change on restart, so restarts are not detected and the restarted node waits up to `interval_ms` for peers to reply. Adding a boot nonce gives a reliable restart signal: a nonce change for a known (addr, port) entry triggers an immediate unicast reply. Requires a wire format version bump, peer table struct update, and changes to the announcement builder and receive logic. - Control grouping: controls should be organizable into named groups for both display organisation (collapsible sections in a UI) and protocol semantics (enumerate controls within a group, set a group of related controls atomically). Relevant for display devices where scale_mode, anchor, position, and size are logically related, and for cameras where white balance, exposure, and gain belong together. The current flat list of (control_id, name, type, value) tuples does not capture this.