- Notifications
You must be signed in to change notification settings - Fork821
Rate limit fix#863
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Open
ricnava00 wants to merge2 commits intophp-curl-class:masterChoose a base branch
base:master
Could not load branches
Branch not found:{{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Open
Rate limit fix#863
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
If $elapsed_seconds > $this->intervalSeconds the first time hasRequestQuota() is called, rateLimitReached is never set to true and the limit is not enforcedThis can happen if the rate limit is set in a callback function some time after startingI completely removed $rateLimitReached since its only use was in the removed condition checkAlternatively, it should be possible to reinitialize currentStartTime when setRateLimit is called, but I still don't really see the usefulness of resetting the timer only when the limit was reached in the previous interval
Hi. I don't believe the rate limit is (currently) intended to be set after starting requests. Here's an example of how $multi_curl =newMultiCurl();$multi_curl->setRateLimit('60/1m');$multi_curl->addGet('https://httpbin.org/ip');// etc.$multi_curl->start(); What's the use case for modifying the rate limit in a callback? |
I use it like this $multi_curl=newMultiCurl();//$multi_curl->error(...);//$multi_curl->setRetry(...);//Other shared settingsforeach(["fast","slow"]as$site){for($n=0;$n<10;$n++){$ch=$multi_curl->addGet("http://".$site."-site.com/?page="+$n);if($n==0&&$site=="slow"){$ch->beforeSend(function()use ($multi_curl){$multi_curl->setRateLimit('60/1m');});}}}$multi_curl->start(); If I had to call setRateLimit while multi_curl is not running I would either have to create another MultiCurl, copy all the callbacks, decide which one to use in the loop, and start them one after the other, or I would have to duplicate the loop for the fast and slow sites and set the rate limit between the two |
I was having a look at this issue again, and I found a more realistic problem that this solves Usually, I handle 429 errors by sleeping for some time useCurl\Curl;useCurl\MultiCurl;$mch=newMultiCurl();$mch->setRateLimit("4/1s");$mch->setConcurrency(4);$mch->setRetry(function(Curl$ch){if($ch->errorCode>=429){echo"Too many requests, sleeping for 1 second\n";sleep(1);returnfalse;#In a real case you would handle the retry logic here}returnfalse;});$startTime=microtime(true);$mch->beforeSend(function($ch)use ($startTime){echo"Sending".$ch->url." at".round((microtime(true)-$startTime)*1000,1)."ms\n";});foreach(array_merge(array_fill(0,10,200),array_fill(0,8,429),array_fill(0,20,200))as$i){$mch->addGet("https://httpstat.us/".$i);}$mch->start(); As you can see, the first 200s are received in batches of 4 each second, which is correct. After sleeping, the batches are all consecutive and no longer wait. |
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
If $elapsed_seconds > $this->intervalSeconds the first time hasRequestQuota() is called, rateLimitReached is never set to true and the limit is not enforced This can happen if the rate limit is set in a callback function some time after starting
I completely removed $rateLimitReached since its only use was in the removed condition check
Alternatively, it should be possible to reinitialize currentStartTime when setRateLimit is called, but I still don't really see the usefulness of resetting the timer only when the limit was reached in the previous interval