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

Drop HTTP client connection pool's acquire queue #716

Open
DerGuteMoritz opened this issue Feb 16, 2024 · 1 comment
Open

Drop HTTP client connection pool's acquire queue #716

DerGuteMoritz opened this issue Feb 16, 2024 · 1 comment

Comments

@DerGuteMoritz
Copy link
Collaborator

DerGuteMoritz commented Feb 16, 2024

As per the discussion on #713, it might make sense to drop the HTTP client connection pool's acquire queue to simplify both implementation (which may result in better performance) and API surface (i.e. no need for dealing with :pool-timeout and PoolTimeoutException anymore). Quoting @alexander-yakushev from that discussion for rationale:

I found that in practice (in my work, I can be biased), Aleph's approach to having a separate queue for acquiring connections is unnecessary. Caching connections is vital because you can't keep creating and disposing them at a high rate (due to TIME_WAIT), but limiting the total number of them is not as important. I personally set the connection limit to 16k per host and pool queue size to 0, so that a fresh connection is immediately created for the acquirer each time there are no free connections in the pool, up until the limit is hit at which point it's an error.

There are some cases where you would use the pool with the queue as a parallelism controller. I think all those cases could be rewritten to something like Semaphores if Aleph connection pools offered no queueing.

So, this is my take. In my world, the breaking Aleph2 would have connection pools with no acquisition queues. Such pools are much much easier to implement, way more performant at high loads, and give the user one less option and timeout to think about. But I understand that people can have other usecases where this won't fly.

To reduce impact of such a change, we could keep around the old pool implementation (perhaps in a separate namespace which isn't loaded by default?) so that affected users can swap it back in.

@DerGuteMoritz
Copy link
Collaborator Author

@alexander-yakushev Can you recommend a robust and performant existing pool implementation or would you indeed roll a custom one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants