Skip to content
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

feat: allow worker listen different port to support nginx sticky #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
55 changes: 37 additions & 18 deletions lib/app_worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,11 @@ const fs = require('fs');
const debug = require('debug')('egg-cluster');
const gracefulExit = require('graceful-process');
const ConsoleLogger = require('egg-logger').EggConsoleLogger;
const consoleLogger = new ConsoleLogger({
level: process.env.EGG_APP_WORKER_LOGGER_LEVEL,
});
const net = require('net');
const consoleLogger = new ConsoleLogger({ level: process.env.EGG_APP_WORKER_LOGGER_LEVEL });
const Application = require(options.framework).Application;
debug('new Application with options %j', options);
const app = new Application(options);
const clusterConfig = app.config.cluster || /* istanbul ignore next */ {};
const listenConfig = clusterConfig.listen || /* istanbul ignore next */ {};
const httpsOptions = Object.assign({}, clusterConfig.https, options.https);
const port = options.port = options.port || listenConfig.port;
const protocol = (httpsOptions.key && httpsOptions.cert) ? 'https' : 'http';

process.send({
to: 'master',
action: 'realport',
data: {
port,
protocol,
},
});
Copy link
Member

Choose a reason for hiding this comment

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

Why delete this one?


app.ready(startServer);

Expand All @@ -57,6 +42,12 @@ function startServer(err) {
exitProcess();
return;
}
const clusterConfig = app.config.cluster || /* istanbul ignore next */ {};
const listenConfig = clusterConfig.listen || /* istanbul ignore next */ {};
const port = options.port = options.port || listenConfig.port;
const httpsOptions = Object.assign({}, clusterConfig.https, options.https);
const protocol = (httpsOptions.key && httpsOptions.cert) ? 'https' : 'http';
process.send({ to: 'master', action: 'realport', data: { port, protocol } });

app.removeListener('startTimeout', startTimeoutHandler);

