fix: [Scheduler] V1->V2 migration-eligibility fix and migrated-start ID - #11462
Merged
davidporter-id-au merged 65 commits intoAug 26, 2026
Conversation
… ID preservation under one v13 Combines PR temporalio#11134 and PR temporalio#11427, which independently introduced a new SchedulerWorkflowVersion = 13 for two different fixes. Shipping them separately would require two separate version-bump deploys for the "same" version number. This merges both behavioral changes under one shared v13: - RefreshBeforeMigrationCheck (from temporalio#11134): the V1 scheduler's automatic CHASM-migration eligibility check read len(s.Info.RunningWorkflows) before the same run-loop iteration's processBuffer() reconciled it, so an actively-firing schedule never observed an idle window and deferred migration forever. Reconcile running-workflow status before the eligibility check so it sees the genuine post-completion window. - PreserveMigratedStartIDs (from temporalio#11427): V1 discarded the workflow/request IDs already stored on buffered starts migrated from CHASM, breaking idempotency identity across the migration handoff. Prefer those IDs when present, falling back to the existing derivation/UUID generation. Following temporalio#11134's two-phase-rollout rationale, this PR only teaches the scheduler to *understand* v13 for safe replay/rollback; both fixes are gated behind hasMinVersion(13) but CurrentTweakablePolicies.Version stays at TriggerImmediatelyTimestamp (12). A follow-up deploy bumps Version to 13 to activate both fixes at once -- a single activation instead of two. Brings in temporalio#11134's replay fixture (testdata/replay_migration_v1_to_v2.json.gz) and integration test file (tests/schedule_migration_v1_to_v2_callback_compat_test.go) verbatim, and temporalio#11427's workflow.go/workflow_test.go changes, retargeting the version gate and forcing the version in TestMigratedBufferedStartPreservesIdempotencyIDs since Version no longer defaults to 13 here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
davidporter-id-au
marked this pull request as ready for review
August 12, 2026 05:36
| ) | ||
| } | ||
|
|
||
| if s.hasMinVersion(MigrationHandoffFixes) && |
Contributor
Author
There was a problem hiding this comment.
this fixes migration because it means the migration trigger can correctly detect if previously the launched work workflow is done or not (and therefore if it can kick off a migration or if it has to wait for it to be done)
Contributor
There was a problem hiding this comment.
does this fix in the retry case as well? retries are currently driven by the spec and we allow the schedule to continue it's work. second attempt could have newly created running workflows.
Contributor
Author
There was a problem hiding this comment.
the retry fix is on line 359, sorry I missed your comment earlier
chaptersix
reviewed
Aug 13, 2026
chaptersix
reviewed
Aug 13, 2026
If EnableCHASMSchedulerMigration is enabled, bounces on a transient error, and is then rolled back, the sleeping V1 scheduler workflow had no way to notice: PendingMigration is a persisted latch that, once set, retries the migration unconditionally on every wake-up regardless of the current flag value. Depending on the schedule's own cadence, that retry (and thus the migration) could fire long after the flag was believed to be off. Fix this in the local activity itself rather than in workflow code: MigrateScheduleToChasm now does a live (uncached) check of EnableCHASMSchedulerMigration right before calling CreateFromMigrationState, and fails if it's off. The workflow's retry loop is unchanged -- it just logs the failure and keeps going, so a disabled migration spins harmlessly (and resumes the moment the flag comes back on) without ever blocking the schedule's own actions. Also arm TestScheduleMigrationV1ToV2_RolloutMigration by setting CurrentTweakablePolicies.Version to MigrationHandoffFixes for the duration of the test, since that fix is still dormant pending a follow-up activation deploy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
davidporter-id-au
force-pushed
the
bugfix/scheduler-v13-merge-migration-and-id-fixes
branch
from
August 14, 2026 06:12
24e649e to
a1a405d
Compare
…mporalio#10817) ## What changed? Adds `worker.schedulerV1VersionCeiling`, a per-namespace dynamic config that clamps the V1 scheduler workflow's recorded `TweakablePolicies.Version` to `min(current, ceiling)`. This will artificially gate the execution's functionality, to enable backwards-compatibility with historical server versions (in a cross-version multi-cluster setup). ## Why? In a cross-version multi-cluster topology, a newer cluster can write scheduler history an older rollback peer cannot replay after a failover plus rollback. Clamping the recorded version to a configured ceiling lets the newer cluster emit history the older cluster can replay. ## How did you test it? - [x] built - [x] added new unit test(s)
Go 1.27 prerequisite that applies the `slicesbackward` Go fixer for reverse iteration. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## What changed The completion (callback) handler's request-scoped logger carried only the namespace, even though a richer one was built just above it. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Go 1.27 prerequisite that makes `UnprocessableTaskError` pointer-only for the stricter errortype vet check.
…kens (temporalio#11582) ## What changed? Refactor NamespaceRateLimitInterceptor with functions to consume N tokens: - removed `tokens` overwrite argument as it's never used - added functions to consume N tokens The changes itself in this PR is no-op since it's introducing new functions to the interface. ## Why? Added flexibility to wrap `NamespaceRateLimitInterceptor`. ## How did you test it? - [x] built - [x] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) ## Potential risks
## What changed? - emit remote cluster and namespace replication lifecycle records under `namespace_lifecycle` - retain compatibility aliases for specialized event-name constants with TODO cleanup - update tests and shared envelope documentation ## Why? Update schema event names to match namespace lifecycle schema which currently exist so they are properly interpreted. ## How did you test it? - [x] built - [x] run locally and tested manually - [ ] added new unit test(s) - [ ] added new functional test(s)
…alio#11695) ## What changed? Adds metrics for how often and by how much queue slices fail to narrow their predicate, and how large persisted queue state actually is. - `queue_slice_pending_keys` — histogram, recorded on every narrowing attempt (declined or succeeded). This is the distribution `queueShrinkPredicateMaxPendingKeys` should be sized against. - `shard_info_size` / `queue_state_size` — histograms recorded when a shard record is actually written, giving whole-record and per-category size. - `queue_state_size_total` / `queue_slice_count_total` — counters paired with the histograms above (and with the existing `queue_slice_count`), so an exact bytes-per-slice ratio is possible. - `queue_slice_count` gains a `task_category` tag (previously untagged beyond `operation`). These are only metrics changes - no behavior changes. ## Why? A slice only narrows its predicate below `queueShrinkPredicateMaxPendingKeys` (10) pending namespaces; above that it stays universal and re-reads the whole range every time. Raising that threshold safely requires knowing the pending-key distribution and the persisted size. This PR is the baseline for evaluating a follow-on encoding. There are two counters because this server's tally-backed Prometheus reporter doesn't preserve the true recorded value when a histogram flushes — it replays each sample as its bucket's upper bound, so a histogram's `_sum` has no more precision than its buckets. ## How did you test it? - [x] built - [x] covered by existing tests - [x] added new unit test(s) - [x] run locally and tested manually (`queue_predicate_resolution_loss` confirmed live against a local server under forced narrowing-decline conditions) ## Potential risks
…ralio#11723) ## What changed? `GetWorkflowExecutionHistory` and `GetWorkflowExecutionHistoryReverse` now check `branch_token` in the page token against the token in mutable state. ## Why? To confirm if it is still the correct branch after conflict resolution. ## How did you test it? - [x] built - [x] run locally and tested manually - [x] covered by existing tests - [x] added new unit test(s) - [x] added new functional test(s)
…ts have the same view of the branch token. (temporalio#11704) ## What changed? ForkHistoryBranchResponse now carries BaseBranchToken, and the two reset paths rebuild through it when the store set it. A store that doesn't (Cassandra, SQL) leaves it nil and both call sites fall back to the token the caller already had, so behavior is unchanged everywhere else. ## Why? ForkHistoryBranch can modify the base branch token in ways the caller may not have visibility into. This PR fixes it so that the changed token is returned to the caller. ## How did you test it? - [ ] built - [ ] added new unit test(s) - [ ] added new functional test(s) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Go 1.27 prerequisite that applies the `rangeint` Go fixer to use integer range loops.
## What changed? - Reorganize `chasm/lib/activity` ## Why? - Improve navigability and codebase comprehensibility ## How did you test it? - [x] covered by existing tests
## What changed? Pass one test-owned context through namespace creation, namespace cache polling, and search attribute registration during functional test setup. ## Why? Reusing the test context avoids creating independent timeout contexts for each setup RPC and ties their resources to the test lifecycle.
## What changed? The `commonpb.Callback`-variant of `commonpb.Callback_Internal` is unused and should be removed entirely. This PR removes the remaining unnecessary instances of that type. (My actual motivation is really to avoid a larger diff later, since introducing Worker-variant callbacks will start returning errors when you try to attach an `Internal`-variant callback.) However, changing `Callback_Internal` to `Callback_Nexus` changed the behavior of `TestDedupLinksFromCallbacks`. After scratching my head for a while, I add a doc comment to clarify exactly what the function does, and then updated the tests to be easier to read and understand. ## Why? The call to `dedupLinksFromCallbacks(...)` in the testcase did _not_ dedupe the links attached to `callbacks[0]` because it was the `commonpb.Callback_Internal` variant. (Relying on a quirk of the function only filtering callbacks from Nexus-variant callbacks.) I kept that behavior in, but added a couple more test scenarios to provide better coverage and clarify the expected behavior. ## How did you test it? - [x] built - [x] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) ## Potential risks None.
## What changed All three warnings in `ConvertNexusLinksToProtoLinks` interpolated the link type into the message, and two also embedded the link URL. Moved both into tags and kept the message static. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
## What changed The HSM Nexus executor logged outbound call failure logs were missing tags. ## Why `chasm/lib/nexusoperation` already logs exactly these fields via `invocationTraceContext.tags()`, --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
## What changed? Fixes a build error, seemingly introduced when multiple changes were merged automatically after approval. ## Why? Because a broken build stops the flow of spice. And the spice must flow. ## How did you test it? - [x] built - [x] run locally and tested manually - [ ] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) ## Potential risks None. But there might be other issues, I'm curious to see if there are any other issues flagged by CI/CD.
chaptersix
reviewed
Aug 24, 2026
chaptersix
reviewed
Aug 24, 2026
chaptersix
reviewed
Aug 24, 2026
chaptersix
approved these changes
Aug 26, 2026
davidporter-id-au
added a commit
that referenced
this pull request
Sep 3, 2026
## Summary We held back several fixes in the V1 schedule workflow and bundled them into a single atomic commit with the intention of avoiding having to perform multiple increments of the workflow versioning. The changes are broadly: Migration v1->v2 fixes + CGS version changes. ## Risk This touches V1 schedules workflow, and a mistake risks nondeterminitism. This is relatively high impact and has quite a bit of subtlety. This is therefore a commit we need to merge with care ## Testing and validation 1. Changes have been manually tested on individual PRs already 2. Changes have been manually run through CGS's validation suite (cc @liam-lowe) Our intention is to a) make the 163 release cut and b) continue to manually testing a few operational scenarios manually while this is being merged in. If we see problems we can hotfix this. Manually doing some scenario testing will take time so we intend to do this in parallel. Some of the scenarios we will test manually are: 1. Deploy and rollback to an earlier version (probably with LLM assist, but running locally) 2. Manually lifting the the version of a schedule via dynamic config (manually) ensuring there's no nondeterminitism risk 3. V2->v1 Rollback works as expected Some of the tests we've already run and manually validated(cc @liam-lowe) 1. Version floor works as expected 2. That this change fixes v1->v2 migration 3. That this change avoids the problematic history entries which broke a customer using the coinbase Ruby SDK ## LLM Summary This PR stacks a series of changes to the V1 (legacy) scheduler workflow: two migration-correctness fixes, and dynamic-config levers to safely roll the V1 workflow version forward/backward across a multi-cluster deployment without requiring continue-as-new. Summary of each PR in the stack, in order: ### #11462 — V1→V2 migration-eligibility fix and migrated-start ID (David Porter) Combines two previously separate fixes under one shared version bump (`v13`), avoiding two separate version-bump deploys for the same version number: - `RefreshBeforeMigrationCheck`: fixes a bug that was preventing V1→V2 migration from ever succeeding under default configuration. - `PreserveMigratedStartIDs`: preserves the request IDs workflows were originally started with, in case of a rollback. - Adds a guard against late migrations caused by a transient error bouncing a migration attempt and the schedule re-attempting it after waking again. - Follows the existing two-phase-rollout pattern: this PR only teaches the scheduler to *understand* v13 (gated behind `hasMinVersion(13)`); `CurrentTweakablePolicies.Version` itself stays at v12 pending a follow-up activation deploy. ### #11588 — Fix schedule action delay after refresh (Alex Stanfield) - `processWatcherResult` only recorded `DesiredTime` on the long-poll path; when a refresh instead discovered the prior action had completed, `DesiredTime` stayed unset, inflating the reported `ScheduleActionDelay` for back-to-back buffered actions. - Backdates `DesiredTime` on the refresh path too, gated behind a new version, `RefreshCompletionDesiredTime` (v14). - Handles two follow-on correctness gaps surfaced in review: `ALLOW_ALL` starts (never blocked by a running workflow, so shouldn't be backdated to an unrelated close time) and multiple tracked executions in one refresh pass (must only move the recorded close time forward, never backward). - Extracts the decision into a pure, unit-testable `shouldBackdateDesiredTime` function, and a shared `IgnoresRunningWorkflow` helper so `ProcessBuffer` and the new backdate logic can't drift apart on what "waits for a running workflow" means. ### #11827 — Sort BufferedStarts by due time on CHASM-to-V1 rollback (Alex Stanfield) - `CHASMToLegacyStartScheduleArgs` (the CHASM→V1 rollback conversion) appended trigger-derived `BufferedStarts` after the regular pending ones unconditionally, without sorting by due time. - V1's buffer-processing code assumes `BufferedStarts[0]` is always the earliest-due pending start — an invariant not guaranteed across a rollback, since manual triggers are built by iterating a Go map (randomized order) and appended regardless of their own due time. - Fix: sort the combined list by `ActualTime` after appending, mirroring the sort already applied to `RecentActions` a few lines above. ### #11831 — Re-evaluate V1 version ceiling per iteration (Alex Stanfield) - The base PR (#10817, by liam-lowe) added `worker.schedulerV1VersionCeiling` but only applied it on the first `tweakables` evaluation, so raising/removing a ceiling left an in-flight workflow stuck at the capped version until continue-as-new. - Re-reads the ceiling on every evaluation: `next version = max(recorded version, min(binary default, current ceiling))`. The recorded version stays monotonic — a newly-lowered ceiling never downgrades a version already recorded in the current run, but a raised/removed ceiling lets the workflow advance at its next wakeup instead of waiting for continue-as-new. - Recorded via the existing `MutableSideEffect`, so replay consumes history rather than re-evaluating live dynamic config. ### #11856 — Add worker.schedulerV1VersionOverride (Alex Stanfield) - #11831 makes the ceiling dynamic, but a ceiling can only restrict a binary default, never promote past it. Adds the namespace-level override needed to activate a newer version already supported by the binary. - `requested version = valid override, otherwise binary default`; `next version = max(recorded version, min(requested version, current ceiling))`. Default `-1` retains the binary default; values below the binary default or above `LatestSchedulerWorkflowVersion` are ignored. - The frontend uses the same override for initial schedule memo/list-info construction, so version-dependent metadata agrees with the first worker task. Base of the stack: #10817 (liam-lowe) introduced the original (static-per-run) `worker.schedulerV1VersionCeiling` dynamic config that #11831/#11856 build on. ## Why? To support safe, gradual rollout/rollback of V1 scheduler workflow version bumps (migration fixes, action-delay-after-refresh fix) in a cross-version multi-cluster topology, without requiring continue-as-new to pick up config changes. ## How did you test it? - [x] built - [x] covered by existing tests - [x] added new unit test(s) --------- Co-authored-by: liam-lowe <56076876+liam-lowe@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: alex.stanfield <13949480+chaptersix@users.noreply.github.com> Co-authored-by: Stephan Behnke <stephanos@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: michaely520 <michaely520@users.noreply.github.com> Co-authored-by: Feiyang Xie <feiyang3cat@outlook.com> Co-authored-by: Kannan <rkannan82@users.noreply.github.com> Co-authored-by: Fred Tzeng <41805201+fretz12@users.noreply.github.com> Co-authored-by: Lakshay <54310363+Lakshaymiddha@users.noreply.github.com> Co-authored-by: samm <sam.mathis@temporal.io> Co-authored-by: Quinn Klassen <klassenq@gmail.com> Co-authored-by: Will Duan <xinw.duan@gmail.com> Co-authored-by: Qian Chen <qyc5937@gmail.com> Co-authored-by: Prathyush PV <prathyush.pv@temporal.io> Co-authored-by: Sean Kane <sean.kane@temporal.io> Co-authored-by: mavemuri <74267563+mavemuri@users.noreply.github.com> Co-authored-by: Rodrigo Zhou <rodrigo.zhou@temporal.io> Co-authored-by: Brian VanLoo <brian.vanloo@gmail.com> Co-authored-by: akbala <akbala@gmail.com> Co-authored-by: Dan Davison <dan.davison@temporal.io> Co-authored-by: Chris Smith <chrsmith@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Combines #11134 and #11427 as two distinct fixes affecting V1 schedules and requiring a version bump. they're joined together. Shipping them separately would require two separate version-bump deploys for the "same" version number. This merges both behavioral changes under one shared v13:
RefreshBeforeMigrationCheck(from Fix: v1->V2 schedules migration guard & fix #11134): This fixes a problem that was preventing v1->v2 migration from ever succeeding under default configurationPreserveMigratedStartIDs(from Preserve IDs for migrated scheduler starts #11427): Try and keep the requestIDs from when workflows are started under when a rollback occurs.Following #11134's two-phase-rollout rationale, this PR only teaches the scheduler to understand v13 for safe replay/rollback: both fixes are gated behind
hasMinVersion(13), butCurrentTweakablePolicies.Versionstays atTriggerImmediatelyTimestamp(12). A follow-up deploy bumpsVersionto 13 to activate both fixes at once — a single activation instead of two.Details
service/worker/scheduler/workflow.go: addsRefreshBeforeMigrationCheckand
PreserveMigratedStartIDs, both= 13, with a shared doc comment; addsa
// TODOonCurrentTweakablePolicies.Versionpointing at the follow-upactivation deploy; ports both fixes' logic unchanged (gated on the
respective constant).
service/worker/scheduler/workflow_test.go: ports all three new tests fromthe two source PRs (
TestAutoMigrateReconcilesRunningWorkflowBeforeCheck,TestMigratedBufferedStartPreservesIdempotencyIDs,TestMigratedBufferedStartUsesLegacyIDsAtOldVersion) plus theTestStartRequestIdassertion.TestMigratedBufferedStartPreservesIdempotencyIDsnow force-sets
CurrentTweakablePolicies.Version(mirroring the other twoversion-forcing tests), since
Versionno longer defaults to 13 here.service/worker/scheduler/testdata/replay_migration_v1_to_v2.json.gzandtests/schedule_migration_v1_to_v2_callback_compat_test.go: brought inverbatim from Fix: v1->V2 schedules migration guard & fix #11134.
How did you test it?
go test -tags test_dep ./service/worker/scheduler/...passes, includingTestReplaysagainst the copied fixture and all three new/updated unittests.
go build/go vetpass for./service/worker/scheduler/...and./tests/....Potential risks
Moderately high risk as this is touching the Schedule V1 code. A problem with nondeterminism could affect schedules quite badly.