Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ jobs:
uses: node-modules/github-actions/.github/workflows/node-test.yml@master
with:
os: 'ubuntu-latest, macos-latest'
version: '14, 16, 18, 20, 22'
version: '14, 16, 18, 20, 22, 24'
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ run
.nyc_output
package-lock.json
.package-lock.json
pnpm-lock.yaml
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

[npm-image]: https://img.shields.io/npm/v/egg-cluster.svg?style=flat-square
[npm-url]: https://npmjs.org/package/egg-cluster
[codecov-image]: https://codecov.io/github/eggjs/egg-cluster/coverage.svg?branch=master
[codecov-url]: https://codecov.io/github/eggjs/egg-cluster?branch=master
[codecov-image]: https://codecov.io/github/eggjs/cluster/coverage.svg?branch=master
[codecov-url]: https://codecov.io/github/eggjs/cluster?branch=master
[snyk-image]: https://snyk.io/test/npm/egg-cluster/badge.svg?style=flat-square
[snyk-url]: https://snyk.io/test/npm/egg-cluster
[download-image]: https://img.shields.io/npm/dm/egg-cluster.svg?style=flat-square
Expand Down Expand Up @@ -53,12 +53,13 @@ startCluster(options, () => {
| workers | `Number` | numbers of app workers |
| sticky | `Boolean` | sticky mode server |
| port | `Number` | port |
| reusePort | `Boolean` | (Required Node.js >= 22.12.0) allows multiple sockets on the same host to bind to the same port. Incoming connections are distributed by the operating system to listening sockets. This option is available only on some platforms, such as Linux 3.9+, DragonFlyBSD 3.6+, FreeBSD 12.0+, Solaris 11.4, and AIX 7.2.5+. **Default:** `false` |
Comment thread
fengmk2 marked this conversation as resolved.
| debugPort | `Number` | the debug port only listen on http protocol |
| https | `Object` | start a https server, note: `key` / `cert` / `ca` should be full path to file |
| require | `Array\|String` | will inject into worker/agent process |
| pidFile | `String` | will save master pid to this file |
| startMode | `String` | default is 'process', use 'worker_threads' to start the app & agent worker by worker_threads |
| ports | `Array` | startup port of each app worker, such as: [7001, 7002, 7003], only effects when the startMode is 'worker_threads' |
| ports | `Array` | startup port of each app worker, such as: [7001, 7002, 7003], only effects when the `startMode` is `'worker_threads'` and `reusePort` is `false` |
| env | `String` | custom env, default is process.env.EGG_SERVER_ENV |

## Env
Expand Down
2 changes: 1 addition & 1 deletion lib/agent_worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ if (options.startMode === 'worker_threads') {
AgentWorker = require('./utils/mode/impl/process/agent').AgentWorker;
}

const debug = require('util').debuglog('egg-cluster');
const debug = require('util').debuglog('egg-cluster:agent_worker');
const ConsoleLogger = require('egg-logger').EggConsoleLogger;
const consoleLogger = new ConsoleLogger({ level: process.env.EGG_AGENT_WORKER_LOGGER_LEVEL });

Expand Down
36 changes: 31 additions & 5 deletions lib/app_worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ if (options.startMode === 'worker_threads') {
AppWorker = require('./utils/mode/impl/process/app').AppWorker;
}

const os = require('os');
const fs = require('fs');
const debug = require('util').debuglog('egg-cluster');
const debug = require('util').debuglog('egg-cluster:app_worker');
const ConsoleLogger = require('egg-logger').EggConsoleLogger;

const consoleLogger = new ConsoleLogger({
level: process.env.EGG_APP_WORKER_LOGGER_LEVEL,
});
Expand All @@ -38,6 +40,18 @@ const port = options.port = options.port || listenConfig.port;
const debugPort = options.debugPort;
const protocol = (httpsOptions.key && httpsOptions.cert) ? 'https' : 'http';

// https://nodejs.org/api/net.html#serverlistenoptions-callback
// https://github.com/nodejs/node/blob/main/node.gypi#L310
// https://docs.python.org/3/library/sys.html#sys.platform
// This option is available only on some platforms, such as Linux 3.9+, DragonFlyBSD 3.6+, FreeBSD 12.0+, Solaris 11.4, and AIX 7.2.5+.
const supportedPlatforms = [ 'linux', 'freebsd', 'sunos', 'aix' ];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The supportedPlatforms array is missing support for DragonFlyBSD. The README.md mentions it as a supported platform, but os.platform() returns 'dragonfly', which is not in this list. This will prevent reusePort from being enabled on that platform.

Suggested change
const supportedPlatforms = [ 'linux', 'freebsd', 'sunos', 'aix' ];
const supportedPlatforms = [ 'linux', 'freebsd', 'sunos', 'aix', 'dragonfly' ];

let reusePort = options.reusePort = options.reusePort || listenConfig.reusePort;
Comment thread
fengmk2 marked this conversation as resolved.
if (reusePort && !supportedPlatforms.includes(os.platform())) {
reusePort = false;
Comment thread
fengmk2 marked this conversation as resolved.
options.reusePort = false;
debug('platform %s is not supported currently, set reusePort to false', os.platform());
}

AppWorker.send({
to: 'master',
action: 'realport',
Expand Down Expand Up @@ -121,10 +135,22 @@ function startServer(err) {
exitProcess();
return;
}
const args = [ port ];
if (listenConfig.hostname) args.push(listenConfig.hostname);
debug('listen options %s', args);
server.listen(...args);
if (reusePort) {
// https://nodejs.org/api/net.html#serverlistenoptions-callback
const listenOptions = { port, reusePort };
Comment thread
fengmk2 marked this conversation as resolved.
if (listenConfig.hostname) {
listenOptions.host = listenConfig.hostname;
}
debug('listen options %j', listenOptions);
server.listen(listenOptions);
} else {
Comment thread
fengmk2 marked this conversation as resolved.
const args = [ port ];
if (listenConfig.hostname) {
args.push(listenConfig.hostname);
}
debug('listen options %s', args);
server.listen(...args);
}
Comment thread
fengmk2 marked this conversation as resolved.
}
if (debugPortServer) {
debug('listen on debug port: %s', debugPort);
Expand Down
1 change: 1 addition & 0 deletions lib/master.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class Master extends EventEmitter {
* - {Object} [plugins] - customized plugins, for unittest
* - {Number} [workers] numbers of app workers, default to `os.cpus().length`
* - {Number} [port] listening port, default to 7001(http) or 8443(https)
* - {Boolean} [reusePort] setting `reusePort` to `true` allows multiple sockets on the same host to bind to the same port. Incoming connections are distributed by the operating system to listening sockets. This option is available only on some platforms, such as Linux 3.9+, DragonFlyBSD 3.6+, FreeBSD 12.0+, Solaris 11.4, and AIX 7.2.5+. **Default:** `false`.
* - {Number} [debugPort] listening a debug port on http protocol
* - {Object} [https] https options, { key, cert, ca }, full path
* - {Array|String} [require] will inject into worker/agent process
Expand Down
82 changes: 55 additions & 27 deletions lib/utils/mode/impl/worker_threads/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,14 @@ class AppWorker extends BaseAppWorker {
}

class AppUtils extends BaseAppUtils {
#workers = [];
#appWorkers = [];

#forkSingle(appPath, options, id) {
// start app worker
const worker = new workerThreads.Worker(appPath, options);
this.#workers.push(worker);

// wrap app worker
const appWorker = new AppWorker(worker, id);
this.#appWorkers.push(appWorker);
this.emit('worker_forked', appWorker);
appWorker.disableRefork = true;
worker.on('message', msg => {
Expand Down Expand Up @@ -129,23 +128,40 @@ class AppUtils extends BaseAppUtils {
to: 'master',
from: 'app',
});

});

// handle worker exit
worker.on('exit', async code => {
this.log('[master] app_worker#%s (tid:%s) exit with code: %s', appWorker.id, appWorker.workerId, code);
// remove worker from workers array
const idx = this.#appWorkers.indexOf(appWorker);
if (idx !== -1) {
this.#appWorkers.splice(idx, 1);
}
// remove all listeners to avoid memory leak
worker.removeAllListeners();

appWorker.state = 'dead';
this.messenger.send({
action: 'app-exit',
data: {
workerId: appWorker.workerId,
code,
},
to: 'master',
from: 'app',
});
try {
this.messenger.send({
action: 'app-exit',
data: {
workerId: appWorker.workerId,
code,
},
to: 'master',
from: 'app',
});
} catch (err) {
this.log('[master][warning] app_worker#%s (tid:%s) send "app-exit" message error: %s',
appWorker.id, appWorker.workerId, err);
}

if (appWorker.disableRefork) {
return;
}
// refork app worker
this.log('[master] app_worker#%s (tid:%s) refork after 1s', appWorker.id, appWorker.workerId);
await sleep(1000);
this.#forkSingle(appPath, options, id);
});
Comment thread
fengmk2 marked this conversation as resolved.
Expand All @@ -155,26 +171,38 @@ class AppUtils extends BaseAppUtils {
this.startTime = Date.now();
this.startSuccessCount = 0;

const ports = this.options.ports;
if (!ports.length) {
ports.push(this.options.port);
if (this.options.reusePort) {
if (!this.options.port) {
throw new Error('options.port must be specified when reusePort is enabled');
}
for (let i = 0; i < this.options.workers; i++) {
const argv = [ JSON.stringify(this.options) ];
const appWorkerId = i + 1;
this.#forkSingle(this.getAppWorkerFile(), { argv }, appWorkerId);
}
Comment thread
fengmk2 marked this conversation as resolved.
} else {
const ports = this.options.ports;
if (!ports.length) {
ports.push(this.options.port);
}
Comment thread
fengmk2 marked this conversation as resolved.
this.options.workers = ports.length;
let i = 0;
do {
const options = Object.assign({}, this.options, { port: ports[i] });
const argv = [ JSON.stringify(options) ];
this.#forkSingle(this.getAppWorkerFile(), { argv }, ++i);
} while (i < ports.length);
Comment on lines +189 to +194
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The do-while loop used here is a bit unconventional and less readable than a standard for loop for this kind of iteration. Using a for loop would make the intent clearer and improve maintainability.

      for (let i = 0; i < ports.length; i++) {
        const options = Object.assign({}, this.options, { port: ports[i] });
        const argv = [ JSON.stringify(options) ];
        this.#forkSingle(this.getAppWorkerFile(), { argv }, i + 1);
      }

}
this.options.workers = ports.length;
let i = 0;
do {
const options = Object.assign({}, this.options, { port: ports[i] });
const argv = [ JSON.stringify(options) ];
this.#forkSingle(this.getAppWorkerFile(), { argv }, ++i);
} while (i < ports.length);

return this;
}

async kill() {
for (const worker of this.#workers) {
this.log(`[master] kill app worker#${worker.id} (worker_threads) by worker.terminate()`);
worker.removeAllListeners();
worker.terminate();
for (const appWorker of this.#appWorkers) {
this.log('[master] kill app_worker#%s (tid:%s) (worker_threads) by worker.terminate()', appWorker.id, appWorker.workerId);
appWorker.disableRefork = true;
appWorker.instance.removeAllListeners();
appWorker.instance.terminate();
}
}
}
Expand Down
1 change: 1 addition & 0 deletions lib/utils/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = function(options) {
framework: '',
baseDir: process.cwd(),
port: options.https ? 8443 : null,
reusePort: false,
workers: null,
plugins: null,
https: false,
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
{
"name": "egg-cluster",
"version": "2.4.0",
"version": "2.5.0-beta.3",
"description": "cluster manager for egg",
"main": "index.js",
"scripts": {
"lint": "eslint .",
"test": "npm run lint -- --fix && npm run test-local",
"test-local": "egg-bin test --ts false",
"cov": "egg-bin cov --prerequire --timeout 100000 --ts false",
"ci": "npm run lint && npm run cov"
"ci": "npm run lint && node test/reuseport_cluster.js && npm run cov"
Comment thread
fengmk2 marked this conversation as resolved.
},
"files": [
"index.js",
"lib"
],
"repository": {
"type": "git",
"url": "git+https://github.com/eggjs/egg-cluster.git"
"url": "git+https://github.com/eggjs/cluster.git"
},
"keywords": [
"egg",
Expand All @@ -26,9 +26,9 @@
"author": "dead-horse <dead_horse@qq.com>",
"license": "MIT",
"bugs": {
"url": "https://github.com/eggjs/egg-cluster/issues"
"url": "https://github.com/eggjs/cluster/issues"
},
"homepage": "https://github.com/eggjs/egg-cluster#readme",
"homepage": "https://github.com/eggjs/cluster#readme",
"dependencies": {
"await-event": "^2.1.0",
"cfork": "^1.7.1",
Expand Down
29 changes: 29 additions & 0 deletions test/app_worker.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,35 @@
.expect(200);
});

it.only('should set reusePort=true in config', async () => {

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 16)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 16)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 18)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 18)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 14)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 14)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 24)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 24)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 22)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 22)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 16)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 16)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 14)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 14)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 18)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 18)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 24)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 24)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 20)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 20)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 22)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 22)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 20)

