-
Notifications
You must be signed in to change notification settings - Fork 153
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
Add oncreated worker #481
base: master
Are you sure you want to change the base?
Add oncreated worker #481
Conversation
Thanks @GwiYeong. Thinking aloud here: can we already achieve this already by using events? Would it be possible to let a worker send it's pid to the main process via |
@josdejong Thank you for responding. |
You can indeed only send/receive // myWorker.js
var workerpool = require('workerpool')
var initialized = false
function wrapperSendPidOnce(fn) {
return (...args) => {
if (!initialized) {
initialized = true
workerpool.workerEmit({ type: 'created', pid: process.pid })
}
return fn(...args)
}
}
function add(a, b) {
return a + b
}
function multiply(a, b) {
return a * b
}
workerpool.worker({
add: wrapperSendPidOnce(add),
multiply: wrapperSendPidOnce(multiply)
}) And then use the worker like so: // main.js
var workerpool = require('workerpool')
var pool = workerpool.pool(__dirname + '/myWorker.js')
function execWithEventListener(method, args) {
function handleEvent(event) {
console.log('Event: ', event)
}
return pool.exec(method, args, { on: handleEvent })
}
async function run() {
console.log(await execWithEventListener('add', [2, 3]))
console.log(await execWithEventListener('add', [7, 2]))
}
run()
.catch(function (err) {
console.error(err)
})
.finally(function () {
pool.terminate()
}) Running this example wil output:
Would that address your case? |
@josdejong yes. I understand what you are saying. |
This is an interesting idea, I agree that since there is an
What do you mean by first time? |
I mean the solution that @josdejong gave to me achieve my goal, but it seems a bit complicated. |
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.
Thanks @GwiYeong, and sorry for the late reply.
I've been thinking about it and you're right, the workaround that I posted is quite complicated. Also, adding support for an onCreatedWorker
option is only a few lines of code, so not much to worry there. So, let's go for it 😄.
I made a couple of inline comments, can you have a look at those?
@@ -431,6 +433,8 @@ Pool.prototype._createWorkerHandler = function () { | |||
workerTerminateTimeout: this.workerTerminateTimeout, | |||
emitStdStreams: this.emitStdStreams, | |||
}); | |||
this.onCreatedWorker(worker) |
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.
If we pass the WorkerHandler
to onCreatedWorker
, we pass a fully undocumented class. How about passing the Worker
instead (ie. this.onCreatedWorker(worker.worker)
?
workerType: "process", | ||
workerTerminateTimeout: 1000, | ||
onCreatedWorker: (worker) => { | ||
const cpuLimitOption = { |
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.
Thanks, this is a nice example!
Can you please change the indentation to 2 spaces? (I know, I should set up a linter)
@@ -208,6 +208,7 @@ The following options are available: | |||
- `workerOpts: Object`: the `workerOpts` option of this pool | |||
- `script: string`: the `script` option of this pool | |||
Optionally, this callback can return an object containing one or more of the above properties. The provided properties will be used to override the Pool properties for the worker being created. | |||
- `onCreatedWorker: Function`. A callback that is called whenever a worker is created. For example, it can be used to add a limit, such as a cpu limit, for each created worker. |
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.
Can you write a unit test for the new option please?
@GwiYeong did you see my last reply? |
I wanted to add a cpu limit for the worker process created when utilizing workerpool in process mode.
However, the current callback doesn't know the pid of the created worker.
This PR adds a callback to get the information of the created worker after the worker is created.
I added a callback called
onCreatedWorker
, which is passed the currently created WorkerHandler directly.If it's a process worker, I can get the created pid via
worker.worker.pid
.The pid can then be used to limit resources, such as cpu limit.
For cpulimit worker, I add an example.