[CHASM] timer queue pure task processing logic #7702
Merged
Conversation
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
yycptt
reviewed
May 2, 2025
yycptt
reviewed
May 2, 2025
| // If this line of code is reached, the task's Validate() function returned no error, which indicates | ||
| // that it is still expected to run. Return ErrTaskRetry to wait the machine to transition on the active | ||
| // cluster. | ||
| return consts.ErrTaskRetry |
There was a problem hiding this comment.
I am slightly concerned that the verification may never completes if new valid tasks are keep getting generated.
We probably want to add VT to the physical pure tasks and only execute/verify those tasks generated <= the VT recorded in the physical task. Can you help create a follow up task on this? Not high priority and we can address it later.
There was a problem hiding this comment.
Created OSS-4273 for follow-up.
yycptt
approved these changes
May 5, 2025
Comment on lines
139
to
143
| if err != nil && errors.Is(err, consts.ErrTaskRetry) { | ||
| return &struct{}{}, nil | ||
| } | ||
|
|
||
| return nil, nil |
There was a problem hiding this comment.
we should still return err if it's not nil and also not ErrTaskRetry?
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?
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?