Add api at mutable state to track potential reapply events #7001
Conversation
| for _, event := range newRunHistory { | ||
| newRunMutableState.AddReapplyCandidateEvent(event) | ||
| } |
There was a problem hiding this comment.
This new run (continue as new) is created by applying events so the history builder in newRunMutableState does contain those events I think.
service/history/workflow/context.go
Outdated
| newWorkflowEventsSeq, | ||
| ); err != nil { | ||
| return err | ||
| if len(updateWorkflowEventsSeq) != 0 || len(newWorkflowEventsSeq) != 0 { |
There was a problem hiding this comment.
As commented above, I think newWorkflowEventsSeq is non-empty here.
We need to check both xxxWorkflowEventsSeq and xxxEventReapplyCandidate and pick the non-empty one if existing for the reapply.
| if err := c.updateWorkflowExecutionEventReapply( | ||
| ctx, | ||
| shardContext, | ||
| updateMode, | ||
| updateWorkflowEventsSeq, | ||
| eventsToReapply, | ||
| newWorkflowEventsSeq, |
There was a problem hiding this comment.
nit: add a comment re. why newWorkflowEventsSeq can be used directly.
service/history/workflow/context.go
Outdated
| // current workflow events will not participate in the events reapplication | ||
| ); err != nil { | ||
| return err | ||
| if len(resetWorkflowEventsSeq) != 0 || len(newWorkflowEventsSeq) != 0 { |
There was a problem hiding this comment.
we should use similar logic as UpdateWorkflowExecutionWithNew?
There was a problem hiding this comment.
yes! updated as UpdateWorkflowExecutionWithNew
| if err != nil { | ||
| return common.EmptyEventTaskID, nil, err | ||
| } | ||
| eventsToReapply = append(eventsToReapply, events...) |
There was a problem hiding this comment.
nit: is it possible to add those events directly to ms here instead of buffering entire history in memory, return and then filter?
There was a problem hiding this comment.
restructure the functions to add events directly to ms
What changed?
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?