Repository navigation
Recreate only the activity timers whose deadline actually moved (#11565) - #11613
Merged
stpierre merged 1 commit intoAug 18, 2026
Merged
Conversation
## Problem
State-based replication blanket-cleared an activity's `TimerTaskStatus`
whenever a replicated update changed the attempt, version or stamp:
```go
if current == nil || ms.ShouldResetActivityTimerTaskMask(current, incoming) {
incoming.TimerTaskStatus = TimerTaskStatusNone
}
```
Every cleared bit lets the next task refresh regenerate a timer task.
`CreateNextActivityTimer` only ever creates the *earliest* timer in the
sequence and bails if that one already exists — so the cost is at most
one task per refresh, but which timer it is depends on the activity's
state.
**Between retry attempts, that earliest timer is schedule-to-close.**
Clearing the started state removes start-to-close and heartbeat from the
sequence, and when the user doesn't set `ScheduleToStartTimeout` it is
[normalized](https://github.com/temporalio/temporal/blob/main/chasm/lib/activity/validator.go)
to the schedule-to-close duration — anchored on `ScheduledTime`, which
advances every retry, so it is always *later* than schedule-to-close
anchored on the fixed `FirstScheduledTime`.
So the passive side regenerates schedule-to-close once per replicated
retry, and because that deadline never moves, **every duplicate carries
an identical `VisibilityTimestamp`**. A concrete case: a 31-day activity
with a flat 10s backoff reached attempt 242,037 — on the order of 242k
tasks piled on a single instant, essentially all of which re-derive the
sequence, find nothing expired, and return `errNoTimerFired`.
The duplicates are dropped at execution, so **there is no correctness
impact**. It is a timer-queue hotspot.
## Approach
Compare the four timer deadlines before and after the update, and keep a
bit only when its deadline is unchanged:
```go
return current.TimerTaskStatus &^
getActivityTimerDeadlines(current).changedMask(getActivityTimerDeadlines(incoming))
```
**Why deadlines are the right and sufficient signal.** An
`ActivityTimeoutTask` is a wake-up at a point in time:
`processSingleActivityTimeoutTask` re-derives the whole sequence from
current mutable state and fires whatever expired, explicitly without
consulting the task's attempt or stamp (*"Note: we don't need to check
activity Stamps"*). A pending task whose deadline did not move is still
correct no matter what else changed; one whose deadline moved is useless
no matter what stayed put.
Attempt and stamp are only ever proxies for "some deadline probably
moved", so they are deliberately not consulted. A retry needs no special
case: clearing the started state removes start-to-close and heartbeat
outright, and schedule-to-start's anchor advances — while
schedule-to-close survives on its untouched `FirstScheduledTime`.
This also fixes a case **the active side still has**: for mutable state
predating `FirstScheduledTime`, schedule-to-close falls back to the
`ScheduledTime` anchor, which a retry does move. Keying off the attempt
preserves that bit and leaves a task pointing at the old, earlier
instant; comparing deadlines clears it.
A cross-cluster version change still resets everything — that is about
task provenance, not deadlines.
The four deadline getters are split into free functions with one-line
method wrappers so the comparison reuses `timerSequenceImpl`'s existing
math rather than duplicating it.
## Scope
Limited to **state-based replication**. The event-based `SyncActivity`
path in `ndc/activity_state_replicator.go` keeps its existing blanket
reset and is untouched — it passes a synthetic `ActivityInfo` carrying
only version and attempt, which cannot support a deadline comparison,
and it is the older path.
## Tests
9 cases. The ones carrying the change:
- `Retry_KeepsOnlyScheduleToClose` — a real retry transition (attempt
bumped, started state cleared, schedule time advanced); start-to-close
and heartbeat bits drop, schedule-to-close survives.
- `Retry_LegacyAnchor_ClearsScheduleToClose` — same retry with
`FirstScheduledTime` nil; the moved deadline is detected and the bit
clears.
- `AttemptChangedWithoutDeadlineMove_KeepsMask` — attempt alone decides
nothing.
- `ClearsOnlyMovedDeadlines` / `TimerDisappears` — shorten or remove
`HeartbeatTimeout`; only that bit drops.
- `UnrelatedOptionChanged_KeepsMask` — stamp bumped, no deadline
affected, mask fully preserved (the old code wiped it).
`go build ./service/...` and `go vet` clean;
`./service/history/workflow/... ./service/history/ndc/...` pass across
repeated runs.
## Two pre-existing flakes (not from this change)
Both reproduce identically on unmodified `main` and neither touches
`ActivityInfo.TimerTaskStatus`. Skipped in the runs above and left alone
here:
- `TestTaskRefresherSuite/TestRefreshSubStateMachineTasks` — **7/12
failures on unmodified main.** HSM timer infos get deadlines differing
by a single nanosecond, so grouping depends on map iteration order.
Worth a separate fix; it currently fails more often than it passes.
-
`TestMutableStateSuite/*/TestApplyWorkflowExecutionOptionsUpdatedEvent_TimeSkippingConfig`
— 1/30 on clean tree, 1/30 with this change. Asserts a renewed
`TargetTime` differs from the initial one; fails when both land on the
same clock reading.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
yiminc
approved these changes
Aug 18, 2026
tusharroy25
approved these changes
Aug 18, 2026
stpierre
approved these changes
Aug 18, 2026
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.
What changed?
Recreate only the activity timers whose deadline actually moved to avoid duplicated schedule to close timer task.
Original PRs
#11565
Why is this necessary as a patch?
To reduce the risk of hot shard
What testing or retesting is appropriate for this patch after merge?
We will run bench failover test against test cluster.