Skip to content

feat(control): fan dispatch out concurrently within each phase - #842

Closed
Sanjin-Maker wants to merge 2 commits into
srcfl:masterfrom
Sanjin-Maker:agent/dispatch-parallel-fanout
Closed

feat(control): fan dispatch out concurrently within each phase#842
Sanjin-Maker wants to merge 2 commits into
srcfl:masterfrom
Sanjin-Maker:agent/dispatch-parallel-fanout

Conversation

@Sanjin-Maker

Copy link
Copy Markdown
Contributor

Summary

Stacked on #841 — the first commit here is that PR; review only the last commit until it lands, then this rebases cleanly.

  • Battery-target and PV-curtail sends ran serially in the control tick, so N slow drivers cost N send-times per tick; a handful of devices at their (new) 2 s per-command deadline could blow the 2 s control interval.
  • Each dispatch phase now fans out concurrently (dispatchCommands in go/cmd/ftw/dispatch_send.go) with a sync.WaitGroup join under a 1.5 s per-phase budget. Cross-driver parallelism is safe: every driver has its own runLoop goroutine and command queue.
  • Phase ordering preserved: EV dispatch stays at the top of the tick, then battery targets, then PV curtailment — parallelism exists only within a phase.
  • Per-driver send latency logged at debug.

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

Sanjin-Maker and others added 2 commits August 6, 2026 22:07
…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>
@Sanjin-Maker
Sanjin-Maker force-pushed the agent/dispatch-parallel-fanout branch from 38185bc to d324f84 Compare August 6, 2026 20:09

@miravoss26 miravoss26 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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's budgetCtx (1.5s) wraps sendDriverCommand's own WithTimeout (2s) — context.WithTimeout takes 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; TestDispatchCommandsRunsConcurrently and TestDispatchCommandsHonorsTickBudget back it with a race-flagged test run per the PR description.
  • No secrets, no new deps, no authz/network surface.
  • ⚠️ Same as #841: CONFLICTING against master, 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.

@frahlg

frahlg commented Aug 7, 2026

Copy link
Copy Markdown
Member

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.

frahlg commented Aug 8, 2026

Copy link
Copy Markdown
Member

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.

@frahlg frahlg closed this Aug 8, 2026
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.

3 participants