Skip to content

fix(daemon/shaper): wake the statusz poll loop on endpoint discovery - #1001

Merged
alex-au merged 1 commit into
mainfrom
01000-shaper-entry-reconcile-race
Aug 13, 2026
Merged

fix(daemon/shaper): wake the statusz poll loop on endpoint discovery#1001
alex-au merged 1 commit into
mainfrom
01000-shaper-entry-reconcile-race

Conversation

@alex-au

@alex-au alex-au commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Description

After block node install --traffic-shaping-enabled, the inet weaver-workload-policy sets stayed
empty for a full poll interval (5 min default) — even though the BN pod was ContainersReady and
statusz was serving the correct roster the whole time. Same on reconfigure, host reboot, and
systemctl restart solo-provisioner-daemon.

Measured on a local ENV-2 install before this change:

Time Event Source
23:08:36 BN pod reaches ContainersReady=True pod-condition poll (2 s)
23:08:37 block node install workflow completes setup_report_20260812_230837.yaml
23:08:38 daemon install completes (802 ms) → daemon starts setup_report_20260812_230838.yaml
23:13:39 bn-publisher / bn-partner-out / bn-backfill populate nft poll (2 s)

Δ = 5 min 01 s — exactly one poll interval. Note the pod was ready two seconds before the daemon
started
, so this was never "waiting for the pod": everything the daemon needed was in place at T₀,
and statusz answered 200 with the right roster throughout.

Root cause

Run starts the pod watcher and the poll loop concurrently (traffic_shaper_monitor.go:126-136), so
on a cold start the entry reconcile (:343) normally runs before the watcher's initial
pods.List() (pod_watcher.go:72) has recorded an endpoint. Then:

  1. effectiveStatuszURL() returns "".
  2. The empty-URL branch returns nil, not an error — so superviseResponsibility does not retry
    it with the 5 s→5 min back-off.
  3. The loop parks on select { <-ctx.Done(); <-ticker.C }. ticker.C was the only wake-up source,
    so nothing told it the endpoint appeared moments later.

A few milliseconds of startup race cost a full interval of convergence.

Why the buffer is load-bearing — the main thing to review

urlChanged is make(chan struct{}, 1) with a non-blocking send. That is not incidental: it is
what makes the signal order-independent. The watcher frequently signals before the poll loop reaches
its select. With capacity 1 the signal waits in the buffer and the select returns immediately; an
unbuffered send would find no receiver, drop silently, and reintroduce the exact race this closes —
just moved a few lines. TestSignalURLChanged_RetainsSignalForLateReceiver pins this property
directly, with no timing dependence.

Relationship to #964 — this is a composition regression

#964 §3 documented the caveat as accepted: "When using pod discovery only (no base_url) … convergence
is then bounded by up to one poll interval after discovery."
Two changes five days apart made that bite:

Date Change Effect
2026-07-30 #936 (issue #918) — discover the endpoint from the BN pod instead of a fixed base_url (enabled by hiero-ledger/hiero-block-node#3316) Moved every default install off the base_url path onto the pod-discovery path. Cost then: ~5 s
2026-08-04 #965 (issue #964) — default poll interval 5 s → 5 min Same latent race now costs 5 min — 60×

Before #936, operators set base_url explicitly in the install TUI, which returns immediately from
effectiveStatuszURL() and never consults the watcher — which is why the guarantee held in practice and
this race went unnoticed. Deliberately not reverting to base_url: discovery follows the pod across
restarts, reschedules and IP changes, which a fixed URL cannot. This PR gives the discovery path the
startup convergence base_url already had.

The caveat also understated the trigger — it describes waiting for the pod to become ready, but here
the pod was ready before the daemon started. The race is against the watcher's initial list, not pod
readiness.

Files changed

File Change
internal/daemon/blocknode/traffic_shaper_monitor.go Add urlChanged (cap 1) + signalURLChanged(); select on it alongside ticker.C; rewrite the entry-reconcile caveat comment
internal/daemon/blocknode/pod_watcher.go Signal on endpoint discovery/change (recordDiscoveredStatusz) and on clear-by-delete (handlePodDelete)
internal/daemon/blocknode/traffic_shaper_monitor_test.go 4 tests: buffering guarantee, coalescing/nil-safety, signal-wakes-before-tick, loss signal is harmless
internal/daemon/blocknode/pod_watcher_test.go 2 tests: signals only on change, signals when the endpoint is cleared
docs/dev/daemon/traffic-shaper-statusz.md Rewrite the pod-discovery paragraph — convergence no longer bounded by poll_interval
docs/dev/traffic-shaper.md Reboot table no longer says "pod readiness + up to one poll interval"

No informer is involved — runPodWatcher is a raw List + Watch, so the WaitForCacheSync wording in
issue #1000's proposed fix was not implementable as written. It also turned out unnecessary: with the
buffered channel the entry reconcile does not need to observe a URL at all.

