-
Notifications
You must be signed in to change notification settings - Fork 821
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Rate limit fix #863
Conversation
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
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 = new MultiCurl();
$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=new MultiCurl();
//$multi_curl->error(...);
//$multi_curl->setRetry(...);
//Other shared settings
foreach(["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 use Curl\Curl;
use Curl\MultiCurl;
$mch=new MultiCurl();
$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);
return false; #In a real case you would handle the retry logic here
}
return false;
});
$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. |
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