REP-5329 Teach retryer to run +1 callback in parallel. #66
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
FGasper
force-pushed
the
REP-5329-simplify-retryer
branch
2 times, most recently
from
33d4342 to
948c454
Compare
tdq45gj
approved these changes
Dec 2, 2024
Collaborator
tdq45gj
left a comment
tdq45gj
left a comment
There was a problem hiding this comment.
LGTM % one small question
internal/verifier/compare.go
Outdated
| } | ||
|
|
||
| if cursor.Err() == nil { | ||
| state.NoteSuccess() |
Collaborator
There was a problem hiding this comment.
Looks like the retryer refreshes lastResetTime once the error group finishes. Is it redundant to call NoteSuccess before the callback function returns?
Since DDL events are forbidden, collection UUIDs are useless to migration-verifier. This changeset removes them from the retryer in anticipation of further changes.
This rewrites much of the retryer so that it can run multiple callbacks in parallel. The parallel retry logic is plugged into the verifier’s document comparison logic. This also synchronizes migration-verifier’s internal list of retryable errors.
FGasper
force-pushed
the
REP-5329-simplify-retryer
branch
from
50e5938 to
5a4b35d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We need retry logic around
findcursor iteration, but the retryer’s IterationSuccess() method won’t work properly with how we compare partitions document-by-document. If, for example, the source hangs but we call IterationSuccess() on every destination success, we’ll never time out the retries. But we need IterationSuccess() to work or else there are no retries for partitions that take a long time to read.This changeset fixes that by rewriting the retryer to take multiple callbacks. Each callback now takes a context that the retryer uses to stop the function whenever another thread has failed.
Since migration-verifier doesn’t handle DDL events, there’s no point to handling collection UUIDs. Thus, the initial commits in this PR resolve longstanding technical debt.
This also synchronizes migration-verifier’s internal list of transient errors with mongosync’s.