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

Change connector's limit parameter #977

Closed
asvetlov opened this issue Jul 20, 2016 · 5 comments
Closed

Change connector's limit parameter #977

asvetlov opened this issue Jul 20, 2016 · 5 comments

Comments

@asvetlov
Copy link
Member

Now it is None by default (infinite amount of parallel connections).

I suggest shrinking the default to large enough but limited value.

The main question is: what the value should be? 10? 100? 1000?
The only thought about default: it should correlate with default number of available file descriptors, which is 1024.

Thus we have a choice between 10-50-100.

Any suggestions?

@ludovic-gasc
Copy link
Contributor

Maybe we could have 900 as default value to be safe, and a warning message when the limit is reached ?
To help the users to spot the bottlenecks. It should be sad they use Go only because they don't change this value.

@tailhook
Copy link
Contributor

Maybe we could have 900 as default value to be safe

It's not too safe because the limit is applied on a single host (right?). And there is no limit on number of hosts you can connect using a single session and a number of sessions you can create.

So I would prefer something at the order of 10 (In fact I would prefer exactly 1 by default, but it's not an easy thing to cope with for most users)

@rmariano
Copy link
Contributor

As agreed with @asvetlov , I will start setting up a limit of 20 connections by default, but letting this to be changed by passing a new parameter.

asvetlov pushed a commit that referenced this issue Jul 23, 2016
* Git ignore `*.swp` files

* Allow `BaseConnector` to be used as a context manager

* Set default limit connection to 20

The default was changed from `None` to 20. `None` used to mean that
there were no limits at all, so all number of connections were allowed
to the same endpoint.

Not, when the user does not specify, the default is 20, but it is still
possible to set `None`.

#977

* Update documentation for the `limit` parameter.

In the docs, document the new behaviour of the `limit` parameter, which
now defaults to 20, instead of `None`.

* Fit doc text in 80 cols.
@asvetlov
Copy link
Member Author

Fixed by #991

@lock
Copy link

lock bot commented Oct 29, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Oct 29, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants