Immediate replication task hydration after successful transaction #4980
Conversation
|
|
||
| func (ms immediateMutableState) IsWorkflowExecutionRunning() bool { | ||
| // Immediate mutable state is always running | ||
| return true |
There was a problem hiding this comment.
What is the impact if the workflow get closed immediately before replication?
There was a problem hiding this comment.
After some consideration I've update this.
This is used as early return when hydrating sync activity tasks. I think this check is somewhat redundant, since there will be no pending activity left after completion, and replication task will not be hydrated either way.
But for now let's keep it here for consistency with regular hydration from loaded mutable state. I may remove this entirely from both immediate and regular hydration later.
| @@ -368,12 +369,12 @@ func (c *contextImpl) CreateWorkflowExecution( | |||
| resp, err := c.createWorkflowExecutionWithRetry(ctx, createRequest) | |||
| if err != nil { | |||
| if c.isPersistenceTimeoutError(err) { | |||
| c.notifyTasksFromWorkflowSnapshot(newWorkflow, true) | |||
| c.notifyTasksFromWorkflowSnapshot(newWorkflow, events.PersistedBlobs{persistedHistory}, true) | |||
There was a problem hiding this comment.
nit, optional: to put more stress on the fact that notifyTasksFromWorkflowSnapshot must be called always. This also makes true/false self explanatory
| c.notifyTasksFromWorkflowSnapshot(newWorkflow, events.PersistedBlobs{persistedHistory}, true) | |
| resp, err := c.createWorkflowExecutionWithRetry(ctx, createRequest) | |
| c.notifyTasksFromWorkflowSnapshot(newWorkflow, events.PersistedBlobs{persistedHistory}, c.isPersistenceTimeoutError(err)) | |
| if err != nil { | |
| return err | |
| } |
There was a problem hiding this comment.
Notification are not call when there is an error but it is not isPersistenceTimeoutError.
This can probably be simplified, but I will leave as is for now.
There was a problem hiding this comment.
yes, missed that return. thanks
service/history/historyEngine.go
Outdated
| versionHistories, | ||
| activities, | ||
| history.Find(info.BranchToken, info.FirstEventID), | ||
| history.Find(info.NewRunBranchToken, 1), |
There was a problem hiding this comment.
will this always be 1?
There was a problem hiding this comment.
Changed magic number 1 to a constant common.FirstEventID used elsewhere as well.
This is always 1 at least in the current implementation: https://github.com/uber/cadence/blob/master/service/history/replication/task_hydrator.go#L227
What changed?
NewImmediateTaskHydratorwith corresponding inner bits for providing readily available data for replication message hydration.Why?
After successful transaction we already have all necessary bits available for replication message hydration. We can take advantage of that - preemptively prepare replication messages and put them into cache, which will later save additional database calls (
readhistorybranchandgetworkflowexecution).This should reduce overall load on database and also reduce replication latency.
How did you test it?
Potential risks
Release notes
Documentation Changes