-
Notifications
You must be signed in to change notification settings - Fork 30k
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
investigate flaky test: test-cluster-net-listen-ipv6only-rr #25813
Comments
https://ci.nodejs.org/job/node-test-commit-linux/25373/nodes=centos6-64-gcc6/consoleFull test-softlayer-centos6-x64-1 00:17:45 not ok 332 parallel/test-cluster-net-listen-ipv6only-rr
00:17:45 ---
00:17:45 duration_ms: 0.754
00:17:45 severity: fail
00:17:45 exitcode: 1
00:17:45 stack: |-
00:17:45 events.js:173
00:17:45 throw er; // Unhandled 'error' event
00:17:45 ^
00:17:45
00:17:45 Error: listen EADDRINUSE: address already in use 0.0.0.0:38498
00:17:45 at Server.setupListenHandle [as _listen2] (net.js:1232:14)
00:17:45 at listenInCluster (net.js:1280:12)
00:17:45 at doListen (net.js:1419:7)
00:17:45 at processTicksAndRejections (internal/process/next_tick.js:76:17)
00:17:45 Emitted 'error' event at:
00:17:45 at emitErrorNT (net.js:1259:8)
00:17:45 at processTicksAndRejections (internal/process/next_tick.js:76:17)
00:17:45 ... |
test-linuxonecc-rhel72-s390x-1 events.js:173
throw er; // Unhandled 'error' event
^
Error: listen EADDRINUSE: address already in use 0.0.0.0:46458
at Server.setupListenHandle [as _listen2] (net.js:1232:14)
at listenInCluster (net.js:1280:12)
at doListen (net.js:1419:7)
at processTicksAndRejections (internal/process/next_tick.js:76:17)
Emitted 'error' event at:
at emitErrorNT (net.js:1259:8)
at processTicksAndRejections (internal/process/next_tick.js:76:17) |
The program causes server listening 4 times by design:
First suspect of course is some entity listening twice on the same port
So the next suspect is whether the count down latch is broken, and was somehow re-entered. Used this patch to disprove that theory, as it never asserted with this change. --- a/test/parallel/test-cluster-net-listen-ipv6only-rr.js
+++ b/test/parallel/test-cluster-net-listen-ipv6only-rr.js
@@ -15,11 +15,13 @@ cluster.schedulingPolicy = cluster.SCHED_RR;
const host = '::';
const WORKER_ACCOUNT = 3;
+let reenter = 0;
if (cluster.isMaster) {
const workers = new Map();
let address;
const countdown = new Countdown(WORKER_ACCOUNT, () => {
+ assert.strictEqual(++reenter, 1);
// Make sure the `ipv6Only` option works.
const server = net.createServer().listen({
host: '0.0.0.0', Next suspect is:
Does this make sense? |
looks like the above theory is confirmed: const cluster = require('cluster');
const net = require('net');
cluster.schedulingPolicy = cluster.SCHED_RR;
const host = '::';
let buf = [];
let j = 0;
if (cluster.isMaster) {
for(var i = 0; i < 3000; i++) {
net.createServer(() => {}).listen({host: '0.0.0.0', port: 0}, function() {
buf.push(this.address().port)
if(++j === 3000) {
cluster.fork().on('listening', (addr) => {
if(buf.includes(addr.port)) {
console.log(`got it at ${addr.port}!`)
process.exit(0);
}
})
}
});
}
} else {
setInterval(() => {
net.createServer((q, r) => {}).listen({host: host, port: 0, ipv6Only: true}, () => {
})
}, 100)
} $ ./node foo So this means that the cluster member can just pick up any listening port. So clearly this conflicts in a parallel test execution environment. |
In test test-cluster-net-listen-ipv6only-rr, the cluster member that listens to `any` port actually has the potential to `grab` any port from the environment which when passed onto the master causes collision when it tries to listen on. Moving the test to sequential alone is not sufficient as the cluster member can in theory catch on to the admin ports on the host. Assigning static port alone is also not sufficient, as it can interfere with other running tests in the parallel category which would be mostly running with `port: any` fashion. So move to sequential, and use a static port. Fixes: nodejs#25813 PR-URL: nodejs#26298 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
In test test-cluster-net-listen-ipv6only-rr, the cluster member that listens to `any` port actually has the potential to `grab` any port from the environment which when passed onto the master causes collision when it tries to listen on. Moving the test to sequential alone is not sufficient as the cluster member can in theory catch on to the admin ports on the host. Assigning static port alone is also not sufficient, as it can interfere with other running tests in the parallel category which would be mostly running with `port: any` fashion. So move to sequential, and use a static port. Fixes: #25813 PR-URL: #26298 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
https://ci.nodejs.org/job/node-test-commit-linux/nodes=ubuntu1604-64/24988/consoleFull
The text was updated successfully, but these errors were encountered: