Skip to content

test: TestRingSetAddrsContention changed to Benchmark #2316

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

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

monkey92t
Copy link
Collaborator

TestRingSetAddrsContention caused a lot of test errors, it should be changed to Benchmark.

See #2292

Signed-off-by: monkey92t <golang@88.com>
@monkey92t monkey92t merged commit 53cc4b4 into redis:master Dec 16, 2022
@monkey92t monkey92t deleted the fix_test branch December 16, 2022 15:35
@AlexanderYastrebov
Copy link
Contributor

I did not implement it as a benchmark originally as go runs benchmark multiple times with increasing N (golang/go#23398) and that was confusing.
Another benefit is that test ensures there is no regression so an alternative could be to lower the fail threshold below 10k to accommodate slugging build workers.

@monkey92t
Copy link
Collaborator Author

I did not implement it as a benchmark originally as go runs benchmark multiple times with increasing N (golang/go#23398) and that was confusing. Another benefit is that test ensures there is no regression so an alternative could be to lower the fail threshold below 10k to accommodate slugging build workers.

The number of pings depends on the performance of the system (network, kernel, redis-server), which leads to multiple build failures, which can confuse other developers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants