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: increase bufsize in child process write test #13626

Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Jun 12, 2017

test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

Fixes: #13603

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test child_process

test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

Fixes: nodejs#13603
@Trott Trott added child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests. labels Jun 12, 2017
@Trott Trott force-pushed the is-that-freedom-rock-well-turn-it-up branch from 77aab91 to 30c1e00 Compare June 12, 2017 04:14
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jun 12, 2017
@Trott
Copy link
Member Author

Trott commented Jun 12, 2017

@Trott
Copy link
Member Author

Trott commented Jun 12, 2017

ubuntu1604-arm64 is green in the CI! Victory! (Unless something else comes back red...)

@Trott
Copy link
Member Author

Trott commented Jun 12, 2017

I'd like to expedite this to get CI back to green.

/cc @nodejs/testing

And a rare /cc @isaacs because they wrote the test originally and may know reasons this approach may be inadvisable.

@Trott
Copy link
Member Author

Trott commented Jun 12, 2017

CI is green!

@@ -51,14 +51,15 @@ function parent() {
// Write until the buffer fills up.
let buf;
do {
buf = Buffer.alloc(BUFSIZE, '.');
sent += BUFSIZE;
bufsize += 1024;
Copy link
Contributor

@refack refack Jun 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it definitely looks like a /pummel/...
But:

ok 184 parallel/test-child-process-stdio-big-write-end
  ---
  duration_ms: 0.332

🤷‍♂️

@refack
Copy link
Contributor

refack commented Jun 12, 2017

I'd like to expedite this to get CI back to green.

👍

@gibfahn
Copy link
Member

gibfahn commented Jun 12, 2017

test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

Probably a stupid question, but why exactly does the buffer fill become an infinite loop on this machine? Is it because there's so much RAM?

@tniessen
Copy link
Member

@gibfahn This is not necessarily caused by too much RAM, but the child process might just clear the buffer too quickly. I think Linux uses something around 64k as the pipe buffer (not all the RAM!).

@Trott
Copy link
Member Author

Trott commented Jun 12, 2017

Landed in b71d677

@Trott Trott closed this Jun 12, 2017
Trott added a commit to Trott/io.js that referenced this pull request Jun 12, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

PR-URL: nodejs#13626
Fixes: nodejs#13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Jun 17, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

PR-URL: #13626
Fixes: #13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax mentioned this pull request Jun 17, 2017
addaleax pushed a commit that referenced this pull request Jun 21, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

PR-URL: #13626
Fixes: #13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@addaleax addaleax mentioned this pull request Jun 21, 2017
MylesBorins pushed a commit that referenced this pull request Jul 17, 2017
test-child-process-stdio-big-write-end was failing on ubuntu1604-arm64
because the while loop that was supposed to fill up the buffer ended up
being an infinite loop.

This increases the size of the writes in the loop by 1K until the buffer
fills up.

PR-URL: #13626
Fixes: #13603
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
@Trott Trott deleted the is-that-freedom-rock-well-turn-it-up branch January 13, 2022 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test: investigate flaky test-child-process-stdio-big-write-end
8 participants