-
Notifications
You must be signed in to change notification settings - Fork 226
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
Races in dial queue tests #239
Comments
I can see this happening. Weirdly, it didn’t error locally on any execution, nor on 2 CIs. I guess tests execute in a different direction down under, just like the drains circling :P I’ll build on your latest PR and make all params adjustable via a struct. Also paves the way to make some form of this queue exportable to other areas of the system that might benefit. |
Sounds ideal. |
Actually you should just merge #241 now, and use a struct in another PR. |
This will stop the CI from failing so we can move on in the short term. |
@anacrolix sounds like a plan. Merged. |
Do you want to try a test for a scaling mute period of 0? As mentioned some of the tests fail with this, so I suspect a logic error somewhere. (There may be some timing assumptions, that will eventually cause problems in the wild.) |
With #248 I can no longer reproduce the other failures I found. |
@raulk There's races in the dial queue tests around the manipulation of the dial queue variables. I'm also able to trigger
panic: sync: negative WaitGroup
inTestDialQueueMutePeriodHonored
when I was messing around trying to fix them if the scaling mute period is set to 0. I think that might indicate some assumptions about timing.The text was updated successfully, but these errors were encountered: