Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Multithreaded Server? #726

Closed
CoryGH opened this issue May 19, 2021 · 6 comments
Closed

Multithreaded Server? #726

CoryGH opened this issue May 19, 2021 · 6 comments

Comments

@CoryGH
Copy link
Contributor

CoryGH commented May 19, 2021

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
Copy link
Contributor Author

CoryGH commented May 19, 2021

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
Copy link
Contributor Author

CoryGH commented May 19, 2021

Created pull request #727 to submit a change for this.

@jsumners
Copy link
Member

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.

@UziTech
Copy link
Member

UziTech commented May 19, 2021

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

@CoryGH
Copy link
Contributor Author

CoryGH commented May 19, 2021

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.)

@jsumners
Copy link
Member

👋

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.

@ldapjs ldapjs locked as resolved and limited conversation to collaborators Feb 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants