-
Notifications
You must be signed in to change notification settings - Fork 635
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
fix: flaky prefetcher tests #8255
Conversation
Prefetcher tests have become flaky in CI lately, failing with `"unexpected number of prefetched values"`. I was not able to reproduce it locally. But there is a timeout that is at least one source for flakyness. This commit removes this source by properly waiting on the background threads to finish instead of relying on a timeout. Hopefully resolves near#8252. (Will have to reopen if not.)
// causes all background threads to stop after they finish the current | ||
// work. It will even join them and wait until all threads are done. | ||
// | ||
// Because threads are joined, there is also a possibility this will |
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.
nit: is it worth clarifying what this
refers to here? Maybe replace it with drop(tries)
?
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.
Yes makes sense, updated now.
This needs a code owner approval before I can merge it. @Longarithm or @akhi3030 maybe? |
Prefetcher tests have become flaky in CI lately, failing with
"unexpected number of prefetched values"
.I was not able to reproduce it locally. But there is a timeout that
is at least one source for flakyness. This commit removes this
source by properly waiting on the background threads to finish
instead of relying on a timeout.
Hopefully resolves #8252. (Will have to reopen if not.)