Expand All @@ -81,7 +72,9 @@ function startServer(err) {
app.emit('server', server);

if (options.sticky) {
server.listen(options.stickyWorkerPort, '127.0.0.1');
const args = [ options.stickyWorkerPort ];
if (listenConfig.hostname) args.push(listenConfig.hostname);
server.listen(...args);
// Listen to messages sent from the master. Ignore everything else.
process.on('message', (message, connection) => {
if (message !== 'sticky-session:connection') {
Expand All @@ -108,6 +101,32 @@ function startServer(err) {
server.listen(...args);
}
}
server.on('listening', () => {
// same as cluster.listening event
// https://github.com/nodejs/node/blob/master/lib/internal/cluster/child.js#L76-L104
// const address = server.address();
let addressType = 4;
let fd = null;
let address = null;
if (listenConfig.path) {
addressType = -1;
fd = server.address().fd;
address = listenConfig.path;
} else if (listenConfig.hostname) {
const isIp = net.isIP(listenConfig.hostname);
addressType = isIp === 0 ? 4 : isIp;
address = listenConfig.hostname;
}
app.messenger.send('app-start', {
workerPid: process.pid,
address: {
addressType,
address,
port,
fd,
},
}, 'master');
});
}

gracefulExit({
Expand Down
49 changes: 17 additions & 32 deletions lib/master.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const terminate = require('./utils/terminate');
const PROTOCOL = Symbol('Master#protocol');
const REAL_PORT = Symbol('Master#real_port');
const APP_ADDRESS = Symbol('Master#appAddress');
const WORKER_ADDRESSES = Symbol('Master#workerAddresses');

class Master extends EventEmitter {

Expand All @@ -38,6 +39,7 @@ class Master extends EventEmitter {
* - {Object} [https] https options, { key, cert, ca }, full path
* - {Array|String} [require] will inject into worker/agent process
* - {String} [pidFile] will save master pid to this file
* - {boolean} [nginxSticky] - use nginx sticky mode, default false
*/
constructor(options) {
super();
Expand All @@ -60,6 +62,7 @@ class Master extends EventEmitter {
if (process.env.EGG_SERVER_ENV === 'local' || process.env.NODE_ENV === 'development') {
this.logMethod = 'debug';
}
this[WORKER_ADDRESSES] = [];

// get the real framework info
const frameworkPath = this.options.framework;
Expand All @@ -84,9 +87,14 @@ class Master extends EventEmitter {

this.ready(() => {
this.isStarted = true;
const stickyMsg = this.options.sticky ? ' with STICKY MODE!' : '';
this.logger.info('[master] %s started on %s (%sms)%s',
frameworkPkg.name, this[APP_ADDRESS], Date.now() - startTime, stickyMsg);
if (this.options.nginxSticky) {
this.logger.info('[master] %s started on %j (%sms) with NGINX STICKY MODE!',
frameworkPkg.name, this[WORKER_ADDRESSES].map(getAddress), Date.now() - startTime);
} else {
const stickyMsg = this.options.sticky ? ' with STICKY MODE!' : '';
this.logger.info('[master] %s started on %s (%sms)%s',
frameworkPkg.name, this[APP_ADDRESS], Date.now() - startTime, stickyMsg);
}

Copy link
Member

Choose a reason for hiding this comment

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

What's the different between nginxSticky and sticky? Seems they are mixed together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • nginxSticky: nginx -> app-worker
  • sticky: nginx -> master -> app-worker

const action = 'egg-ready';
this.messenger.send({
Expand Down Expand Up @@ -361,17 +369,6 @@ class Master extends EventEmitter {
from: 'app',
});
});
cluster.on('listening', (worker, address) => {
this.messenger.send({
action: 'app-start',
data: {
workerPid: worker.process.pid,
address,
},
to: 'master',
from: 'app',
});
});
Copy link
Member

Choose a reason for hiding this comment

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

Why remove this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

send port to master after egg ready and before server listen.

}

/**
Expand Down Expand Up @@ -547,16 +544,12 @@ class Master extends EventEmitter {
onAppStart(data) {
const worker = this.workerManager.getWorker(data.workerPid);
const address = data.address;

// worker should listen stickyWorkerPort when sticky mode
if (this.options.sticky) {
if (String(address.port) !== String(this.options.stickyWorkerPort)) {
return;
}
// worker should listen REALPORT when not sticky mode
} else if (!isUnixSock(address)
&& (String(address.port) !== String(this[REAL_PORT]))) {
return;
address.protocol = this[PROTOCOL];
if (this.options.nginxSticky) {
this[WORKER_ADDRESSES].push(address);
} else {
address.port = this.options.sticky ? this[REAL_PORT] : address.port;
this[APP_ADDRESS] = getAddress(address);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why delete isUnixSock checking?


// send message to agent with alive workers
Expand Down Expand Up @@ -598,10 +591,6 @@ class Master extends EventEmitter {
worker.disableRefork = false;
}

address.protocol = this[PROTOCOL];
address.port = this.options.sticky ? this[REAL_PORT] : address.port;
this[APP_ADDRESS] = getAddress(address);

if (this.options.sticky) {
this.startMasterSocketServer(err => {
if (err) return this.ready(err);
Expand Down Expand Up @@ -720,7 +709,3 @@ function getAddress({
const hostname = address || '127.0.0.1';
return `${protocol}://${hostname}:${port}`;
}

function isUnixSock(address) {
return address.addressType === -1;
}
2 changes: 1 addition & 1 deletion test/fixtures/apps/app-listen-hostname/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module.exports = app => {
// don't use the port that egg-mock defined
app._options.port = undefined;
app.options.port = undefined;
};
2 changes: 1 addition & 1 deletion test/fixtures/apps/app-listen-path/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module.exports = app => {
// don't use the port that egg-mock defined
app._options.port = undefined;
app.options.port = undefined;
};
2 changes: 1 addition & 1 deletion test/fixtures/apps/app-listen-port/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module.exports = app => {
// don't use the port that egg-mock defined
app._options.port = undefined;
app.options.port = undefined;
};
2 changes: 1 addition & 1 deletion test/fixtures/apps/app-listen-port/app/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ module.exports = app => {
});

app.get('/port', function* () {
this.body = this.app._options.port;
this.body = this.app.options.port;
});
};
2 changes: 1 addition & 1 deletion test/fixtures/apps/app-listen-without-port/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

module.exports = app => {
// don't use the port that egg-mock defined
app._options.port = undefined;
app.options.port = undefined;
};
6 changes: 6 additions & 0 deletions test/fixtures/apps/cluster_mod_nginx_sticky/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;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use strict';

exports.index = function* () {
this.body = 'hi cluster';
};
9 changes: 9 additions & 0 deletions test/fixtures/apps/cluster_mod_nginx_sticky/app/router.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict';

module.exports = function(app) {
// GET / 302 to /portal/i.htm
app.redirect('/', '/portal/i.htm', 302);

// GET /portal/i.htm => controllers/home.js
app.get('/portal/i.htm', app.controller.home.index);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

const { worker } = require('cluster');

module.exports = {
keys: '123',
cluster: {
listen: {
port: worker ? 17010 + worker.id : 17010,
},
},
};
3 changes: 3 additions & 0 deletions test/fixtures/apps/cluster_mod_nginx_sticky/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "cluster_mod_nginx_sticky"
}
2 changes: 1 addition & 1 deletion test/fixtures/apps/cluster_mod_sticky/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const net = require('net');

module.exports = app => {
// don't use the port that egg-mock defined
app._options.port = undefined;
app.options.port = undefined;
const server = net.createServer();
server.listen(9500);

Expand Down
Empty file added test/index.test.js
Empty file.
29 changes: 27 additions & 2 deletions test/master.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,7 @@ describe('test/master.test.js', () => {
it('should every app worker will get message', function* () {
yield sleep(1000);
// start two workers
app.expect('stdout', /#1 agent get 1 workers \[ \d+ \]/);
app.expect('stdout', /#2 agent get 2 workers \[ \d+, \d+ \]/);
app.expect('stdout', /agent get 2 workers \[ \d+, \d+ \]/);
});

it('agent should get update message after app died', function* () {
Expand Down Expand Up @@ -837,6 +836,32 @@ describe('test/master.test.js', () => {
});
});

describe('--nginx-sticky', () => {
before(() => {
app = utils.cluster('apps/cluster_mod_nginx_sticky', {
nginxSticky: true,
workers: 2,
});
app.debug();
return app.ready();
});

after(() => app.close());

it('should listen multi port', function* () {
app.expect('stdout', /\[master] egg started on \["http:\/\/127\.0\.0\.1:17011|2","http:\/\/127\.0\.0\.1:17012|1"]/);
const urls = [
'http://127.0.0.1:17011',
'http://127.0.0.1:17012',
];
yield Promise.all(urls.map(t =>
request(t).get('/portal/i.htm')
.expect('hi cluster')
.expect(200)
));
});
});

describe('agent and worker exception', () => {
it('should not exit when local env', function* () {
mm.env('local');
Expand Down