Workflow.getVersion() could cause NonDeterminsticError with multithreading + timer #1430
Description
Hey Temporal team,
We found that Workflow.getVersion() is letting workflow thread to yield, which causes NonDeterminsticError because the thread execution ordering is changed — this has made it very hard for us to use this getVersion().
Imagine we have two threads:
ThreadA:
{
executeActivity(A)
}
ThreadB
{
Workflow.sleep()
}
And thread A is invoked first.
And we got a JSON history for replay test. The history looks like this:
WF start
WF task scheduled
WF task started
WF task completed
activity task scheduled
timer scheduled
Then we are changing thread A to :
{
if( Workflow.getVersion() >= X ){
executeActivity(B)
}
executeActivity(A)
}
Then the replay test gets broken.
Because the thread A will yield at Workflow.getVersion() and thread B will then try to schedule before the activity A.
In Java SDK, it’s enforcing the eventId to be exactly matched with the history. So the replay test would run into NDE error.
Some thoughts to improve:
I think GoSDK is only forcing within a workflow task completed batch commands. This may be more reasonable because I don’t think the order really matter within a workflow task results.
Or if we could do special case in Workflow.getVersion() to let it not yield the current thread…
To reproduce
public void anyWorklfowMethod(){
Async.procedure( ()->{
Workflow.sleep(1s)
}
)
activityStub.hello()
}
Got a history for replay
Then change the code to:
public void anyWorklfowMethod(){
Async.procedure( ()->{
Workflow.sleep(1s)
}
)
Workflow.getVersion("changeId, Workflow.DefaultVersion, 1)
activityStub.hello()
}
And this should break the replay test