-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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: deflake test-http-regr-gh-2928 #49574
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
It is still flaky: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/426/. |
It seems that data stops flowing in the FreeBSD 12 machine where the stress test is run. Here is an example https://ci.nodejs.org/view/Stress/job/node-stress-single-test/429/. I don't know why. I can't reproduce the issue in a locally installed FreeBSD 12 VM. |
f06e780
to
68fcb02
Compare
6250590
to
a2f0ca3
Compare
Hard code the value of the host parameter to `common.localhostIPv4` in `server.listen()` and `net.connect()`. This 1. ensures that the client `socket._handle` is not reinitialized during connection due to the family autodetection algorithm, preventing `parser.consume()` from being called with an invalid `socket._handle` parameter. 2. works around an issue in the FreeBSD 12 machine where the stress test is run where some sockets get stuck after connection. Closes: nodejs#49565 Fixes: nodejs#49564
a2f0ca3
to
6db17f7
Compare
I now occasionally get a
on macOS but that can probably be fixed by reducing the number of sockets. The test does not need to create 1k sockets. Let's see how it goes. |
The reason why some sockets get stuck with no data flowing on the FreeBSD 12 machine when the host is not specified should be investigated (faulty IPv6?, buggy family autodetection?) but I think it should not block this. |
FreeBSD 12 stress test: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/436/ |
cc: @nodejs/testing |
Is it in shape for review now? (Can’t say for sure from the thread) |
@joyeecheung yes. |
Commit Queue failed- Loading data for nodejs/node/pull/49574 ✔ Done loading data for nodejs/node/pull/49574 ----------------------------------- PR info ------------------------------------ Title test: deflake test-http-regr-gh-2928 (#49574) ⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile! Branch lpinca:deflake/test-http-regr-gh-2928 -> nodejs:main Labels test, needs-ci Commits 1 - test: deflake test-http-regr-gh-2928 Committers 1 - Luigi Pinca PR-URL: https://github.com/nodejs/node/pull/49574 Fixes: https://github.com/nodejs/node/pull/49565 Fixes: https://github.com/nodejs/node/issues/49564 Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell ------------------------------ Generated metadata ------------------------------ PR-URL: https://github.com/nodejs/node/pull/49574 Fixes: https://github.com/nodejs/node/pull/49565 Fixes: https://github.com/nodejs/node/issues/49564 Reviewed-By: Yagiz Nizipli Reviewed-By: James M Snell -------------------------------------------------------------------------------- ℹ This PR was created on Sat, 09 Sep 2023 12:59:22 GMT ✔ Approvals: 2 ✔ - Yagiz Nizipli (@anonrig) (TSC): https://github.com/nodejs/node/pull/49574#pullrequestreview-1618710274 ✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/49574#pullrequestreview-1629983759 ✔ Last GitHub CI successful ℹ Last Full PR CI on 2023-09-10T19:40:28Z: https://ci.nodejs.org/job/node-test-pull-request/53872/ - Querying data for job/node-test-pull-request/53872/ ✔ Last Jenkins CI successful -------------------------------------------------------------------------------- ✔ No git cherry-pick in progress ✔ No git am in progress ✔ No git rebase in progress -------------------------------------------------------------------------------- - Bringing origin/main up to date... From https://github.com/nodejs/node * branch main -> FETCH_HEAD 5c39ee6f87..b651e37d2e main -> origin/main ✔ origin/main is now up-to-date main is out of sync with origin/main. Mismatched commits: - b651e37d2e 2023-09-18, Version 20.7.0 (Current) -------------------------------------------------------------------------------- HEAD is now at b651e37d2e 2023-09-18, Version 20.7.0 (Current) ✔ Reset to origin/main - Downloading patch for 49574 From https://github.com/nodejs/node * branch refs/pull/49574/merge -> FETCH_HEAD ✔ Fetched commits as b651e37d2e36..6db17f7c5518 -------------------------------------------------------------------------------- [main 26c4258a63] test: deflake test-http-regr-gh-2928 Author: Luigi Pinca Date: Sat Sep 9 14:44:31 2023 +0200 1 file changed, 2 insertions(+), 2 deletions(-) ✔ Patches applied -------------------------------------------------------------------------------- ⚠ Found Fixes: https://github.com/nodejs/node/issues/49564, skipping.. --------------------------------- New Message ---------------------------------- test: deflake test-http-regr-gh-2928https://github.com/nodejs/node/actions/runs/6226235586 |
Hard code the value of the host parameter to `common.localhostIPv4` in `server.listen()` and `net.connect()`. This 1. ensures that the client `socket._handle` is not reinitialized during connection due to the family autodetection algorithm, preventing `parser.consume()` from being called with an invalid `socket._handle` parameter. 2. works around an issue in the FreeBSD 12 machine where the stress test is run where some sockets get stuck after connection. PR-URL: #49574 Closes: #49565 Fixes: #49564 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Landed in 18e00a5. |
Hard code the value of the host parameter to `common.localhostIPv4` in `server.listen()` and `net.connect()`. This 1. ensures that the client `socket._handle` is not reinitialized during connection due to the family autodetection algorithm, preventing `parser.consume()` from being called with an invalid `socket._handle` parameter. 2. works around an issue in the FreeBSD 12 machine where the stress test is run where some sockets get stuck after connection. PR-URL: #49574 Closes: #49565 Fixes: #49564 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Hard code the value of the host parameter to `common.localhostIPv4` in `server.listen()` and `net.connect()`. This 1. ensures that the client `socket._handle` is not reinitialized during connection due to the family autodetection algorithm, preventing `parser.consume()` from being called with an invalid `socket._handle` parameter. 2. works around an issue in the FreeBSD 12 machine where the stress test is run where some sockets get stuck after connection. PR-URL: nodejs#49574 Closes: nodejs#49565 Fixes: nodejs#49564 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Hard code the value of the host parameter to `common.localhostIPv4` in `server.listen()` and `net.connect()`. This 1. ensures that the client `socket._handle` is not reinitialized during connection due to the family autodetection algorithm, preventing `parser.consume()` from being called with an invalid `socket._handle` parameter. 2. works around an issue in the FreeBSD 12 machine where the stress test is run where some sockets get stuck after connection. PR-URL: #49574 Backport-PR-URL: #52384 Closes: #49565 Fixes: #49564 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Hard code the value of the host parameter to `common.localhostIPv4` in `server.listen()` and `net.connect()`. This 1. ensures that the client `socket._handle` is not reinitialized during connection due to the family autodetection algorithm, preventing `parser.consume()` from being called with an invalid `socket._handle` parameter. 2. works around an issue in the FreeBSD 12 machine where the stress test is run where some sockets get stuck after connection. PR-URL: nodejs/node#49574 Closes: nodejs/node#49565 Fixes: nodejs/node#49564 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: James M Snell <[email protected]>
Hard code the value of the host parameter to
common.localhostIPv4
inserver.listen()
andnet.connect()
. Thissocket._handle
is not reinitialized duringconnection due to the family autodetection algorithm, preventing
parser.consume()
from being called with an invalidsocket._handle
parameter.
is run where some sockets get stuck after connection.
Closes: #49565
Fixes: #49564