keep a reasonable history events order when time skipping happens#10912
Open
feiyang3cat wants to merge 3 commits into
Open
keep a reasonable history events order when time skipping happens#10912feiyang3cat wants to merge 3 commits into
feiyang3cat wants to merge 3 commits into
Conversation
8469444 to
6a2e8b0
Compare
6a2e8b0 to
0ac2732
Compare
c1f77ac to
895243e
Compare
feiyang3cat
commented
Jul 5, 2026
| case // Buffer this event to ensure: | ||
| // 1) The transition is emitted after WORKFLOW_EXECUTION_OPTIONS_UPDATED in the | ||
| // same transaction, so history reflects when time skipping actually becomes enabled. | ||
| // 2) The transition is not inserted between workflow task events when the |
Contributor
Author
There was a problem hiding this comment.
I actually think both are trivial OR maybe 2) is a serious problem for some history reasons?
…saction A time-skipping transition must be the last event of the transaction that produces it: it is a server-generated state change applied synchronously (it bumps AccumulatedSkippedDuration and drives timer-task regeneration in the same close-transaction), so it must order after every other event in that transaction. WORKFLOW_EXECUTION_OPTIONS_UPDATED is a buffered event type while the transition previously was not. When an options update re-enables (or enables) time skipping on an idle workflow with a pending timer, the close-transaction emits a transition in the same transaction as the buffered OPTIONS_UPDATED. The non-buffered transition was allocated an event ID immediately, before the buffered OPTIONS_UPDATED was flushed in closeTransactionPrepareEvents, inverting their order and producing non-monotonic event timestamps. Make the transition a buffered event instead. Buffered events drain at the transaction's end-of-close flush (Finish -> FlushBufferToCurrentBatch), appended after every non-buffered event already in the batch and in insertion order among themselves; reorderBuffer only moves activity/child/nexus completion events to the end, and those cannot exist on a skippable (idle) workflow. So the transition, added last, flushes last — OPTIONS_UPDATED then transition — with monotonic IDs and timestamps, and no explicit FlushBufferedEvents call injected into time-skipping logic. Buffering also fixes the fast-forward corner (executeTimeSkippingTimerTask firing while a workflow task is started): the non-buffered transition used to be wedged inside the open WorkflowTaskStarted->WorkflowTaskCompleted window; as a buffered event it instead follows standard semantics (the in-flight WT is failed with UnhandledCommand and retried, so the worker processes the transition). The event is never dropped: it only lingers in the buffer while a WT is started, and every workflow-finishing path (worker complete via UnhandledCommand, terminate/timeout via failWorkflowTask) flushes buffered events before writing the finish event. Add TestTransitionEventIsLastInTransaction to TimeSkippingTestSuite guarding the invariant.
895243e to
6891f33
Compare
6891f33 to
b985926
Compare
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?
change the time-skipping-transitioned event to be a buffered event
Why?
Currently, the time-skipping-transitioned event is not buffered, while the update-workflow-options event is. This inconsistency leads to two minor (non-critical) issues that are still worth addressing:
Buffering the time-skipping-transitioned event resolves both issues while preserving the existing time-skipping behavior and logic.
How did you test it?