Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle system time move backward case in timer task processing #7030

Merged
merged 3 commits into from
Jan 16, 2025

Conversation

yycptt
Copy link
Member

@yycptt yycptt commented Dec 21, 2024

What changed?

  • Handle system time move backward case in timer task processing by using max(now(), task visibility timestamp)

Why?

  • Monotonic time is used in time.Time comparison only when both operands have monotonic time value. In our case, the timestamp stored/derived in/from mutable state doesn't have monotonic, thus the comparison logic will use wall clock time to decide if a timer task should be processed. If wall clock move backwards (after we verifies that now() > task visibility timestamp when submitting the task to the task scheduler), the timer task will be dropped and cause workflow to stuck.

How did you test it?

  • Unit tests

Potential risks

  • In worst case, we will execute a timer task logic earlier than expected.

Documentation

Is hotfix candidate?

  • Could be. But the bug should be very rare as the use millisecond precision when doing time comparison in timer task processing.

@yycptt yycptt requested a review from bergundy December 21, 2024 02:22
@yycptt yycptt requested a review from a team as a code owner December 21, 2024 02:22
@yycptt yycptt force-pushed the fix-queues-is-time-expired branch from fb5f5fd to c3f87c1 Compare December 21, 2024 02:24
@yycptt yycptt requested a review from bergundy January 6, 2025 18:10
@yycptt yycptt force-pushed the fix-queues-is-time-expired branch from 4e5fb64 to 84dcd5e Compare January 16, 2025 01:36
@yycptt yycptt merged commit a5b3c01 into temporalio:main Jan 16, 2025
49 checks passed
@yycptt yycptt deleted the fix-queues-is-time-expired branch January 16, 2025 17:41
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.

2 participants