Skip to content

refactor(curtailment): make control loops restartable (5/9) - #785

Merged
ankitgoswami merged 9 commits into
mainfrom
ankitg/runtimejobs-curtailment
Jul 23, 2026
Merged

refactor(curtailment): make control loops restartable (5/9)#785
ankitgoswami merged 9 commits into
mainfrom
ankitg/runtimejobs-curtailment

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +238/-141 across 4 files (excludes generated, test, and story files).

Summary

Curtailment reconciliation, MQTT ingest, and curtailment alert metrics now share the same restartable lifecycle contract. A passive Fleet can keep these control-input loops stopped, while a clean demotion drains the current activation before a later owner starts them again.

Stack:

  • Foundation: #780 (merged)
  • Parallel domain refactors: diagnostics #781, IP scanning #782, scheduling #783, curtailment #785, command execution #787, and telemetry #786
  • Orchestration: #784
  • Catalog and fleetd cutover: #788

This PR is the curtailment lifecycles slice and now targets main. Merged #780 supplies the shared runtimejobs.Lifecycle contract; the remaining domain and orchestration refactors are independent sibling PRs and can merge in any order. #788 is temporarily based on the integration branch so its reviewable diff contains only catalog/cutover work; after the siblings merge it will be rebased and retargeted to main. Passive-mode coordinator wiring, epoch fencing, and request gating remain later HA work.

How it works

Each concrete loop owns one activation context and drain state. Cancellation of the Start context and an explicit Stop request the same cancellation; Stop then waits within the caller's deadline. The reconciler passes that context into tick database and control work, so activation cancellation also reaches already-admitted work instead of leaving it detached. MQTT gives each activation its own worker accounting and completion signal, and a timed-out stop continues cleanup asynchronously before permitting restart. The alert metrics loop likewise retains ownership until any in-flight tick exits.

flowchart LR
    A["Activation"] --> R["Curtailment reconciler"]
    A --> M["MQTT subscriber"]
    A --> X["Alert metrics loop"]
    D["Activation cancellation or Stop"] --> C["Cancel activation work"]
    C --> R
    C --> M
    C --> X
    R --> Z["Drain before restart"]
    M --> Z
    X --> Z
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
curtailment/reconciler/ Uses one activation context for the loop and admitted tick work Prevents duplicate control actions across owners
curtailment/mqttingest/ Serializes lifecycle and waits for subscriber workers Prevents overlapping source subscriptions
curtailment/alert_metrics.go Adds restartable loop and in-flight tick ownership Avoids old-activation metric writes after restart
server/cmd/fleetd/main.go Uses bounded stop contexts for all three loops Preserves standalone behavior and finite shutdown

Key technical decisions & trade-offs

  • Keep three domain lifecycles separate because they have different dependencies and enablement, while grouping them in one reviewable curtailment PR.
  • Preserve conditional alert-metrics construction when metrics are disabled.
  • Use one run context per lifecycle so cancellation and explicit stop have identical reach.
  • Block restart after a timed-out drain until surviving work actually exits.

Related

Related: #740

Testing & validation

  • go test -short -race -count=1 ./internal/domain/curtailment ./internal/domain/curtailment/mqttingest ./internal/domain/curtailment/reconciler ./cmd/fleetd
  • Covers activation cancellation, timeout, eventual restart, and no overlap; focused cancellation/restart tests were repeated 25 times under the race detector.

Post-Deploy Monitoring & Validation

Watch reconciler errors/control-action counts, MQTT connection and subscription state, alert metric freshness, and shutdown drain logs through at least one reconcile interval. Roll back on duplicate control actions, overlapping MQTT clients, stale alert metrics, or repeated stop-budget failures.


Compound Engineering

@github-actions github-actions Bot added the review-policy: needs-review Managed by the Review Policy workflow. label Jul 21, 2026
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated security-focused code review generated by Codex.
It should be used as a supplementary check alongside human review.
False positives are possible - use your judgment.

Scope summary

  • Reviewed pull request diff only (eaaec6d1026d77e0cb92ae360e31f46758c3e634...c7365e9f12320a1a5a8c64728d1305b94c0cdc5b, exact PR three-dot diff)
  • Model: gpt-5.5

