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

[FEATURE] Consistent connection pooling setting #844

Closed
samypr100 opened this issue Nov 10, 2024 · 1 comment · Fixed by #845
Closed

[FEATURE] Consistent connection pooling setting #844

samypr100 opened this issue Nov 10, 2024 · 1 comment · Fixed by #845
Labels
enhancement New feature or request

Comments

@samypr100
Copy link
Contributor

samypr100 commented Nov 10, 2024

Is your feature request related to a problem?

When using OpenSearch client, setting pool_maxsize allows you to increase the connection pool size as shown in the documentation guides.

The same is not true when using AsyncOpenSearch and it seems pool_maxsize does not have the desired effect.

The argument has to be named maxsize for it to work and take effect as shown in

maxsize: Optional[int] = 10,

What solution would you like?

I would like pool_maxsize to work for AsyncOpenSearch, e.g. by also setting maxsize internally.

What alternatives have you considered?

Right now, I instantiate OpenSearch/AsyncOpenSearch clients with both pool_maxsize and maxsize set to ensure consistent settings, but its undesired from a DevEx perspective to have to use one or the other when moving between sync and async.

Do you have any additional context?

Debugger can show self._limit set to 10 (the default) if pool_maxsize is set when creating a new instance of AsyncOpenSearch. The value can be changed only if maxsize is passed instead to AsyncOpenSearch.

connector=aiohttp.TCPConnector(
limit=self._limit, use_dns_cache=True, ssl=self._ssl_context
),

@samypr100 samypr100 added enhancement New feature or request untriaged Need triage labels Nov 10, 2024
@dblock
Copy link
Member

dblock commented Nov 11, 2024

Thanks for reporting this, please do contribute a fix. Generally I'd want a lot of the setup code to be refactored so that we don't have differences between sync/async for anything that can be set in both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants