8000 kv/concurrency: bump goroutine stall delay by nvanbenschoten · Pull Request #57355 · cockroachdb/cockroach · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

kv/concurrency: bump goroutine stall delay #57355

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

Conversation

nvanbenschoten
Copy link
Contributor

Fixes #57273.

This commit bumps the goroutine stall delay in the TestConcurrencyManagerBasic
framework from 1ms to 5ms. In #57273, we saw a flake due to a goroutine being
classified as stalled when it was actually waiting for a multi-goroutine
chain of events to fire (request exits lockTable, signals lockTable wait-queue,
which is noticed by watchForNotifications, which cancels the pushers context).
This chain of events plus the 1ms wait in the framework led to a flake under
stress roughly every 16k iterations. With this delay bumped to 3ms, I have never
reproduced the flake after over 2M iterations. This PR bumps the delay all the
way up to 5ms to be extra sure. This increases the runtime of all subtests
combined from 1.6s to 3.9s.

Fixes cockroachdb#57273.

This commit bumps the goroutine stall delay in the TestConcurrencyManagerBasic
framework from 1ms to 5ms. In cockroachdb#57273, we saw a flake due to a goroutine being
classified as stalled when it was actually waiting for a multi-goroutine
chain of events to fire (request exits lockTable, signals lockTable wait-queue,
which is noticed by watchForNotifications, which cancels the pushers context).
This chain of events plus the 1ms wait in the framework led to a flake under
stress roughly every 16k iterations. With this delay bumped to 3ms, I have never
reproduced the flake after over 2M iterations. This PR bumps the delay all the
way up to 5ms to be extra sure. This increases the runtime of all subtests
combined from 1.6s to 3.9s.
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nvanbenschoten
Copy link
Contributor Author

TFTR!

bors r+

@craig
Copy link
Contributor
craig bot commented Dec 2, 2020

Build succeeded:

@craig craig bot merged commit 5af065e into cockroachdb:master Dec 2, 2020
@nvanbenschoten nvanbenschoten deleted the nvanbenschoten/conManTestDelay branch December 10, 2020 18:45
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.

kv/kvserver/concurrency: TestConcurrencyManagerBasic failed
3 participants
0