Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

schedule update should start from previous action instead of current time #4866

Closed
dnr opened this issue Sep 12, 2023 · 0 comments · Fixed by #5381
Closed

schedule update should start from previous action instead of current time #4866

dnr opened this issue Sep 12, 2023 · 0 comments · Fixed by #5381
Labels
potential-bug schedules

Comments

@dnr
Copy link
Member

dnr commented Sep 12, 2023

Actual Behavior

Suppose you have a 5-minute schedule with 1 minute jitter. The next nominal time is 12:20:00, but with jitter it's 12:20:33. Now at 12:20:10 you do an update to the exact same spec. It throws out the previous times and calculates the next times from there. The next nominal time is 12:25:00 now (we're past 12:20:00) and then adds jitter to that. So the run at 12:20:33 is lost.

Expected Behavior

If there is a matching time including jitter after the update time, it should still run, even if its nominal time was before the update time.

Possible Fix

For interval specs, it could just look back one interval, but with calendar specs we can't (easily) look backwards. One idea is to start from the previous action time and start calculating forwards, discarding any times where the actual time (including jitter) is in the past (of the update time). Or better, start from max(create time, last update time, previous action time). We use that same construction as the "retention base" for idle schedules so that makes some sense.
@dnr dnr mentioned this issue Feb 1, 2024
Merged
@dnr dnr closed this as completed in #5381 Mar 2, 2024
dnr added a commit that referenced this issue Mar 2, 2024
@dnr
Recalculate schedule times from previous action on update (#5381)
b51b899 
## What changed?
After an update, the schedule workflow goes back to the previous
action/update and recomputes next times from there, instead of from the
current time.

## Why?
Fixes #4866. Fixes the situation where an update happens in between the
nominal time and the jittered time of an action.

## How did you test it?
new unit test
stephanos pushed a commit to stephanos/temporal that referenced this issue Mar 21, 2024
@dnr @stephanos
Recalculate schedule times from previous action on update (temporalio…
1f8ced4 
…#5381)

## What changed?
After an update, the schedule workflow goes back to the previous
action/update and recomputes next times from there, instead of from the
current time.

## Why?
Fixes temporalio#4866. Fixes the situation where an update happens in between the
nominal time and the jittered time of an action.

## How did you test it?
new unit test
yycptt pushed a commit that referenced this issue Apr 27, 2024
@dnr @yycptt
Recalculate schedule times from previous action on update (#5381)
9701ef0 
## What changed?
After an update, the schedule workflow goes back to the previous
action/update and recomputes next times from there, instead of from the
current time.

## Why?
Fixes #4866. Fixes the situation where an update happens in between the
nominal time and the jittered time of an action.

## How did you test it?
new unit test
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
potential-bug schedules
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Recalculate schedule times from previous action on update dnr/temporal
2 participants
@dnr @yiminc