lukechilds/node - node - Gitea: Git with a cup of tea

13841 Commits

131 Branches

446 Tags

325 MiB

Tree: f136f72180

Commit Graph

Author	SHA1	Message	Date
Rich Trott	0634311a9a	test: move cluster tests to parallel Two cluster tests have recently changed so that they are no longer resource intensive. Move them back to parallel. Ref: https://github.com/nodejs/node/pull/4736 Ref: https://github.com/nodejs/node/pull/4739 PR-URL: https://github.com/nodejs/node/pull/4774 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>	9 years ago
Rich Trott	44aba1ab1c	test: improve test-cluster-disconnect-suicide-race Previously, test-cluster-disconnect-suicide-race had two issues: * Magic numbers: How many times to spawn a worker was determined through empirical experimentation. This means that as new platforms and new CPU/RAM configurations are tested, the magic numbers require more and more refinement. This brings us to... * Non-determinism: The test seems to fail all the time when the bug it tests for is present, but it's really a judgment based on sampling. "Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try 16..." This revised version of the test takes a different approach. The fix for the bug that the test was written for means that the disconnect event will fire on a subsequent tick. So we check for that and the test still fails when the fix is not in the code base and succeeds when it is. Advantages of this approach include: * The test runs much faster. * The test should be reliable on any new platform regardless of CPU and RAM. PR-URL: https://github.com/nodejs/node/pull/4739 Ref: https://github.com/nodejs/node/pull/4674 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>	9 years ago
Santiago Gimeno	9571be12f6	cluster: fix race condition setting suicide prop There is no guarantee that the `suicide` property of a worker in the master process is going to be set when the `disconnect` and `exit` events are emitted. To fix it, wait for the ACK of the suicide message from the master before disconnecting the worker. Also, there's no need to send the suicide message from the worker if the disconnection has been initiated in the master. Add `test-cluster-disconnect-suicide-race` that forks a lot of workers to consistently reproduce the issue this patch tries to solve. Modify `test-regress-GH-3238` so it checks both the `kill` and `disconnect` cases. Also take into account that the `disconnect` event may be received after the `exit` event. PR-URL: https://github.com/nodejs/node/pull/4349 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>	9 years ago

Author

SHA1

Message

Date

Rich Trott

0634311a9a

test: move cluster tests to parallel

Two cluster tests have recently changed so that they are no longer
resource intensive. Move them back to parallel.

Ref: https://github.com/nodejs/node/pull/4736
Ref: https://github.com/nodejs/node/pull/4739
PR-URL: https://github.com/nodejs/node/pull/4774
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

9 years ago

Rich Trott

44aba1ab1c

test: improve test-cluster-disconnect-suicide-race

Previously, test-cluster-disconnect-suicide-race had two issues:

* Magic numbers: How many times to spawn a worker was determined through
empirical experimentation. This means that as new platforms and new
CPU/RAM configurations are tested, the magic numbers require more
and more refinement. This brings us to...

* Non-determinism: The test seems to fail all the time when the bug
it tests for is present, but it's really a judgment based on sampling.
"Oh, with 8 workers per CPU, it fails about 80% of the time. Let's try
16..."

This revised version of the test takes a different approach. The fix
for the bug that the test was written for means that the disconnect
event will fire on a subsequent tick. So we check for that and the test
still fails when the fix is not in the code base and succeeds when it
is.

Advantages of this approach include:

* The test runs much faster.
* The test should be reliable on any new platform regardless of CPU and
RAM.

PR-URL: https://github.com/nodejs/node/pull/4739
Ref: https://github.com/nodejs/node/pull/4674
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>

9 years ago

Santiago Gimeno

9571be12f6

cluster: fix race condition setting suicide prop

There is no guarantee that the `suicide` property of a worker in the
master process is going to be set when the `disconnect` and `exit`
events are emitted.

To fix it, wait for the ACK of the suicide message from the master
before disconnecting the worker. Also, there's no need to send the
suicide message from the worker if the disconnection has been
initiated in the master.

Add `test-cluster-disconnect-suicide-race` that forks a lot of workers
to consistently reproduce the issue this patch tries to solve.

Modify `test-regress-GH-3238` so it checks both the `kill` and
`disconnect` cases. Also take into account that the `disconnect` event
may be received after the `exit` event.

PR-URL: https://github.com/nodejs/node/pull/4349
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

9 years ago

1 Commits (f136f721800d749cca3fe519a05e529eeed86676)