fix(control): bound control-loop driver sends with a per-command deadline - #841
fix(control): bound control-loop driver sends with a per-command deadline#841Sanjin-Maker wants to merge 1 commit into
Conversation
…line Battery dispatch, PV curtailment and loadpoint sends waited on Registry.Send with the long-lived loop context, so one wedged driver stalled dispatch to every other driver. Wrap each send in the same 2 s deadline the watchdog default path already uses; failures are logged and recovery stays with the watchdog/staleness paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Sanjin Naidu <sanjin@sanrowconsulting.com>
707a5aa to
173e3bc
Compare
miravoss26
left a comment
There was a problem hiding this comment.
Bounds every control-loop driver send (battery, PV curtail, loadpoint) with a 2s deadline via a new sendDriverCommand helper, so a driver wedged mid-poll can no longer stall dispatch to every other driver for the rest of the tick.
- Correctness: matches the existing
driverDefaultTimeoutpattern, errors are logged not propagated (recovery stays with watchdog/staleness, as stated).dispatch_send_test.gocovers the blocked-driver case, pass-through, and swallowed errors — good coverage for a concurrency-adjacent fix. - The loadpoint wrapper (
main.golpSend) correctly derives its deadline from the parent ctx viacontext.WithTimeout, so an already-shorter parent deadline still wins. - No secrets, no new deps, no network/authz surface.
⚠️ GitHub reports this PR asCONFLICTINGagainstmasterand shows no CI check runs yet — worth confirming CI actually ran and rebasing before merge.
Safe to merge from my read once it's rebased and green.
|
Queue-order note: this PR changes go/cmd/ftw/main.go, which the older #701 already owns. #701 is waiting for the required human browser review. Under the repository file-order rule, #701 keeps right of way, so I am not rebasing or merging #841 yet. The current branch is also conflicting and has no checks on this head. Once #701 clears, rebase, run the race test and full make verify, then reassess #841 before #842. |
|
Thank you @Sanjin-Maker for digging into a real control-loop liveness risk and for adding focused tests. We reviewed this stack against current We are closing this PR as superseded, not rejecting the problem you identified. If you want to continue in this area, please start with a written issue rather than a replacement code PR. The issue should describe the remaining behavior on current Thank you for the time and care you put into this. |
Summary
Registry.Sendwith the long-lived loop context; one driver wedged mid-poll (cmdCh full, runLoop busy) stalled dispatch to every other driver for the rest of the tick and beyond.sendDriverCommand(newgo/cmd/ftw/dispatch_send.go) applying the same 2 s deadline the watchdog default path already uses (sendDriverDefault,driverDefaultTimeout), and route the battery + curtail loops through it.loadpoint.NewControllerwith the same deadline — the controller passes long-lived (orcontext.Background()) contexts in several paths.Relationship to #798: no file overlap (that PR reworks
registry.go's default-command queueing; this one bounds the callers incmd/ftw). The two compose — with #798's priority slot, a timed-out normal command can never starve a safety default. Happy to rebase if #798 lands first.Verification
go test ./cmd/ftw/ -count=1including newdispatch_send_test.go: blocked-driver send returns at the deadline with a deadline-carrying context; payload pass-through; errors swallowed-and-logged.go build ./...,go vet ./cmd/ftw/.🤖 Generated with Claude Code