use transition history if enabled when long poll history #7185
Conversation
| if len(transitionHistory) != 0 { | ||
| if versionedTransition != nil && workflow.TransitionHistoryStalenessCheck(transitionHistory, versionedTransition) != nil { | ||
| return false | ||
| } | ||
| return true |
There was a problem hiding this comment.
nit
| if len(transitionHistory) != 0 { | |
| if versionedTransition != nil && workflow.TransitionHistoryStalenessCheck(transitionHistory, versionedTransition) != nil { | |
| return false | |
| } | |
| return true | |
| if len(transitionHistory) != 0 && versionedTransition != nil { | |
| return workflow.TransitionHistoryStalenessCheck(transitionHistory, versionedTransition) == nil | |
| } |
| transitionHistory := response.GetTransitionHistory() | ||
| if len(transitionHistory) != 0 { | ||
| if request.VersionedTransition != nil && workflow.TransitionHistoryStalenessCheck(transitionHistory, request.VersionedTransition) != nil { | ||
| logger.Warn(fmt.Sprintf("Request versioned transition and transition history prior to polling the mutable state. Request: %v, current: %v", |
There was a problem hiding this comment.
| logger.Warn(fmt.Sprintf("Request versioned transition and transition history prior to polling the mutable state. Request: %v, current: %v", | |
| logger.Warn(fmt.Sprintf("Request versioned transition and transition history don't match prior to polling the mutable state. Request: %v, current: %v", |
| tag.WorkflowRunID(workflowKey.GetRunID())) | ||
| return nil, serviceerrors.NewCurrentBranchChanged(response.CurrentBranchToken, request.CurrentBranchToken) | ||
| } | ||
| } else if !versionhistory.ContainsVersionHistoryItem(currentVersionHistory, request.VersionHistoryItem) { |
There was a problem hiding this comment.
hmm shall we fallback to version history check when either transitionHistory or request.VersionedTransition is empty?
Looks right now, if transitionHistory is no empty but request.VersionedTransition is empty, we will just skip the validation priori to polling mutable state.
There was a problem hiding this comment.
I have the same question on if this is a valid case on transitionHistory is no empty but request.VersionedTransition is empty
There was a problem hiding this comment.
it will happen when we start to rollout state based replication so we have to handle it. I think we can just validate version history in that case.
| tag.WorkflowRunID(workflowKey.GetRunID())) | ||
| return nil, serviceerrors.NewCurrentBranchChanged(response.CurrentBranchToken, request.CurrentBranchToken) | ||
| } | ||
| } else if !versionhistory.ContainsVersionHistoryItem(eventVersionHistory, request.VersionHistoryItem) { |
| var lastVersionedTransition *persistencespb.VersionedTransition | ||
| transitionHistory := response.GetTransitionHistory() | ||
| if transitionHistory != nil { | ||
| lastVersionedTransition = transitionHistory[len(transitionHistory)-1] | ||
| } |
There was a problem hiding this comment.
nit: make this a util function as well for getting the last versioned transition? I think I have seen this logic in couple of places.
What changed?
use transition history if enabled when long poll history
Why?
For state-based replication, we do allow event version version to go backwards as long as the overall versioned transition is keep increasing. So we need to change token to use versioned transition.
How did you test it?
unit test.
Potential risks
Documentation
Is hotfix candidate?