-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: add negative test to detect replication failures #1445
Merged
Conversation
This file contains 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
Previously, our integration tests were written assuming consistent reads. To avoid false positives caused by stale reads in low-latency environments, we enforced consistent reads by setting the consistent read header. While this reduced noise from false positives, it also meant we lacked test coverage for replica reads and replication behavior. To address this gap, we've added a negative test that detects replication failures by verifying that no stale reads occur after the expected replication delay. The new test suite `replica-reads` implements a test that runs 10 trials in parallel. Each trial performs the following steps: - Set: Writes a random key-value pair to the cache using `cacheClientWithBalancedReadConcern`. - Wait: 1 second for reads to propagate - Get: Reads the value for the key. - Assert: Confirms that the retrieved value matches the value set earlier. Trials are started with slight delays between them to distribute load (delayBetweenTrials and a small random offset).
anitarua
reviewed
Oct 11, 2024
anitarua
previously approved these changes
Oct 12, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
krispraws
previously approved these changes
Oct 14, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, left a question for clarification.
Previously we had a separate function in the inner loop of promise creation. Instead there is a single trial function that is parameterized by the trial number. We refactor the test to pull out the trial function and build the promise in the loop.
krispraws
approved these changes
Oct 14, 2024
anitarua
approved these changes
Oct 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously, our integration tests were written assuming consistent
reads. To avoid false positives caused by stale reads in low-latency
environments, we enforced consistent reads by setting the consistent
read header. While this reduced noise from false positives, it also
meant we lacked test coverage for replica reads and replication behavior.
To address this gap, we've added a negative test that detects
replication failures by verifying that no stale reads occur after the
expected replication delay.
The new test suite
replica-reads
implements a test that runs 10trials in parallel. Each trial performs the following steps:
cacheClientWithBalancedReadConcern
.earlier.
Trials are started with slight delays between them to distribute
load (delayBetweenTrials and a small random offset).