it.only not permitted

Check warning on line 235 in test/app_worker.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 20)

it.only not permitted
Comment thread
fengmk2 marked this conversation as resolved.
Outdated
app = utils.cluster('apps/app-listen-reusePort');
app.debug();
await app.ready();

app.expect('code', 0);
app.expect('stdout', /egg started on http:\/\/127.0.0.1:17010/);

await request('http://0.0.0.0:17010')
.get('/')
.expect('done')
.expect(200);

await request('http://127.0.0.1:17010')
.get('/')
.expect('done')
.expect(200);

await request('http://localhost:17010')
.get('/')
.expect('done')
.expect(200);

await request('http://127.0.0.1:17010')
.get('/port')
.expect('17010')
.expect(200);
});

it('should use hostname in config', async () => {
const url = address.ip() + ':17010';

Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/apps/app-listen-reusePort/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
'use strict';

module.exports = app => {
// don't use the port that egg-mock defined
app._options.port = undefined;
};
9 changes: 9 additions & 0 deletions test/fixtures/apps/app-listen-reusePort/app/router.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports = app => {
app.get('/', ctx => {
ctx.body = 'done';
});

app.get('/port', ctx => {
ctx.body = ctx.app._options.port;
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module.exports = {
keys: '123',
cluster: {
listen: {
port: 17010,
reusePort: true,
},
},
};
3 changes: 3 additions & 0 deletions test/fixtures/apps/app-listen-reusePort/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "app-listen-reusePort"
}
12 changes: 12 additions & 0 deletions test/master.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,18 @@
.end(done);
});

it.only('start success with reusePort=true', done => {

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 16)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 16)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 18)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 18)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 14)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 14)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 24)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 24)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 22)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 22)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 16)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 16)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 14)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 14)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 18)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 18)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 24)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 24)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 20)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 20)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 22)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (macos-latest, 22)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 20)

it.only not permitted

Check warning on line 32 in test/master.test.js

View workflow job for this annotation

GitHub Actions / Node.js / Test (ubuntu-latest, 20)

it.only not permitted
mm.env('local');
app = utils.cluster('apps/master-worker-started', { reusePort: true });
app.debug();

app.expect('stdout', /egg start/)
.expect('stdout', /egg started/)
.notExpect('stdout', /\[master\] agent_worker#1:\d+ start with clusterPort:\d+/)
.expect('code', 0)
.end(done);
});
Comment thread
fengmk2 marked this conversation as resolved.
Outdated

it('start success in prod env', done => {
mm.env('prod');
app = utils.cluster('apps/mock-production-app').debug(false);
Expand Down
Loading
Loading