node-ldapjs icon indicating copy to clipboard operation
node-ldapjs copied to clipboard

Multithreaded Server?

Open CoryGH opened this issue 3 years ago • 5 comments

Reading through the API and the code I'm not seeing an obvious way to do it, so I'm writing to see if there's a way that would be recommended by people better familiar with the architectural design of ldapjs. Is it possible to run ldapjs as a server with multiple threads? My ideal would probably be something akin to the express connection callback wherein a new connection forms and is sent off to another thread with threads pre-built using something like node.js's cluster library (plus I already have a thread manager using this model for the application I'd like to use ldapjs in.)

CoryGH avatar May 19 '21 15:05 CoryGH

Upon further review it looks like this could be achieved with a hook into the newConnection callback of the createServer calls in lib/server.js:

if ((options.cert || options.certificate) && options.key) {
    options.cert = options.cert || options.certificate
    this.server = tls.createServer(options, newConnection)
} else {
    this.server = net.createServer(newConnection)
}

CoryGH avatar May 19 '21 17:05 CoryGH

Created pull request https://github.com/ldapjs/node-ldapjs/pull/727 to submit a change for this.

CoryGH avatar May 19 '21 18:05 CoryGH

I don't think this is a good idea. While the docs do not indicate it (https://nodejs.org/dist/latest-v14.x/docs/api/cluster.html), people I know that work on Node core do not recommend the cluster API to be used.

jsumners avatar May 19 '21 19:05 jsumners

I think worker threads were supposed to fix most of the issues in node cluster.

UziTech avatar May 19 '21 20:05 UziTech

I don't think this is a good idea. While the docs do not indicate it (https://nodejs.org/dist/latest-v14.x/docs/api/cluster.html), people I know that work on Node core do not recommend the cluster API to be used.

It would be required regardless of the mode of multithreading you're using unless you're arraying node instances of the app created on a port for every thread and using some other tool to route traffic at the network level between them for load balancing. Having a hook strikes me as cleaner and affords more freedom for development (e.g. not requiring the server to be single-threaded.)

CoryGH avatar May 19 '21 20:05 CoryGH

👋

On February 22, 2023, we released version 3 of this library. As a result, we are closing this issue/pull request.

Please see issue #839 for more information, including how to proceed if you feel this closure is in error.

jsumners avatar Feb 22 '23 19:02 jsumners