Add ApplyMutation and ApplySnapshot methods for mutable state #6517
Conversation
proto/internal/temporal/server/api/replication/v1/message.proto
Outdated
Show resolved
Hide resolved
| val := reflect.ValueOf(key) | ||
| if val.Kind() == reflect.String { |
|
|
||
| ms.ClearStickyTaskQueue() | ||
|
|
||
| if ms.executionInfo.WorkflowTaskType == enumsspb.WORKFLOW_TASK_TYPE_SPECULATIVE { |
There was a problem hiding this comment.
should be checking incoming WorkflowTaskType.
There was a problem hiding this comment.
nit: actually we can sync all WorkflowTask* fields and invoke deleteWorkflowTask() if the workflowTaskType is Speculative after sync?
| return err | ||
| } | ||
|
|
||
| return currentNode.Sync(incomingNode) |
There was a problem hiding this comment.
This internally performs a task refresh for the synced node. If we go with this approach (which I am fine with), caller needs to clear all generated tasks (can be done with ms.PopTasks()) before doing a task refresh. cc @xwduan
| ¤t.TaskGenerationShardClockTimestamp, | ||
| ¤t.UpdateInfos, | ||
| } | ||
| err := common.MergeProtoExcludingFields(current, incoming, doNotSyncFields...) |
There was a problem hiding this comment.
nit: I am a little bit concerned with the reflection usage here TBH since this will be a quite hot code path that got invoked for every replication task. When you have time can you do a benchtest on this util function to see if it will become a bottleneck? Thanks.
There was a problem hiding this comment.
Plz add more tests before landing.
What changed?
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?