REP-5306 Migrate integration tests to use externally-provisioned clusters. #48
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
tdq45gj
approved these changes
Nov 20, 2024
| task := VerificationTask{QueryFilter: qfilter} | ||
|
|
||
| verifier := NewVerifier(VerifierSettings{}) | ||
| //verifier.SetStartClean(true) |
Collaborator
There was a problem hiding this comment.
Is this intended to be commented?
Collaborator
Author
There was a problem hiding this comment.
That’s the way it is upstream, curiously enough.
FGasper
added a commit
that referenced
this pull request
Nov 22, 2024
Previously to finalize its change stream the verifier would just TryNext() until no responses arrived. This works fine for unsharded clusters, but for sharded clusters it can potentially miss events. This changeset adopts more “tried-and-true” logic: grab a timestamp via `appendOplogNote`, then iterate the change stream until the stream’s resume token’s timestamp meets or exceeds that timestamp. As verification of that change, this changeset removes the test skips that REP-5306 (PR #48) imposed for sharded clusters. The timestamp-grabbing logic is unconditionally retryable, which requires an update to the retryer module to implement. This changeset refactors that module a bit to prevent further duplication. Additional changes: - go.sum is now part of the repo. - Go 1.22 is now a minimum requirement. - Tests no longer spew output when the worker delay is 0. The log message for that delay is also tweaked to show the delay. - Mention in code of the defunct “refetch” collection is removed. - The change stream no longer logs whenever a batch of events arrives. For busy migrations it would be excessive. - Verifier instances in tests now wait 10ms rather than 15s between generations, which makes a few tests much quicker. - A new CheckRunner improves the ergonomics of running CheckDriver in tests. - TestGenerationalRechecking used to verify that an insert after WritesOff() gets handled as a recheck. This doesn’t make a lot of sense since WritesOff() means, by definition, that no more events should happen on the server. Thus, this changeset removes that test, which was proving rather race-prone with the rest of these changes. - The tests now use majority read & write concern. - CI now parallelizes a bit more of its setup. - A new CheckRunner struct smooths over running Check from e2e tests. - Tests now use a 10ms verificationStatusCheckInterval rather than 15-minute.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
This project’s test suite has been provisioning its own clusters via custom logic. That logic can’t provision sharded clusters, though.
REP-5299 (PR #47) documents a fix for sharding. To test that ticket’s fix we need to run the test suite against sharded clusters. Thus, this changeset:
Additional notes/changes:
stableversion. There’s little reason to retain compatibility with prior Go releases.timestampTimerather thanclockTime.util.