-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
WIP: cluster: make scheduler configurable #11546
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,17 +2,19 @@ | |
const assert = require('assert'); | ||
const net = require('net'); | ||
const { sendHelper } = require('internal/cluster/utils'); | ||
const create = Object.create; | ||
const getOwnPropertyNames = Object.getOwnPropertyNames; | ||
const uv = process.binding('uv'); | ||
|
||
module.exports = RoundRobinHandle; | ||
|
||
function RoundRobinHandle(key, address, port, addressType, fd) { | ||
function RoundRobinHandle(key, address, port, addrType, fd, flags, scheduler) { | ||
this.key = key; | ||
this.all = {}; | ||
this.free = []; | ||
this.all = create(null); | ||
this.workers = []; | ||
this.handles = []; | ||
this.handle = null; | ||
this.scheduler = scheduler; | ||
this.server = net.createServer(assert.fail); | ||
|
||
if (fd >= 0) | ||
|
@@ -30,6 +32,17 @@ function RoundRobinHandle(key, address, port, addressType, fd) { | |
}); | ||
} | ||
|
||
RoundRobinHandle.scheduler = function(workers) { | ||
const worker = workers.shift(); | ||
|
||
if (worker === undefined) | ||
return; | ||
|
||
workers.push(worker); // Add to the back of the ready queue. | ||
return worker; | ||
}; | ||
|
||
|
||
RoundRobinHandle.prototype.add = function(worker, send) { | ||
assert(worker.id in this.all === false); | ||
this.all[worker.id] = worker; | ||
|
@@ -44,7 +57,8 @@ RoundRobinHandle.prototype.add = function(worker, send) { | |
send(null, null, null); // UNIX socket. | ||
} | ||
|
||
this.handoff(worker); // In case there are connections pending. | ||
this.workers.push(worker); | ||
this.handoff(); // In case there are connections pending. | ||
}; | ||
|
||
if (this.server === null) | ||
|
@@ -66,10 +80,10 @@ RoundRobinHandle.prototype.remove = function(worker) { | |
return false; | ||
|
||
delete this.all[worker.id]; | ||
const index = this.free.indexOf(worker); | ||
const index = this.workers.indexOf(worker); | ||
|
||
if (index !== -1) | ||
this.free.splice(index, 1); | ||
this.workers.splice(index, 1); | ||
|
||
if (getOwnPropertyNames(this.all).length !== 0) | ||
return false; | ||
|
@@ -84,32 +98,46 @@ RoundRobinHandle.prototype.remove = function(worker) { | |
|
||
RoundRobinHandle.prototype.distribute = function(err, handle) { | ||
this.handles.push(handle); | ||
const worker = this.free.shift(); | ||
|
||
if (worker) | ||
this.handoff(worker); | ||
this.handoff(); | ||
}; | ||
|
||
RoundRobinHandle.prototype.handoff = function(worker) { | ||
if (worker.id in this.all === false) { | ||
return; // Worker is closing (or has closed) the server. | ||
RoundRobinHandle.prototype.handoff = function() { | ||
const handle = this.handles[0]; | ||
var socket; | ||
|
||
// There are currently no requests to schedule. | ||
if (handle === undefined) | ||
return; | ||
|
||
if (this.scheduler.exposeSocket === true) { | ||
socket = new net.Socket({ | ||
handle, | ||
readable: false, | ||
writable: false, | ||
pauseOnCreate: true | ||
}); | ||
} | ||
|
||
const handle = this.handles.shift(); | ||
const worker = this.scheduler(this.workers, socket); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this .slice() or should there be a note in the documentation that the callee is not allowed to modify the workers array? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docs currently say:
I think slicing would reduce the chance of shooting yourself in the foot, but it would make it more work to do something like round robin because we modify the array order in the scheduler. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but that could be interpreted as meaning that workers.sort() is okay. I'd add a stronger prohibition. |
||
|
||
if (handle === undefined) { | ||
this.free.push(worker); // Add to ready queue again. | ||
// An invalid worker was returned, or the worker is closing the server. | ||
if (worker === null || typeof worker !== 'object' || | ||
worker.id in this.all === false) { | ||
return; | ||
} | ||
|
||
this.handles.shift(); // Successfully scheduled, so dequeue. | ||
|
||
const message = { act: 'newconn', key: this.key }; | ||
|
||
sendHelper(worker.process, message, handle, (reply) => { | ||
if (reply.accepted) | ||
if (reply.accepted) { | ||
handle.close(); | ||
else | ||
this.distribute(0, handle); // Worker is shutting down. Send to another. | ||
} else { | ||
// Worker is shutting down. Send the handle to another worker. | ||
this.distribute(0, handle); | ||
} | ||
|
||
this.handoff(worker); | ||
this.handoff(); | ||
}); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
'use strict'; | ||
const common = require('../common'); | ||
const assert = require('assert'); | ||
const cluster = require('cluster'); | ||
const http = require('http'); | ||
const net = require('net'); | ||
|
||
if (cluster.isMaster) { | ||
const numWorkers = 2; | ||
const pattern = [2, 1, 2, 2, 1, 2, 1, 1, 2]; | ||
let index = 0; | ||
let readyCount = 0; | ||
|
||
// The scheduler moves through pattern. Each request is scheduled to the | ||
// worker id specified in the current pattern index. | ||
function scheduler(workers, socket) { | ||
const id = pattern[index]; | ||
const worker = workers.filter((w) => { | ||
return w.id === id; | ||
}).pop(); | ||
|
||
if (id === 2) { | ||
assert.strictEqual(scheduler.exposeSocket, true); | ||
assert(socket instanceof net.Socket); | ||
} else { | ||
assert.strictEqual(scheduler.exposeSocket, false); | ||
assert.strictEqual(socket, undefined); | ||
} | ||
|
||
if (worker !== undefined) | ||
index++; | ||
|
||
return worker; | ||
} | ||
|
||
// Create a getter for exposeSocket. If the current item in the pattern is 2, | ||
// then expose the socket. Otherwise, hide it. | ||
Object.defineProperty(scheduler, 'exposeSocket', { | ||
get() { return pattern[index] === 2; } | ||
}); | ||
|
||
function onMessage(msg) { | ||
// Once both workers send a 'ready' signal, indicating that their servers | ||
// are listening, begin making HTTP requests. | ||
assert.strictEqual(msg.cmd, 'ready'); | ||
readyCount++; | ||
|
||
if (readyCount === numWorkers) | ||
makeRequest(0, msg.port); | ||
} | ||
|
||
function makeRequest(reqCount, port) { | ||
// Make one request for each element in pattern and then shut down the | ||
// workers. | ||
if (reqCount >= pattern.length) { | ||
for (const id in cluster.workers) | ||
cluster.workers[id].disconnect(); | ||
|
||
return; | ||
} | ||
|
||
http.get({ port }, (res) => { | ||
res.on('data', (data) => { | ||
assert.strictEqual(+data.toString(), pattern[reqCount]); | ||
reqCount++; | ||
makeRequest(reqCount, port); | ||
}); | ||
}); | ||
} | ||
|
||
cluster.setupMaster({ scheduler }); | ||
|
||
for (let i = 0; i < numWorkers; i++) | ||
cluster.fork().on('message', common.mustCall(onMessage)); | ||
|
||
} else { | ||
const server = http.createServer((req, res) => { | ||
res.end(cluster.worker.id + ''); | ||
}).listen(0, () => { | ||
process.send({ cmd: 'ready', port: server.address().port }); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readable = writable = false. Is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I figured that the socket should only be used to determine where to pass the handle, and not actually read or written.