flaky test-http2-reset-flood #29802
Comments
sam-github
added
freebsd
This test is somewhat regularly flaky on 12.x and it appears master as well. There appears to be an issue tracking flakes for 6+ months. Seems reasonable to mark this to keep us from having red CI. Refs: nodejs#29802
This test is somewhat regularly flaky on 12.x and it appears master as well. There appears to be an issue tracking flakes for 6+ months. Seems reasonable to mark this to keep us from having red CI. Refs: #29802 PR-URL: #32595 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This test is somewhat regularly flaky on 12.x and it appears master as well. There appears to be an issue tracking flakes for 6+ months. Seems reasonable to mark this to keep us from having red CI. Refs: #29802 PR-URL: #32595 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
We are seeing this as flaky on 12.x on a bunch of other linux distros as well https://ci.nodejs.org/job/node-test-commit/37126/ Not sure what we should do about this |
|
@addaleax also seeing this fail on windows now too (all on 12.x). Do you think this is a real failure? Should we mark this flaky across all release lines? Expand scope of this issue? |
|
Here is an example of it failing across almost every platform |
|
@MylesBorins The v12.x failures seem very different from the failure on master. I could reproduce the v12.x failures locally on Linux. This patch fixes it for me and shouldn’t affect the test’s functionaliy: diff --git a/test/parallel/test-http2-reset-flood.js b/test/parallel/test-http2-reset-flood.js
index 7ee534cf4faa..d721384ab374 100644
--- a/test/parallel/test-http2-reset-flood.js
+++ b/test/parallel/test-http2-reset-flood.js
@@ -69,9 +69,10 @@ const worker = new Worker(__filename).on('message', common.mustCall((port) => {
h2header.writeIntBE(streamId, 5, 4); // Stream ID
streamId += 2;
// 0x88 = :status: 200
- conn.write(Buffer.concat([h2header, Buffer.from([0x88])]));
+ if (conn.writable)
+ conn.write(Buffer.concat([h2header, Buffer.from([0x88])]));
}
- if (!gotError)
+ if (conn.writable && !gotError)
setImmediate(writeRequests);
}
I’m not sure why it’s not needed on master but it’s probably because @ronag put a lot of work into streams finally making sense. Either way, feel free to take that patch and open a PR with it 🙂 |
Refs: #29802 (comment) PR-URL: #32607 Reviewed-By: Anna Henningsen <anna@addaleax.net>
This test is somewhat regularly flaky on 12.x and it appears master as well. There appears to be an issue tracking flakes for 6+ months. Seems reasonable to mark this to keep us from having red CI. Refs: #29802 PR-URL: #32595 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Failed in master here: https://ci.nodejs.org/job/node-test-commit-linux/nodes=fedora-last-latest-x64/34123/ |
This test is somewhat regularly flaky on 12.x and it appears master as well. There appears to be an issue tracking flakes for 6+ months. Seems reasonable to mark this to keep us from having red CI. Refs: #29802 PR-URL: #32595 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
still flaky on various platforms, linuxone in this case https://ci.nodejs.org/job/node-test-pull-request/30678/ |
|
failed in https://ci.nodejs.org/job/node-test-commit-linux-containered/19466/, linux-containered, x86 |
Trott
changed the title
Refs: nodejs#29802 Refs: nodejs#32595 PR-URL: nodejs#32825 Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com>
|
Timed out at https://ci.nodejs.org/job/node-test-commit-linux/36018/nodes=debian9-64/console on test-digitalocean-debian9-x64-1: |
Move boolean check to start of writeRequest() to avoid race condition. Fixes: nodejs#29802
|
I changed the test to track calls and to timeout after two seconds (because it normally takes about 100 milliseconds on the debian9 machine in CI) and it is showing things like 167450 calls to the test's Here's the modified test I ran: https://github.com/nodejs/node/blob/09713d420bfd65ab550840eb55e10e4996bbcba2/test/parallel/test-http2-reset-flood.js And here's a sample failure in the stress test on CI: The key part is that first line that shows that I had previously inserted a bunch of |
|
The timeouts we're seeing now are different than the issue that was first reported (which appears to have been fixed). I did a bisect using CI today (since it's easy to make this happen in the stress tests) and came up with efefdd6 as the first commit where the problem was introduced. I'll do some more tests to confirm, but if someone knowledgable wants to take a look there, that would be great. |
|
Setting |
Set `allowHalfOpen: true` in the client. Fixes: nodejs#29802 Refs: nodejs#31806
From #29727, which seems wholly unrelated, since it changes the link flags on linux, and this is a freebsd failure.
@nodejs/platform-freebsd
The text was updated successfully, but these errors were encountered: