Skip to content

refactor(schedule): make processor restartable (4/9) - #783

Merged
ankitgoswami merged 8 commits into
mainfrom
ankitg/runtimejobs-scheduler
Jul 22, 2026
Merged

refactor(schedule): make processor restartable (4/9)#783
ankitgoswami merged 8 commits into
mainfrom
ankitg/runtimejobs-scheduler

Conversation

@ankitgoswami

@ankitgoswami ankitgoswami commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Reviewable diff: +152/-84 across 2 files (excludes generated, test, and story files).

Summary

Schedule processing can now be stopped, fully drained, and started again without rebuilding fleetd. This prepares scheduled command recovery, cron callbacks, and end-of-window reverts for active/passive demotion and promotion while preserving standalone behavior.

Stack:

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

This PR targets main and builds on the runtimejobs.Lifecycle contract merged in #780. The sibling domain refactors can merge independently; #788 performs the final catalog/cutover integration. Passive-mode coordinator wiring, epoch fencing, and request gating remain later HA work.

How it works

Each Start creates one schedule activation with two phases. Its admission context is canceled immediately when ownership is withdrawn or Stop begins, preventing loops, timers, and cron from starting new work. Work already admitted keeps a separate activation-owned context through target resolution, command dispatch, and final database state persistence, so a demotion cannot strand a schedule halfway through its workflow.

Stop(ctx) waits for admitted work to drain. If the caller's shutdown deadline expires, it cancels that work context and returns the deadline error; the processor remains unavailable for restart until the old activation has actually exited. Startup database work runs outside the lifecycle lock, allowing Stop to honor its own deadline even when startup is blocked.

flowchart LR
    S["Start activation"] --> R["Recover and register schedules"]
    R --> A["Admit timer, cron, and reconciliation work"]
    A --> W["Run schedule workflow"]
    W --> D["Dispatch command"]
    D --> P["Persist final schedule state"]
    X["Activation canceled or Stop called"] --> C["Close admission"]
    C --> G["Drain admitted work"]
    G --> N["Allow next activation"]
    T["Stop deadline expires"] --> F["Force-cancel admitted work"]
    F --> N
Loading
stateDiagram-v2
    [*] --> Starting
    Starting --> Running: recovery succeeds
    Starting --> Draining: startup fails or is canceled
    Running --> Draining: activation canceled or Stop called
    Draining --> Stopped: callbacks and timers exit
    Draining --> Draining: Stop deadline force-cancels work
    Stopped --> Starting: next Start
Loading

Areas of the code involved

Area / package / file What changed Why it matters for review
server/internal/domain/schedule/processor.go Adds activation-owned admission and work contexts, restart-safe lifecycle state, and bounded Stop behavior Defines the ownership handoff and guarantees that admitted schedule workflows drain as one unit
server/cmd/fleetd/main.go Stops the processor through the shared standalone lifecycle helper Preserves the process-wide shutdown budget
Schedule processor tests Cover startup/stop races, admitted schedule and window-revert draining, deadline cancellation, timer admission, and restart Exercises lifecycle ordering under the race detector

Key technical decisions & trade-offs

  • Model shutdown once at the activation boundary instead of giving individual database writes special cancellation exceptions.
  • Cancel admission immediately, but preserve one context for the full already-admitted workflow so dispatch and persisted state cannot diverge.
  • Let the caller's Stop deadline decide when graceful draining becomes forced cancellation rather than embedding another processor-specific timeout.
  • Reject restart until the previous activation has actually drained, avoiding overlapping owners after a timed-out stop.

Related

Related: #740

Testing & validation

  • go test -short -race -count=1 ./internal/domain/schedule ./cmd/fleetd
  • Focused lifecycle tests repeated 100 times under the race detector.
  • golangci-lint passed for ./internal/domain/schedule/... and ./cmd/fleetd/....
  • Covers activation cancellation during a schedule and end-of-window revert, forced cancellation at the Stop deadline, blocked startup, timer admission, drain completion, and restart.
  • Does not cover future HA coordinator or epoch-fencing behavior; those are outside this PR.

Post-Deploy Monitoring & Validation

Watch schedule recovery/start errors, command creation from due schedules, end-of-window reverts, and shutdown drain logs through at least one schedule interval. Roll back if schedules are duplicated, due work stops dispatching, reverts are missed, or processor drains repeatedly exceed the shutdown budget.


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 (4bf7e7a0e3c126f3cf358badbdd50fc12a4a0860...8ddc468a2efd53bc5f1591aec2ba83674dd6e253, 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 identified in the changed hunks.

Notes

Reviewed .git/codex-review.diff only. The PR scope is limited to schedule processor lifecycle/shutdown behavior, fleetd shutdown wiring, and related tests. No auth, SQL/database, network discovery, plugin, protobuf, frontend, infrastructure, or pool configuration changes were present in the reviewed diff.

Tests were not run in this read-only review environment.


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

@ankitgoswami
ankitgoswami force-pushed the ankitg/runtimejobs-scheduler branch from 83f3395 to 5ad0b87 Compare July 21, 2026 20:22
- begin asynchronous processor cleanup when activation is canceled
- cover restart after cancellation and drain
@ankitgoswami
ankitgoswami marked this pull request as ready for review July 22, 2026 17:10
@ankitgoswami
ankitgoswami requested a review from a team as a code owner July 22, 2026 17:10
Copilot AI review requested due to automatic review settings July 22, 2026 17:10

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

Refactors the schedule processor to implement the shared runtimejobs.Lifecycle contract so it can be started, stopped/drained, and started again without rebuilding fleetd, as groundwork for future active/passive activation control.

Changes:

  • Make schedule.Processor restartable by introducing explicit lifecycle state, per-activation cron/job tracking, and context-aware Start(ctx) / Stop(ctx).
  • Update fleetd to stop the schedule processor via the shared bounded standalone stop helper.
  • Expand schedule processor tests to cover deadline-bounded stop, activation-cancellation behavior, and restart semantics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
server/internal/domain/schedule/processor.go Implements runtimejobs.Lifecycle for the schedule processor, including activation-scoped state and stop/drain logic.
server/internal/domain/schedule/processor_test.go Updates tests for the new Stop(context.Context) API and adds coverage for restart/drain/cancellation cases.
server/cmd/fleetd/main.go Switches schedule processor shutdown to stopStandaloneJob(...) to preserve bounded shutdown behavior.

Comment thread server/internal/domain/schedule/processor.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8513c9b462

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/schedule/processor.go Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a39dd8ae89

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/schedule/processor.go Outdated
- reject restart while a canceled activation is draining
- fence cancellation watchers to their originating activation
- cover both handoff races with deterministic tests

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 80de634ce7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/schedule/processor.go Outdated
- keep post-claim state writes alive after activation cancellation
- bound each finalization and repair attempt
- cover cancellation immediately after successful dispatch

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88f26c96ff

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/schedule/processor.go Outdated
Separate admission cancellation from admitted work so graceful stop drains a schedule through dispatch and final persistence. Keep startup outside the lifecycle lock and force-cancel work only when Stop exhausts its caller deadline.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3553efeb38

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/internal/domain/schedule/processor.go Outdated
Make concurrent Start callers wait for the published activation's startup result while preserving cancellation and deadline-bounded Stop behavior.
@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 22, 2026
@ankitgoswami
ankitgoswami merged commit eaaec6d into main Jul 22, 2026
71 checks passed
@ankitgoswami
ankitgoswami deleted the ankitg/runtimejobs-scheduler branch July 22, 2026 23:32
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