Skip to content

schedulertest: scheduler invariants and deterministic tables#10727

Draft
davidporter-id-au wants to merge 7 commits into
temporalio:mainfrom
davidporter-id-au:feature/chasm-scheduler-invariants
Draft

schedulertest: scheduler invariants and deterministic tables#10727
davidporter-id-au wants to merge 7 commits into
temporalio:mainfrom
davidporter-id-au:feature/chasm-scheduler-invariants

Conversation

@davidporter-id-au

Copy link
Copy Markdown
Contributor

What

Adds CheckInvariants, evaluated after every settled transition via an AfterStep hook, encoding the legitimate-quiescence rules from Scheduler.isHeldOpen and getIdleExpiration:

  • no-stuck (primary): a non-closed schedule may have no pending task only when held open (paused or draining a backfill); any other taskless, non-closed state is the stuck-open bug class.
  • idle-not-while-held-open: a held-open schedule must not arm an idle close.
  • closed-is-terminal: a closed schedule never reopens.
  • high-water-mark monotonicity for the generator and invoker.

Deterministic table tests run several schedules (running, paused, exhausted, short-interval) forward with the hook wired in, plus direct unit tests proving the predicate flags a stuck state and an HWM regression while allowing a legitimate held-open-without-task state.

Stack

Stacked on #10726 (harness) → #10725 (bugfix). Diff against main is cumulative; net-new here is invariants.go + invariants_test.go.

Test plan

  • go test ./chasm/lib/scheduler/schedulertest/ -run 'TestInvariants|TestCheckInvariants'

🤖 Generated with Claude Code

…g schedules

The V1 scheduler's automatic CHASM-migration eligibility check read
len(s.Info.RunningWorkflows) before the same run-loop iteration's
processBuffer() reconciled it. processTimeRange sets NeedRefresh when it
buffers a new action, but the refresh that prunes completed executions only
runs inside processBuffer -- after the check, which also starts the
replacement action. For a continuously-firing schedule the check therefore
always saw the previous action as still running, so len(RunningWorkflows)==0
was never observed and, under the default
EnableCHASMSchedulerMigrationWithRunningWorkflows=false guard, migration was
deferred forever.

Reconcile running-workflow status before the eligibility check so the check
sees the genuine post-completion window and migrates before the next action
starts. Gated behind a new SchedulerWorkflowVersion
(RefreshBeforeMigrationCheck = 13) because refreshWorkflows issues recorded
local activities and reordering it would otherwise break replay of existing
histories.

Tests:
- workflow_test.go: TestAutoMigrateReconcilesRunningWorkflowBeforeCheck unit
  guard (fails without the version bump).
- testdata/replay_migration_v1_to_v2.json.gz: new replay fixture exercising
  the v13 migration branch; TestReplays fails if the early-refresh block is
  reordered/removed. Regenerate via tests/schedule_migration_replay_fixture_gen_test.go
  (GENERATE_REPLAY_FIXTURE=1, inert in CI otherwise).
- tests/schedule_migration_v1_to_v2_callback_compat_test.go: integration
  coverage; _AutomaticMigrationForActiveSchedule now passes.
  _NoUnsupportedHistoryEventOnRunningWorkflow stays skipped (separate
  admin-path callback-attach bug).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@davidporter-id-au
davidporter-id-au force-pushed the feature/chasm-scheduler-invariants branch 2 times, most recently from a814fd8 to fdf1ccd Compare July 18, 2026 20:15
Addresses the PR review notes on the callback-compat tests and clears
the golangci failures on the changed files.

Test unification (schedule_migration_v1_to_v2_callback_compat_test.go):
- Extract shared helpers (createV1Schedule, awaitRunningAction,
  awaitAnyAction, awaitV1SchedulerCompleted, requireNoOptionsUpdatedEvent,
  requireV2ScheduleExists) to kill the ~4x-duplicated boilerplate.
- Rename the admin-path test to
  TestScheduleMigrationV1ToV2_AdminMigratePreservesRunningWorkflowHistory
  (it is the tdbg/CLI poke-migration flow) and tighten the skip reason:
  the admin MigrateSchedule path still injects the unsupported event and
  is genuinely unfixed.
- Merge the two dynamic-config tests into
  TestScheduleMigrationV1ToV2_RolloutMigration, which asserts both that a
  busy schedule migrates (regression guard for the ordering fix in this
  PR) and that no fired workflow gains
  EVENT_TYPE_WORKFLOW_EXECUTION_OPTIONS_UPDATED. The old single-action +
  refresh-nudge scaffolding existed only to sidestep the ordering bug this
  PR fixes, so it is dropped along with the stale "fails on main" framing
  and the local-path reference.

Lint (golangci diff-mode now clean on the changed files):
- Put ctx first in helpers (revive context-as-argument).
- require.Eventually -> await.RequireTrue/RequireTruef in the rollout test
  and the replay-fixture generator (forbidigo).
- s.NoError -> s.Require().NoError in the new workflow_test guard
  (testifylint require-error).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
davidporter-id-au and others added 5 commits July 18, 2026 16:15
Test drivers that step a component forward need to invoke node-level task
dispatch (EachPureTask/ExecuteSideEffectTask) and inspect accumulated physical
tasks, which the Engine API did not expose. Add:

- NodeForRef / BackendForRef accessors on the in-memory Engine.
- WithNodeBackendDecorator option, applied to each execution's MockNodeBackend
  at creation time, so callers can supply handlers the default leaves unset
  (e.g. HandleGetNamespaceEntry, or a constant transition counter).

All additive and default-nil; existing chasmtest users are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit aeddc4227acc0ec1c8a8b6b0034efa0d4c6bc42b)
Add a harness that builds a real CHASM scheduler on a chasmtest.Engine and steps
it forward through every task it schedules for itself, advancing a controllable
clock. Pure tasks dispatch via Node.EachPureTask and side-effect tasks via
Node.ExecuteSideEffectTask (the production dispatch paths). The driver exposes
Step/RunToQuiescence, Pause/Unpause/TriggerImmediately/MigrateToWorkflow
operations, a Snapshot of observable state, and an AfterStep hook.

This lives in a non-test package so property and migration tests can import it.
library.go mirrors the in-package fixtures (NewTestLibrary, DefaultSchedule, ...)
but exported and wired with the mock clients the side-effect handlers need.

Smoke tests cover the three settled behaviors: a running interval schedule ticks
forever, a paused schedule is held open and keeps ticking, and a schedule with
no remaining actions idles then closes.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 77342fdbbdf5ef7fadaa16553a87c496edb2db81)
- library.go: gofmt the NewSpecBuilder call indentation and import
  order (both flagged by CI's fmt/gci checks).
- driver.go: drop the historyservice/historyservicemock import aliases
  -- .github/.golangci.yml's importas rule requires api/*service(mock)
  packages to be unaliased; these matched the package's own name so
  the alias was redundant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Add CheckInvariants, evaluated after every settled transition via an AfterStep
hook, encoding the legitimate-quiescence rules from Scheduler.isHeldOpen and
getIdleExpiration:

- no-stuck (primary): a non-closed schedule may have no pending task only when
  held open (paused or draining a backfill); any other taskless, non-closed
  state is the stuck-open bug class.
- idle-not-while-held-open: a held-open schedule must not arm an idle close.
- closed-is-terminal: a closed schedule never reopens.
- high-water-mark monotonicity for the generator and invoker.

Deterministic table tests run several schedules (running, paused, exhausted,
short-interval) forward with the hook wired in, plus direct unit tests proving
the predicate flags a stuck state and an HWM regression while allowing a
legitimate held-open-without-task state.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 1c2ac604e4638e60b8f2ca180ba25e671f0c936b)
Adds two invariants to CheckInvariants, with the driver instrumentation
they need:

- buffer-bound: total buffered starts must never exceed MaxBufferSize
  (the generator sizes each fill against this cap).
- action-budget: a single InvokerExecuteTask must not take more actions
  (start + terminate + cancel RPCs) than MaxActionsPerExecution. Guards
  the double-spend class where the budget resets across the
  terminate/cancel/start phases.

Driver now counts action RPCs via the mock clients, tracks the max taken
by any single side-effect task dispatch, and exposes BufferedStartsCount
/ MaxBufferSize / MaxActionsObserved / MaxActionsPerExecution on Snapshot.
Adds WithConfig so tests can set small Tweakables to exercise the caps.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant