josdejong/workerpool

MaxListenersExceededWarning with worker_threads in node.js

Michsior14 opened this issue · 5 comments

Michsior14 commented

Currently there is no way to set max listeners (10 by default) for worker_thread, thus if you have high load across more than 10 threads the warning will popup.

I am interested in fixing this upstream either by setting the listeners value to higher value (e.g. 100) by default or providing the new option for WorkerHandler. What would be your preference?

josdejong commented

Thanks. Some questions/thoughts:

  1. Is there a downside to just setting a higher value like 100?
  2. Is this something users will want to fiddle with?
  3. Can we automatically set max listeners according to the configured maxWorkers option?
Michsior14 commented
  1. Could be harder to detect potential memory leak, although I saw similar fixes for worker implementations.
  2. Possibly yes, in some cases it can narrow down the place where leak could occur.
  3. Maybe, I would need to check a bit more the implementation to make sure it's feasible (e.g. are the listeners cleared/shared across the worker calls).
josdejong commented

Ok so it makes sense to have the setting configurable, like for debugging memory leaks. It would be nice if we can reuse maxWorkers for this I think, please let me know if you think that will work.

Michsior14 commented

Uhhh, it turned out that it's not workerpool fault and everything works as expected here (only one listener for parent and one for child thread) . The issue was with sentry initialization during job execution. Sorry!

josdejong commented

👍