💡 Click "edited" above to see previous reviews for this PR.


Review Summary

Overall Risk: NONE

Findings

No security, correctness, or reliability findings were identified in the reviewed diff.

Notes

Reviewed the changed lifecycle/shutdown handling for the curtailment reconciler, MQTT subscriber, alert metrics loop, and fleetd stop wiring. I attempted targeted Go tests, but the read-only sandbox prevented creating the Go module/cache directories, so validation here is static only.


Generated by Codex Security Review |
Triggered by: @ankitgoswami |
Review workflow run

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 refactors the curtailment control-input loops (reconciler, MQTT ingest subscriber, and alert-metrics loop) to implement the shared runtimejobs.Lifecycle contract so they can be cleanly started, stopped (with bounded drain), and restarted without overlapping activations—supporting future active/passive Fleet ownership handoffs while preserving standalone fleetd behavior.

Changes:

  • Refactors the curtailment reconciler to use an activation-scoped context and a restart-safe Start(ctx)/Stop(ctx) lifecycle.
  • Refactors MQTT ingest subscriber to serialize activations, drain worker goroutines before restart, and expose Stop(ctx) error.
  • Refactors curtailment alert metrics loop to be activation-scoped and restart-safe, including timeout/overlap-prevention behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
server/internal/domain/curtailment/reconciler/reconciler.go Converts reconciler to runtimejobs.Lifecycle with activation-scoped context and restart-safe state.
server/internal/domain/curtailment/reconciler/reconciler_test.go Updates tests for new lifecycle signatures and adds restart/stop-overlap coverage.
server/internal/domain/curtailment/mqttingest/subscriber.go Refactors subscriber to per-activation state, worker draining, and Stop(ctx) semantics.
server/internal/domain/curtailment/mqttingest/subscriber_test.go Adds lifecycle restart/timeout tests and updates stop calls to pass contexts.
server/internal/domain/curtailment/alert_metrics.go Refactors alert metrics loop to runtimejobs.Lifecycle with activation ownership and bounded stop.
server/internal/domain/curtailment/alert_metrics_test.go Adds restart and timeout overlap-prevention tests for the alert metrics loop.
server/cmd/fleetd/main.go Updates standalone startup/shutdown wiring for the three curtailment loops.

Comment thread server/cmd/fleetd/main.go
Comment thread server/internal/domain/curtailment/mqttingest/subscriber.go
chatgpt-codex-connector[bot]

This comment was marked as outdated.

ankitgoswami added a commit that referenced this pull request Jul 22, 2026
- make MQTT cleanup activation-owned and asynchronous
- restore restartability after cancellation or a timed-out stop
ankitgoswami added a commit that referenced this pull request Jul 22, 2026
- derive draining state from activation cancellation
- allow Stop to interrupt MQTT startup
- reuse the bounded standalone shutdown helper
@ankitgoswami
ankitgoswami force-pushed the ankitg/runtimejobs-curtailment branch from 24b14ca to a08a28a Compare July 22, 2026 22:05
- make MQTT cleanup activation-owned and asynchronous
- restore restartability after cancellation or a timed-out stop
- derive draining state from activation cancellation
- allow Stop to interrupt MQTT startup
- reuse the bounded standalone shutdown helper
@ankitgoswami
ankitgoswami force-pushed the ankitg/runtimejobs-curtailment branch from 222b04c to 5ce20e5 Compare July 22, 2026 23:36
chatgpt-codex-connector[bot]

This comment was marked as outdated.

chatgpt-codex-connector[bot]

This comment was marked as outdated.

@github-actions github-actions Bot added review-policy: human-approved Managed by the Review Policy workflow. and removed review-policy: needs-review Managed by the Review Policy workflow. labels Jul 23, 2026
@ankitgoswami
ankitgoswami merged commit e20651d into main Jul 23, 2026
71 checks passed
@ankitgoswami
ankitgoswami deleted the ankitg/runtimejobs-curtailment branch July 23, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-policy: human-approved Managed by the Review Policy workflow. server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants