Repository navigation
Investigate flaky test-inspector-port-zero-cluster #13343
Description
Activity
/cc @bnoordhuis
Happened again, this time on ubuntu1604_docker_alpine34-64.
https://ci.nodejs.org/job/node-test-commit-linux/10277/nodes=ubuntu1604_docker_alpine34-64/console
not ok 1413 inspector/test-inspector-port-zero-cluster --- duration_ms: 0.329 severity: fail stack: |- Mismatched <anonymous> function calls. Expected exactly 1, actual 0. at Object.exports.mustCall (/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1604_docker_alpine34-64/test/common/index.js:483:10) at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1604_docker_alpine34-64/test/inspector/test-inspector-port-zero-cluster.js:13:33) at Module._compile (module.js:569:30) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:503:32) at tryModuleLoad (module.js:466:12) at Function.Module._load (module.js:458:3) at Function.Module.runMain (module.js:605:10) at startup (bootstrap_node.js:158:16) Debugger listening on ws://127.0.0.1:45189/8f57c8a5-a17f-4f7e-809b-1f1672a54ae3 For help see https://nodejs.org/en/docs/inspector Debugger listening on ws://127.0.0.1:45191/38e5e2f4-cadb-4c0a-a1fd-af925e5e7e36 For help see https://nodejs.org/en/docs/inspector Starting inspector on 127.0.0.1:45190 failed: address already in use Debugger listening on ws://127.0.0.1:45192/c595ccac-dbea-4443-8c2b-8cd003d52a9a For help see https://nodejs.org/en/docs/inspector
In both cases, one worker fails because the port is already in use:
Starting inspector on 127.0.0.1:32832 failed: address already in useStarting inspector on 127.0.0.1:45190 failed: address already in useIt looks like this is not guaranteeing an available port but perhaps just using sequential ports?
This failure is replicable by running
tools/test.py --repeat 10000 test/inspector/test-inspector-port-zero-cluster.jsin two separate shells at once. Tests aren't necessarily expected to be multi-process safe, but in this case, it seems like they ought not interfere with each other, but they do. And the resulting error is similar to be what we're seeing in CI with a port in use and therefore the callback not firing for one of the workers:$ tools/test.py --repeat 10000 test/inspector/test-inspector-port-zero-cluster.js === release test-inspector-port-zero-cluster === Path: inspector/test-inspector-port-zero-cluster Mismatched <anonymous> function calls. Expected exactly 1, actual 0. at Object.exports.mustCall (/Users/trott/io.js/test/common/index.js:483:10) at Object.<anonymous> (/Users/trott/io.js/test/inspector/test-inspector-port-zero-cluster.js:13:33) at Module._compile (module.js:569:30) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:503:32) at tryModuleLoad (module.js:466:12) at Function.Module._load (module.js:458:3) at Function.Module.runMain (module.js:605:10) at startup (bootstrap_node.js:158:16) Debugger listening on ws://127.0.0.1:59247/6206a5c6-560a-4d48-85ce-86da803afe59 For help see https://nodejs.org/en/docs/inspector Starting inspector on 127.0.0.1:59248 failed: address already in use Debugger listening on ws://127.0.0.1:59249/75356721-69d4-47df-bdb9-a856946f42a1 For help see https://nodejs.org/en/docs/inspector Debugger listening on ws://127.0.0.1:59250/8c1ec737-202f-4dd7-9fa8-9765fffed167 For help see https://nodejs.org/en/docs/inspector Command: out/Release/node --inspect=0 /Users/trott/io.js/test/inspector/test-inspector-port-zero-cluster.js [snip]
Tests aren't necessarily expected to be multi-process safe
Than that's a new "known-issue" since cluster has always set sequential debug ports for it's children (initial port=0 or not)
https://github.com/nodejs/node/blob/master/lib/internal/cluster/master.js#L113My assumption in #13373 is that there might be an test-internal race.
cluster has always set sequential debug ports for it's children
Ooof. That seems like a bug to me, albeit one that would cause problems only infrequently and one that might be very tricky to fix.
OK, so to summarize my understanding thus far:
-
The test is probably not internally safe. Workers can both claim the same port. That needs to be fixed in the test itself and @refack is on the case in test: fix test-inspector-port-zero-cluster #13373. (EDIT: Actually, I'm not sure this is the problem because the CI output above does not show two workers grabbing the same port. Maybe it's conflicting with a pre-existing process on the host? In which case, I'm not sure there's anything we can do about it in the test itself?)
-
Incrementing port numbers this way is probably not ideal, but it's not clear that there's a better solution. Perhaps there's a way to use port
0inlib/internal/cluster/master.jsand have everything work in harmony, but there might be all sorts of problems that arise in trying to implement that because we're going from something deterministic and synchronous to something non-deterministic and asynchronous. But it might be A Good Thing to do because... -
...it seems likely that the sort of bug we're seeing in the test here probably occurs in the real world. We probably don't hear about it because it causes problems only intermittently.
Sound about right?
-
Perhaps use
0if the cluster master is called with0, otherwise use predetermined ports so as to be backwards-compatible?10 remaining items
No, it's a feature and one that should be preserved.
Does it make sense to preserve that behavior when a port is specified or the default port is used, but not when started with the port
0option?If that makes sense, then lets do that.
If that doesn't make sense, then the test needs to be changed to allow for unpredictable port collisions.
Does it make sense to preserve that behavior when a port is specified or the default port is used, but not when started with the port 0 option?
I'm personally of the opinion that having predictable port numbers is an important feature for ease-of-use in debugging.
I don't expect that users are going to run into port conflicts often, that's just something we'll have to deal with in our test suite.
- added a commit that references this issue
on Jun 5, 2017 - added a commit that references this issue
on Jun 16, 2017 - added a commit that references this issue
on Jun 16, 2017 - added a commit that references this issue
on Sep 19, 2017
https://ci.nodejs.org/job/node-test-commit-linux/10262/nodes=fedora24/console