test: fix test-cluster-disconnect-leak.js for AIX #4674

imran-iq · 2016-01-13T19:45:56Z

lowered the number of spawned workers as EAGAIN errors were
being thrown.

lowered the number of spawned workers as EAGAIN errors were being thrown.

Trott · 2016-01-13T20:12:12Z

Would you be able to confirm that the modified test you are proposing here still fails on AIX if the three lines added to cluster.js in f9f1dd9 are removed? This test should fail without that fix, but should succeed if that fix is there.

If you have nvm or some other convenient way to switch between Node versions, you can simply check that the test fires an AssertionError when executed with Node.js 5.4.0 and passes when executed with Node.js 5.4.1. Here's what it looks like on my OS X machine:

$ nvm use 5.4.0
Now using node v5.4.0 (npm v3.3.12)
$ node --expose-internals test/sequential/test-cluster-disconnect-leak.js

assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: Resource leak detected.
    at removeWorker (cluster.js:317:7)
    at Worker.disconnect (cluster.js:408:5)
    at Worker.<anonymous> (/Users/trott/io.js/test/common.js:395:15)
    at emitNone (events.js:67:13)
    at Worker.emit (events.js:166:7)
    at online (cluster.js:438:12)
    at Worker.onmessage (cluster.js:425:7)
    at ChildProcess.<anonymous> (cluster.js:714:8)
    at emitTwo (events.js:92:20)
    at ChildProcess.emit (events.js:172:7)
$ nvm use 5.4.1
Now using node v5.4.1 (npm v3.3.12)
$ node --expose-internals test/sequential/test-cluster-disconnect-leak.js
$

imran-iq · 2016-01-13T21:20:16Z

Hi, it seems that the issue is unrelated to the fix.
We ended up hitting the maximum number of processes allowed per user on default AIX.

/usr/sbin/lsattr -E -l sys0 | grep maxuproc
maxuproc        128                                  Maximum number of PROCESSES allowed per user      True

Without my change set and increasing the maxuproc to 150 with

/usr/sbin/chdev -l sys0 -a maxuproc=150

allows the test to pass without my changes. So I guess the question is does the number have to be that high for when we have more than 8 cores? Or should I say why is there a need for 16 worker forks per cpu?

santigimeno · 2016-01-14T11:25:51Z

@iwuzhere it's just an empiric value: it was chosen so that the test wuold fail without the changes in lib/cluster.js consistently in multiple platforms. What would be the max value that would be acceptable for you? We could try with it and see if it still fails in the rest of platforms

Trott · 2016-01-14T16:25:29Z

Alternatively, maybe we could also try switching to an exponential ramp-up in the test so that we're not relying on calibrating the number like this. The magic number may change as hardware and operating systems evolve, so it might be slightly more future-proof to do it that way.

imran-iq · 2016-01-15T15:06:16Z

This is also happening in test-cluster-disconnect-suicide-race.js. The test fails with EAGAIN on AIX for spawning 128 ( 64 * 2 ) workers.

Trott · 2016-01-18T03:43:42Z

I think I've found a faster and more reliable/deterministic way to test this issue. PR coming in a bit...

Trott · 2016-01-18T05:02:52Z

See alternative at #4736

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: nodejs#4674

@santigimeno

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. Ref: nodejs#4674 cc @santigimeno @iwuzhere

@santigimeno

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. Ref: nodejs#4674 cc @santigimeno @iwuzhere

cjihrig · 2016-01-18T18:00:35Z

Closing in favor of #4736. If that doesn't work out, we can reopen this one.

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: #4674 PR-URL: #4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: #4739 Ref: #4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: #4674 PR-URL: #4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: #4739 Ref: #4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: #4674 PR-URL: #4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: #4739 Ref: #4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: #4674 PR-URL: #4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: #4739 Ref: #4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: nodejs#4674 PR-URL: nodejs#4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: nodejs#4739 Ref: nodejs#4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: nodejs#4674 PR-URL: nodejs#4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: nodejs#4739 Ref: nodejs#4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: nodejs#4674 PR-URL: nodejs#4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: nodejs#4739 Ref: nodejs#4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

Previously, test-cluster-disconnect-leak 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 reliably for a single worker. 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 now works on Windows. The previous version skipped Windows. * The test should be reliable on any new platform regardless of CPU and RAM. Ref: nodejs#4674 PR-URL: nodejs#4736 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Johan Bergström <[email protected]>

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: nodejs#4739 Ref: nodejs#4674 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>

test: fix test-cluster-disconnect-leak.js for AIX

72ec371

lowered the number of spawned workers as EAGAIN errors were being thrown.

r-52 added cluster Issues and PRs related to the cluster subsystem. test Issues and PRs related to the tests. labels Jan 13, 2016

jasnell added the lts-watch-v4.x label Jan 13, 2016

Trott mentioned this pull request Jan 18, 2016

test: make test-cluster-disconnect-leak reliable #4736

Closed

Trott mentioned this pull request Jan 18, 2016

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

Closed

cjihrig closed this Jan 18, 2016

MylesBorins removed the lts-watch-v4.x label Jan 28, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: fix test-cluster-disconnect-leak.js for AIX #4674

test: fix test-cluster-disconnect-leak.js for AIX #4674

imran-iq commented Jan 13, 2016

Trott commented Jan 13, 2016

imran-iq commented Jan 13, 2016

santigimeno commented Jan 14, 2016

Trott commented Jan 14, 2016

imran-iq commented Jan 15, 2016

Trott commented Jan 18, 2016

Trott commented Jan 18, 2016

cjihrig commented Jan 18, 2016

test: fix test-cluster-disconnect-leak.js for AIX #4674

test: fix test-cluster-disconnect-leak.js for AIX #4674

Conversation

imran-iq commented Jan 13, 2016

Trott commented Jan 13, 2016

imran-iq commented Jan 13, 2016

santigimeno commented Jan 14, 2016

Trott commented Jan 14, 2016

imran-iq commented Jan 15, 2016

Trott commented Jan 18, 2016

Trott commented Jan 18, 2016

cjihrig commented Jan 18, 2016