Skip to content

Conversation

mgreene
Copy link

@mgreene mgreene commented Jul 10, 2013

Purpose

While having the ThrottledRequestFilter providing a concurrency limit and implicit back pressure into the caller, another gauge would be helpful in fine tuning request speed. I've added the ability to specify the number of requests per second in addition to the number of concurrent requests allowed at any given time.

Implementation Decisions

  • I've utilized Guava's RateLimiter to do most of the work
  • With regard to the maxWait argument found in the ThrottledRequestFilter, I decided to keep with that theme. Given that both the semaphore and the rate limiter perform a tryAcquire(), I've decided to split the maxWait across both of those operations. So if you provide a maxWait of 1000 ms, the algorithm will reduce any ms spent in the semaphore acquisition and pass that reduced amount into the rate limiter's tryAcquire.
  • I've bumped the Guava version to 14.x, I did not notice any class loading issues in my testing.

Mark Greene added 3 commits July 10, 2013 11:28
This was needed in order to utilize their RateLimiter class.
Also cleaned up the ThrottleRequestFilter a bit to remove class level members
that were not needed.
…limiting

found in the ThrottledRequestFilter
@slandelle
Copy link
Contributor

Oh man, it looks like you've developed what I've dreamed of for my own project for months!
@jfarcand @rlubke I'll soon be off and won't be able to review and test this before about 2 weeks, so feel free ;)

jfarcand added a commit that referenced this pull request Jul 10, 2013
Add a rate limited request filter
@jfarcand jfarcand merged commit 22ac9b4 into AsyncHttpClient:master Jul 10, 2013
@jfarcand
Copy link
Contributor

Pretty good work, I've merged it after review. I will take another look later this week, but so far so good!!! More patch like that welcomed!

@mgreene
Copy link
Author

mgreene commented Jul 11, 2013

@jfarcand Thanks, happy to help :). Any idea when this will make its way into maven central?

@rodrigoalvesvieira
Copy link

This is amazing. Thanks!

cs-workco pushed a commit to cs-workco/async-http-client that referenced this pull request Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants