GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — test: deflake test-http-regr-gh-2928 by lpinca · Pull Request #49574 · nodejs/node

Conversation

@lpinca

@lpinca lpinca commented Sep 9, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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: #49565
Fixes: #49564

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Sep 9, 2023
@lpinca

This comment was marked as outdated.

Sign in to view
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 9, 2023
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 9, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 9, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53839/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 9, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53841/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca

lpinca commented Sep 9, 2023

Copy link
Copy Markdown
Member Author

It is still flaky: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/426/.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca lpinca added the blocked PRs that are blocked by other issues or PRs. label Sep 9, 2023
@github-actions github-actions Bot mentioned this pull request Sep 10, 2023
CI Reliability 2023-09-10 nodejs/reliability#661
Open
19 tasks
anonrig
anonrig approved these changes Sep 10, 2023
View reviewed changes
@lpinca

lpinca commented Sep 10, 2023

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca lpinca mentioned this pull request Sep 10, 2023
test-http-regr-gh-2928 is flaky #49564
Closed
@lpinca
lpinca marked this pull request as draft September 10, 2023 13:26
@lpinca
lpinca force-pushed the deflake/test-http-regr-gh-2928 branch from f06e780 to 68fcb02 Compare September 10, 2023 13:55
@lpinca
lpinca marked this pull request as ready for review September 10, 2023 15:20
@lpinca lpinca added commit-queue-squash PRs the Commit Queue should land as one squashed commit. and removed blocked PRs that are blocked by other issues or PRs. labels Sep 10, 2023
@lpinca
lpinca force-pushed the deflake/test-http-regr-gh-2928 branch from 6250590 to a2f0ca3 Compare September 10, 2023 15:24
@lpinca
test: deflake test-http-regr-nodejsgh-2928
6db17f7
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
@lpinca
lpinca force-pushed the deflake/test-http-regr-gh-2928 branch from a2f0ca3 to 6db17f7 Compare September 10, 2023 15:25
@lpinca lpinca removed the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label Sep 10, 2023
@lpinca

lpinca commented Sep 10, 2023

Copy link
Copy Markdown
Member Author

I now occasionally get a connect ETIMEDOUT error when running

$ python3 tools/test.py --repeat=100 test/sequential/test-http-regr-gh-2928.js

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca

lpinca commented Sep 10, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 10, 2023
@lpinca

lpinca commented Sep 10, 2023

Copy link
Copy Markdown
Member Author

FreeBSD 12 stress test: https://ci.nodejs.org/view/Stress/job/node-stress-single-test/436/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 10, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53866/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53871/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Sep 10, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/53872/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca added a commit to lpinca/node that referenced this pull request Sep 10, 2023
@lpinca
test: add flaky test for investigation
c7f54c6
Refs: nodejs#49574 (comment)
Refs: nodejs#49574 (comment)
This was referenced Sep 11, 2023
CI Reliability 2023-09-11 nodejs/reliability#662
Open
CI Reliability 2023-09-12 nodejs/reliability#663
Open
CI Reliability 2023-09-13 nodejs/reliability#664
Open
CI Reliability 2023-09-14 nodejs/reliability#665
Open
CI Reliability 2023-09-15 nodejs/reliability#666
Open
@lpinca

lpinca commented Sep 15, 2023

Copy link
Copy Markdown
Member Author

cc: @nodejs/testing

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@joyeecheung

joyeecheung commented Sep 16, 2023

Copy link
Copy Markdown
Member

Is it in shape for review now? (Can’t say for sure from the thread)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca

lpinca commented Sep 16, 2023

Copy link
Copy Markdown
Member Author

@joyeecheung yes.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jasnell
jasnell approved these changes Sep 16, 2023
View reviewed changes
@lpinca lpinca added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 18, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 18, 2023
@nodejs-github-bot

nodejs-github-bot commented Sep 18, 2023

Copy link
Copy Markdown
Collaborator
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-2928

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: #49565
Fixes: #49564
PR-URL: #49574
Fixes: #49565
Reviewed-By: Yagiz Nizipli [email protected]
Reviewed-By: James M Snell [email protected]

[main 9756b0d684] test: deflake test-http-regr-gh-2928
Author: Luigi Pinca [email protected]
Date: Sat Sep 9 14:44:31 2023 +0200
1 file changed, 2 insertions(+), 2 deletions(-)
✖ 9756b0d68462303f0e35346e6df4bb695fcbf022
✔ 0:0 no Co-authored-by metadata co-authored-by-is-trailer
✔ 12:7 Valid fixes URL. fixes-url
✖ 14:7 Pull request URL must reference a comment or discussion. fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 13:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✔ 0:0 Title is formatted correctly. title-format
✔ 0:0 Title is <= 50 columns. title-length

ℹ Please fix the commit message and try again.
Please manually ammend the commit message, by running
git commit --amend
Once commit message is fixed, finish the landing command running
git node land --continue

https://github.com/nodejs/node/actions/runs/6226235586

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca added a commit that referenced this pull request Sep 18, 2023
@lpinca
test: deflake test-http-regr-gh-2928
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]>
@lpinca

lpinca commented Sep 18, 2023

Copy link
Copy Markdown
Member Author

Landed in 18e00a5.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@lpinca lpinca closed this Sep 18, 2023
@lpinca
lpinca deleted the deflake/test-http-regr-gh-2928 branch September 18, 2023 18:16
ruyadorno pushed a commit that referenced this pull request Sep 28, 2023
@lpinca @ruyadorno
test: deflake test-http-regr-gh-2928
b9e5b43
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]>
This was referenced Sep 28, 2023
v20.7.1 proposal #49917
Closed
v20.8.0 proposal #49932
Merged
@lpinca lpinca mentioned this pull request Apr 6, 2024
[v18.x backport] backport 49574 and 50240 to v18.x #52384
Closed
@lpinca lpinca added the backport-open-v18.x label Apr 6, 2024
richardlau pushed a commit that referenced this pull request Apr 17, 2024
@lpinca @richardlau
test: deflake test-http-regr-gh-2928
5186e45
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]>
@lpinca lpinca added backported-to-v18.x and removed backport-open-v18.x labels Apr 17, 2024
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
@lpinca
test: deflake test-http-regr-gh-2928
e14335c
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]>
@richardlau richardlau mentioned this pull request May 16, 2024
v18.20.3 proposal #53028
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@anonrig anonrig anonrig approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test-http-regr-gh-2928 is flaky

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@lpinca @nodejs-github-bot @joyeecheung @jasnell @anonrig
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.