[CHASM] timer queue pure task processing logic #7702
Conversation
| // We haven't done any work, return without committing. | ||
| if processedTimers == 0 { | ||
| return nil, nil | ||
| } | ||
|
|
||
| if t.config.EnableUpdateWorkflowModeIgnoreCurrent() { | ||
| return nil, wfContext.UpdateWorkflowExecutionAsPassive(ctx, t.shardContext) | ||
| } | ||
|
|
||
| // TODO: remove following code once EnableUpdateWorkflowModeIgnoreCurrent config is deprecated. | ||
| if mutableState.GetExecutionState().State == enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED { | ||
| // Can't use UpdateWorkflowExecutionAsPassive since it updates the current run, | ||
| // and we are operating on a closed workflow. | ||
| return nil, wfContext.SubmitClosedWorkflowSnapshot( | ||
| ctx, | ||
| t.shardContext, | ||
| historyi.TransactionPolicyPassive, | ||
| ) | ||
| } | ||
| return nil, wfContext.UpdateWorkflowExecutionAsPassive(ctx, t.shardContext) |
There was a problem hiding this comment.
standby executor doesn't need this part I think.
|
|
||
| tree := ms.ChasmTree() | ||
| if tree == nil { | ||
| return 0, consts.ErrStaleReference |
There was a problem hiding this comment.
nit: shall we return an internal error here? I think this is not expected?
| return 0, fmt.Errorf("failed type assertion for ChasmTree") | ||
| } | ||
|
|
||
| pureTasks, err := tree.GetPureTasks(t.Now()) |
There was a problem hiding this comment.
commented in the previous PR, t.Now() is not good enough. Check queues.IsTimeExpired().
There was a problem hiding this comment.
Fixed. I address skew between DB and physical task queue within the timer task queue executor, and then also truncate times against the reference time within the CHASM tree.
| // 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.
What changed?
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?