Skip to content
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

Plans for ConnectionPool beyond SingleNodeConnectionPool? #197

Open
mooreniemi opened this issue Oct 26, 2023 · 3 comments
Open

Plans for ConnectionPool beyond SingleNodeConnectionPool? #197

mooreniemi opened this issue Oct 26, 2023 · 3 comments
Labels
enhancement New feature or request performance Make it fast.

Comments

@mooreniemi
Copy link

/// to get the next [Connection]. The simplest type of [ConnectionPool] is [SingleNodeConnectionPool],

I'm a bit confused that the only existing implementation vended is the SingleNodeConnectionPool. Is the intent to create a client per request, or to have users manage a pool of clients, or implement connection pools that actually pool connections?

@Xtansia
Copy link
Collaborator

Xtansia commented Oct 29, 2023

@mooreniemi The naming of ConnectionPool is a bit of a misnomer as it doesn't itself handle "connection pooling" in the sense of keeping network resources around for re-use. It just manages the set of nodes to connect to, which in this case is a "single node". For the cases of cloud hosted versions such as Amazon Managed OpenSearch, they put all nodes behind a single load balancer, which means from the clients perspective there's only a single endpoint/node available. Having an alternative implementation that takes an array of node endpoints and round-robins them or tracks their health would be beneficial for self-hosted clusters where there's no load balancer. There's currently no plan or timeline for implementing such yet, but if someone were to PR something in that direction it'd be gladly appreciated.

We use reqwest::Client which itself implements an actual connection pool, it is recommended to keep a single long-lived client that you reuse rather than constructing for each request or similar.

@mooreniemi
Copy link
Author

Ah, I should have read further in the code, I'm sorry!

Here I see connection is just used to get the url passed in (and nothing more):

let connection = self.conn_pool.next();
let url = connection.url.join(path.trim_start_matches('/'))?;

I understand how in a context where a load balancer is not giving me a single url I would want to change implementation of next for my use case.

Thanks for explaining! It might still be good to offer an example implementation of a multi-node case or to update the documentation here.

@dblock dblock added the enhancement New feature or request label Dec 4, 2023
@dblock
Copy link
Member

dblock commented Dec 4, 2023

@mooreniemi Want to contribute a sample? We've been adding those in other clients, e.g. https://github.com/opensearch-project/opensearch-py/tree/main/samples, along with guides.

@dblock dblock added the performance Make it fast. label Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance Make it fast.
Projects
None yet
Development

No branches or pull requests

3 participants