Skip to content
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

Extend timeout for mpsc buffer test #411

Merged
merged 9 commits into from
Oct 14, 2023
Merged

Conversation

bitfaster
Copy link
Owner

@bitfaster bitfaster commented Oct 14, 2023

It seems like the core problem is that xunit will start to heavily parallelize test execution, and the github action runs on a 2 core machine. Previous bug fix PR added two more soak tests, which led to more severe thread starvation. Increasing the timeout makes this much more stable. Next step will be to break out soak tests and not run them in parallel.

  • Removed use of [Fact(Timeout=x] because it is documented as undefined behavior when tests are run in parallel
  • Added Task.TimeoutAfter extension method. This is likely not better, because the real issue is xunit scheduling too many threads.
  • Cleaned up using statement/redundant cast in MpscBoundedBuffer

@bitfaster bitfaster changed the title spin Fix unstable mpsc buffer test Oct 14, 2023
@coveralls
Copy link

coveralls commented Oct 14, 2023

Coverage Status

coverage: 97.211%. remained the same when pulling e4a68f8 on users/alexpeck/mpscteststab2 into f6fec6c on main.

Alex Peck added 2 commits October 14, 2023 09:24
@bitfaster
Copy link
Owner Author

It could be that this is caused by the timeout attribute:

image

@bitfaster
Copy link
Owner Author

TimeoutAfter taken from here.

@bitfaster bitfaster changed the title Fix unstable mpsc buffer test Extend timeout for mpsc buffer test Oct 14, 2023
@bitfaster bitfaster merged commit 8a5aba4 into main Oct 14, 2023
14 checks passed
@bitfaster bitfaster deleted the users/alexpeck/mpscteststab2 branch October 14, 2023 20:12
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