Skip time in UpdateWorkflowSuite [WiP]#9145
Skip time in UpdateWorkflowSuite [WiP]#9145stephanos wants to merge 1 commit intotemporalio:mainfrom
Conversation
stephanos
force-pushed
the
manual-faults3
branch
3 times, most recently
from
12c3393 to
d58fd6d
Compare
stephanos
changed the title
stephanos
force-pushed
the
manual-faults3
branch
5 times, most recently
from
8b69955 to
59d2130
Compare
stephanos
force-pushed
the
manual-faults3
branch
from
59d2130 to
de785a0
Compare
|
|
||
| // All grpc handlers should be cancelled now. Give them a little time to return. | ||
| t := time.AfterFunc(2*time.Second, func() { | ||
| t := s.timeSource.AfterFunc(2*time.Second, func() { |
There was a problem hiding this comment.
This sort of thing is my concern with this idea: I would argue this one really should be 2 seconds of wall-clock time and not virtual time. It's in shutdown so doesn't really matter, but maybe others do and it's hard to audit them all (but we can try?)
There was a problem hiding this comment.
Yeah, I was just curious to see what happens if I try :)
| batch := partitions[i:min(len(partitions), i+batchSize)] | ||
| wait := backoff.Jitter(delay, 0.1) | ||
| time.AfterFunc(wait, func() { | ||
| e.timeSource.AfterFunc(wait, func() { |
There was a problem hiding this comment.
This one also (arguably) requires real time.. virtual time isn't threaded through membership, so ringpop propagation delays will not respect virtual time. Afaik we don't have any functional tests that exercise this functionality though.
In principle we could thread fake time into ringpop, it does support one (different interface though, we'd need an adapter)
There was a problem hiding this comment.
That's interesting about ringpop support a fake time. But generally yes, this shouldn't be converted. My instructions were to replace everything because I preferred converting too many instead of too few to verify my test case.
2 participants
What changed?
Replaced all "real time"
timepackage uses withclock.TimeSourceacross History and Matching.Why?
To replace
time.Sleeps withs.AdvanceinUpdateWorkflowSuiteto skip time; one for History, one for Matching.Before: TBD
After: TBD
How did you test it?
Tests were run 10x locally without issues.
Potential risks
Any change is risky. Identify all risks you are aware of. If none, remove this section.