REP-5329 Fix retry on document reads. #69
Conversation
FGasper
changed the title
internal/retry/retry.go
Outdated
| } | ||
|
|
||
| eg.Go(func() error { | ||
| if curFunc == nil { |
There was a problem hiding this comment.
Why do we check curFunc to be non-nil both twice?
There was a problem hiding this comment.
This 2nd check is probably redundant. I was just trying to suss out why I was seeing segfaults, and I figured I might as well leave the panics in.
| WithBefore(func() { | ||
| srcChannel, dstChannel, readSrcCallback, readDstCallback = verifier.getFetcherChannelsAndCallbacks(task) | ||
| }). |
There was a problem hiding this comment.
I'm trying to understand why do we need before func. Is it the same as calling the function at the start of the retried function except that the error from it won't be retried?
There was a problem hiding this comment.
The trick is that there are multiple retried funcs. Having a “before” func guarantees execution of one piece before all the others … which is crucial here because the callbacks & channels have to get recreated every time.
I’ll clarify this in docs.
2 participants
Commit 31cc703 was broken because it didn’t actually recreate the callbacks as needed to redo the reads.
(Unfortunately there’s little good way to test this directly, but mongosync’s CI testing caught it.)
This changeset fixes that by adding a callback to the retryer that runs at the start of each iteration.