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

Test injection_queue_depth_multi_thread is flaky #6847

Open
Darksonn opened this issue Sep 14, 2024 · 3 comments
Open

Test injection_queue_depth_multi_thread is flaky #6847

Darksonn opened this issue Sep 14, 2024 · 3 comments
Labels
A-tokio Area: The main tokio crate E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate M-metrics Module: tokio/runtime/metrics

Comments

@Darksonn
Copy link
Contributor

This test has been observed to fail in CI:

#[test]
fn injection_queue_depth_multi_thread() {
let rt = threaded();
let metrics = rt.metrics();
let barrier1 = Arc::new(Barrier::new(3));
let barrier2 = Arc::new(Barrier::new(3));
// Spawn a task per runtime worker to block it.
for _ in 0..2 {
let barrier1 = barrier1.clone();
let barrier2 = barrier2.clone();
rt.spawn(async move {
barrier1.wait();
barrier2.wait();
});
}
barrier1.wait();
for i in 0..10 {
assert_eq!(i, metrics.injection_queue_depth());
rt.spawn(async {});
}
barrier2.wait();
}

To close this issue, figure out why it is failing and fix it.

@Darksonn Darksonn added E-help-wanted Call for participation: Help is requested to fix this issue. A-tokio Area: The main tokio crate E-medium Call for participation: Experience needed to fix: Medium / intermediate M-metrics Module: tokio/runtime/metrics labels Sep 14, 2024
@Darksonn Darksonn changed the title injection_queue_depth_multi_thread Test injection_queue_depth_multi_thread is flaky Sep 14, 2024
@jofas
Copy link
Contributor

jofas commented Sep 15, 2024

I presume it's these two recent jobs that timed out

that have been affected by the flakiness of injection_queue_depth_multi_thread? I'd assume so because if this assert

assert_eq!(i, metrics.injection_queue_depth());

fails, the main thread panics and we never get to synchronise on barrier2

barrier2.wait();

here, causing the test to run forever (or until CI times out).

@Darksonn
Copy link
Contributor Author

Yes. Good point with the assert. That explains why it times out instead of failing normally.

@jofas
Copy link
Contributor

jofas commented Sep 16, 2024

I wonder if a first step1 would be to convert the assert into an if-statement where we'd panic only after calling barrier2.wait(). Then we'd get better diagnostics but more importantly wouldn't time out CI any more.

Footnotes

  1. In case this isn't a quick fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate M-metrics Module: tokio/runtime/metrics
Projects
None yet
Development

No branches or pull requests

2 participants