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.

Sign up for GitHub

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

Jump to bottom

There should be some kind of rate-limiting on getaddrinfo threads #10

Closed
njsmith opened this issue Jan 22, 2017 · 2 comments
Closed

There should be some kind of rate-limiting on getaddrinfo threads #10

njsmith opened this issue Jan 22, 2017 · 2 comments
Labels
polish

Comments

@njsmith
Copy link
Member

njsmith commented Jan 22, 2017

trio.socket.getaddrinfo (and some of its friends, like socket.resolve_*_address) internally spawn a thread. Right now there's no limited on this, so if you spawn 1000 tasks and they each make a socket connection, you'll briefly get 1000 threads at once. Probably it would be better if you instead, got, like, 20 threads at once, 50 times. Or maybe not, I haven't tried it!

The obvious thing would be to make a global (well, thread-local) semaphore and require getaddrinfo to hold it when calling run_in_worker_thread. Not so obvious how to allow the number of threads to be tuned etc. though.

See also: #6
@njsmith njsmith added the polish label Jan 22, 2017
@njsmith njsmith mentioned this issue Jan 22, 2017
Closed
@njsmith
Copy link
Member Author

njsmith commented Feb 21, 2017 •
edited
Loading

This depends on #57

Probably the easy way to allow thread count tuning is to provide current_getaddrinfo_parallelism, adjust_getaddrinfo_parallelism type functions in trio.socket, that issue the appropriate release/acquire calls. (This might need to allow for semaphores to become negative? The other option is that adjusting down could block, but that seems a bit annoying and pointless? or maybe it's fine idk)

@njsmith njsmith mentioned this issue Mar 1, 2017
Closed
@njsmith
Copy link
Member Author

njsmith commented May 24, 2017

This should probably share a default limit with the threads used for #20

njsmith added a commit to njsmith/trio that referenced this issue Jun 18, 2017
@njsmith
Implement capacity limitation for threads
2cf6081 
- New synchronization primitive: CapacityLimiter. Like a Semaphore but
  more specialized. See python-triogh-182 for rationale.

- Add limiter= argument to run_in_worker_thread, that allows one to
  correctly (modulo
  python-trio#6 (comment))
  control the number of active threads.

- Added new function current_default_worker_thread_limiter(), which
  creates or returns a run-local CapacityLimiter, and made
  run_in_worker_thread use it by default when no other limiter= is
  given.

Closes: python-triogh-10, python-triogh-57, python-triogh-156
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
polish
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@njsmith