Review guide

Checklist

  • traffic_shaper_monitor.gourlChanged is created with capacity 1; a change to 0 silently
    restores the bug (only the new unit test would catch it)
  • signalURLChanged() uses select { case ch <- struct{}{}: default: } — must never block, since it
    is called from the watch-event path
  • Nil-channel guard is present: unit-test scaffolding constructs TrafficShaperMonitor directly, and a
    nil channel in select blocks forever rather than panicking (so a missing guard would be silent)
  • pod_watcher.go:279 — signal is sent after m.mu.Unlock(), never while holding the mutex
  • handlePodDelete — signal is sent after the unlock and before the if !ok { return } early
    return, so a pod with no recorded veth still reports the loss
  • handlePodDelete only signals when endpointCleared — an unrelated pod's delete must stay silent
  • No change to the digest gate: a "" → url transition already resets lastDigest/lastApply
    (:256-275), so the woken reconcile does a fresh apply rather than short-circuiting

Test commands

go test -race -count=1 ./internal/daemon/blocknode/...
go test -race -count=1 -run 'TestSignalURLChanged|TestRunStatuszPoll_DiscoverySignal|TestRecordDiscoveredStatusz_SignalsOnlyOnChange|TestHandlePodDelete_SignalsWhenEndpointCleared' ./internal/daemon/blocknode/
task vm:test:unit    # full suite; internal/mount is Linux-only so macOS cannot run it
task lint

TestRunStatuszPoll_DiscoverySignalWakesLoopBeforeNextTick uses a 1-hour poll interval, so the ticker
cannot mask a missing signal. Verified it fails without the fix — replacing the new select case with a
never-ready channel produces timed out waiting for count >= 1 (last: 0).

Manual UAT

Requires a BN host with the plane installed and converged, and a BN whose statusz serves
/statusz/inbound|outbound (verify with curl -s -o /dev/null -w '%{http_code}\n' http://<podIP>:40983/statusz/inbound
200). Run the observer in one shell throughout:

sudo -v
while :; do
  R=$(kubectl -n block-node get pod -l app.kubernetes.io/name=block-node-server \
        -o jsonpath='{.items[0].status.conditions[?(@.type=="ContainersReady")].status}' 2>/dev/null)
  EL=$(sudo nft list set inet weaver-workload-policy bn-publisher 2>/dev/null \
        | tr -d '\n' | sed -n 's/.*elements = {\(.*\)}.*/\1/p')
  printf '%s ready=%-5s publisher={%s}\n' "$(date +%H:%M:%S)" "${R:-none}" "${EL:-EMPTY}"
  sleep 2
done

Set STS=$(kubectl -n block-node get statefulset -o name | head -1) for the cases below.

UAT-1 — Fresh install (the reported bug). Run block node install --traffic-shaping-enabled … with
the observer already running. Record pod-ready time, daemon start
(systemctl show solo-provisioner-daemon -p ActiveEnterTimestamp) and the first non-empty publisher={…}.

Expected: membership within seconds of pod-ready. Before the fix: pod-ready + ~5 min.

UAT-2 — Daemon restart with no ready pod (fast loop, ~2 min).

kubectl -n block-node scale $STS --replicas=0
kubectl -n block-node wait --for=delete pod -l app.kubernetes.io/name=block-node-server --timeout=120s
sudo systemctl restart solo-provisioner-daemon; echo "T0=$(date +%H:%M:%S)"
sudo nft flush set inet weaver-workload-policy bn-publisher
sudo nft flush set inet weaver-workload-policy bn-partner-out
kubectl -n block-node scale $STS --replicas=1

Expected: publisher={…} within seconds of ready=True, not at T0 + 5 min. This is the primary
gate — it forces the daemon to start with no endpoint, which is the race.

UAT-3 — Control: convergence must stop tracking poll_interval. Repeat UAT-2 with
components.block_node.statusz.poll_interval set to 30s, then 5m, in
/opt/solo/weaver/config/daemon.yaml (restart the daemon — there is no hot-reload).

Expected: convergence latency is the same in both runs and tracks pod-ready. Before the fix it
tracked the interval — that correlation was the diagnostic that proved the bug, so it disappearing is the
proof it is fixed.

UAT-4 — Pod reschedule changes the endpoint. With everything converged,
kubectl -n block-node delete pod -l app.kubernetes.io/name=block-node-server. The replacement usually
gets a new pod IP.

Expected: journalctl -u solo-provisioner-daemon shows TrafficShaperStatuszDiscovered with the new
statusz_url, then TrafficShaperStatuszEndpointResolved and a reconcile within seconds — not one
interval later. Membership re-populates. Also confirm the $VETH HTB re-attaches (tc class show dev <veth>).

UAT-5 — base_url path unregressed. Set statusz.base_url to the pod IP endpoint, restart the daemon,
repeat UAT-2.

Expected: unchanged prompt convergence. The override short-circuits effectiveStatuszURL() before the
discovered URL is read, so signals are inert on this path.

UAT-6 — Endpoint loss is logged promptly. kubectl -n block-node scale $STS --replicas=0.

Expected: TrafficShaperStatuszEndpointLost within seconds, not up to one interval later. No exec fires
(the woken reconcile no-ops on an empty URL) — confirm no reconcile-shaper invocation in the journal.

UAT-7 — No signal storm / no redundant applies. Leave the node idle and converged for ~15 min.

Expected: one --check probe per tick, and no privileged apply while the roster is unchanged (the
digest gate holds). Only the hourly force-resync should apply. Confirms extra wake-ups are cheap.

Risks / rollback

  • Signal storm — a pod flapping ready/not-ready could wake the loop repeatedly. Bounded by the
    capacity-1 buffer (coalescing) and the digest gate (no apply when unchanged), so the worst case is one
    unprivileged --check probe per distinct transition. UAT-7 covers this.
  • Send on nil / closed channel — the channel is never closed and the send is non-blocking; the nil case
    is guarded for direct-construction in tests.
  • Rollback — revert the commit. No on-disk format, config, or CLI surface changes; behaviour returns to
    waiting for the tick.

Not covered here: GET /block_node/traffic_shaper/status reports {"monitor":{"state":"running"}} even
while a responsibility is in permanent back-off — observed during this investigation and filed as field
evidence on #750, which already owns per-responsibility health reporting.

Related Issues

The traffic-shaper poll loop's only wake-up source was its ticker, so a
cold start converged a full poll interval (5 min by default) late even
when the BN pod was already ready and statusz was serving.

Run starts the pod watcher and the poll loop concurrently, so the entry
reconcile normally runs before the watcher's initial list has recorded an
endpoint. effectiveStatuszURL then returns "", the empty-URL branch
returns nil rather than an error (so superviseResponsibility does not
retry it), and the loop parks on ticker.C with nothing to tell it the
endpoint arrived moments later.

Add a capacity-1 urlChanged channel, signalled by the pod watcher when it
records, changes, or clears the discovered endpoint, and select on it
alongside the ticker. The buffer is load-bearing: it makes the signal
order-independent, so a send landing before the loop reaches its select
is retained rather than dropped — an unbuffered channel would reintroduce
the same race. Extra wake-ups are absorbed by the existing digest gate.

Measured on a local install before the fix: pod ContainersReady 23:08:36,
daemon start 23:08:38, membership 23:13:39 — 5m01s, exactly one poll
interval, with statusz returning 200 throughout.

Fixes #1000

Signed-off-by: alex-au <alex.w.aus@gmail.com>
@alex-au
alex-au requested a review from a team as a code owner August 12, 2026 14:26
@alex-au
alex-au requested a review from JeffreyDallas August 12, 2026 14:26
@swirlds-automation

swirlds-automation commented Aug 12, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a startup convergence race in the block-node traffic-shaper daemon by waking the statusz poll loop immediately when the BN statusz endpoint is discovered/changed/lost via the pod watcher, rather than waiting for the next poll tick. This improves time-to-populate the daemon-owned nft sets on cold starts, restarts, and reschedules—especially after the default poll interval increase to 5 minutes.

Changes:

  • Add a buffered, non-blocking urlChanged signal channel to wake runStatuszPoll as soon as the discovered statusz URL changes.
  • Emit wake-up signals from the pod watcher on endpoint discovery/change and on endpoint clear-by-delete.
  • Add focused unit tests that pin the buffering/coalescing behavior and validate signal-driven convergence independent of the ticker cadence.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/daemon/blocknode/traffic_shaper_monitor.go Adds urlChanged + signalURLChanged() and wakes the poll loop on discovery signals.
internal/daemon/blocknode/pod_watcher.go Signals the poll loop when the discovered statusz endpoint is recorded or cleared.
internal/daemon/blocknode/traffic_shaper_monitor_test.go Adds regression + behavior tests for buffered signalling and signal-driven wake-ups.
internal/daemon/blocknode/pod_watcher_test.go Adds tests ensuring signals fire only on endpoint changes and on endpoint clear.
docs/dev/traffic-shaper.md Updates reboot/convergence documentation to reflect immediate wake-up on discovery.
docs/dev/daemon/traffic-shaper-statusz.md Updates pod-discovery behavior docs to remove “bounded by poll interval” caveat.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@brunodam brunodam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@alex-au
alex-au merged commit f135d25 into main Aug 13, 2026
21 checks passed
@alex-au
alex-au deleted the 01000-shaper-entry-reconcile-race branch August 13, 2026 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(daemon/shaper): entry reconcile loses a startup race with the pod watcher, delaying nft membership by a full poll interval

4 participants