feat(control): fan dispatch out concurrently within each phase - #842
feat(control): fan dispatch out concurrently within each phase#842Sanjin-Maker wants to merge 2 commits 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>
Battery and PV-curtail sends ran serially, so N slow drivers cost N send-times per tick. Each phase now fans out with a WaitGroup under a 1.5 s budget; per-driver deadlines from the previous change still apply. Phase ordering (EV, battery, curtail) is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Sanjin Naidu <sanjin@sanrowconsulting.com>
38185bc to
d324f84
Compare
miravoss26
left a comment
There was a problem hiding this comment.
Stacked on #841 (its first commit is that PR) — reviewing the incremental piece: fans battery/PV-curtail dispatch out concurrently within a phase via a new dispatchCommands, bounded by a 1.5s per-phase budget so N slow drivers can't blow the 2s control tick.
- Correctness:
dispatchCommands'sbudgetCtx(1.5s) wrapssendDriverCommand's ownWithTimeout(2s) —context.WithTimeouttakes the earlier deadline, so every send is effectively capped at 1.5s as intended. Phase ordering (EV → battery → curtail) is preserved; parallelism is only within a phase. - Cross-driver parallelism claim (each driver owns its runLoop + queue) matches the architecture described in #841;
TestDispatchCommandsRunsConcurrentlyandTestDispatchCommandsHonorsTickBudgetback it with a race-flagged test run per the PR description. - No secrets, no new deps, no authz/network surface.
⚠️ Same as #841:CONFLICTINGagainstmaster, no CI check runs shown yet. This one also can't land before #841 does.
Safe to merge from my read, after #841 lands and this rebases clean.
|
Merge blocker: dispatchTickBudget is applied separately to the battery phase and the PV-curtail phase, but those phases run one after the other. If both contain blocked sends, dispatchCommands can wait about 1.5 s twice, so this code can spend more than 3 s in dispatch before the rest of a nominal 2 s control tick. The budget test covers only one phase and misses that combined path. Use one deadline shared by both phases, or pass the remaining tick budget into the second phase. Add a test that runs blocked battery and curtail phases in sequence and proves their combined wait stays below the one tick budget. #841 and the older main.go owner #701 must also land first. |
|
Thank you @Sanjin-Maker. Concurrent fan-out may still be useful, but this PR carries the superseded #841 implementation and gives the battery and PV phases separate 1.5 s budgets. In sequence, those phases can exceed one 2 s control tick. We are closing this version with the stack. A future proposal should start as a written issue and define one shared whole-tick budget, phase ordering, cancellation behavior and a test that runs the combined battery-plus-PV path. Once we agree on that contract, it can become one focused PR. |
Summary
Stacked on #841 — the first commit here is that PR; review only the last commit until it lands, then this rebases cleanly.
dispatchCommandsingo/cmd/ftw/dispatch_send.go) with async.WaitGroupjoin under a 1.5 s per-phase budget. Cross-driver parallelism is safe: every driver has its own runLoop goroutine and command queue.Verification
go test ./cmd/ftw/ -count=1 -race: concurrency test (4×200 ms sends complete in ~1 delay, not 4), budget test (wedged senders return at the 1.5 s budget), empty no-op.go build ./...,go vet ./cmd/ftw/. Patch changeset included.🤖 Generated with Claude Code