From 437cecf93939d7b7df50605ba770cdcd1ad4c081 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Tue, 13 Jun 2017 08:39:30 -0700 Subject: [PATCH 1/4] Fixes #283 : Fixes CPU busy loop when using request_multiple. See #284 for background information This will require more testing across various libcurl versions. I doubt that the timeout is necessary for curl_multi_select (calls select() if it can), but leaving in one just in case of bugs, so that it will end. - Haven't thoroughly checked for relevant libcurl bugs. Asynchronously wait for events with a short timeout if CURLM_CALL_MULTI_PERFORM fails. Use shorter timeouts when the total time elapsed so far is shorter. Make the largest possible manual usleep 2ms. --- library/Requests/Transport/cURL.php | 31 +++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index 4429edb64..640e4d0d5 100644 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -214,14 +214,44 @@ public function request_multiple($requests, $options) { $request['options']['hooks']->dispatch('curl.before_multi_exec', array(&$multihandle)); + $made_progress = true; + $all_requests_start = microtime(true); + $microseconds_taken_by_last_curl_operation = 0; do { $active = false; + if (!$made_progress) { + // For a small fraction of slow requests, curl_multi_select will busy loop and return immediately with no indication of error (it returns 0 immediately with/without setting curl_multi_errno). + // (https://github.com/rmccue/Requests/pull/284) + // To mitigate this, add our own sleep if curl returned but no requests completed (failing or succeeding) + // - The amount of time to sleep is the smaller of 1ms or 10% of total time spent waiting for curl requests + // (10% was arbitrarily chosen to slowing down requests that would normally take less than 1 millisecond) + // - The amount of time that was already spent in curl_multi_exec+curl_multi_select in the previous request is subtracted from that. + // (E.g. if curl_multi_select already slept for 2 milliseconds, curl_multi_select might be operating normally) + $elapsed_microseconds = (microtime(true) - $all_requests_start) * 1000000; + $microseconds_to_sleep = min($elapsed_microseconds / 10, 2000) - $microseconds_taken_by_last_curl_operation; + if ($microseconds_to_sleep >= 1) { + usleep($microseconds_to_sleep); + } + } + $operation_start = microtime(true); + $made_progress = false; + $is_first_multi_exec = true; do { + if (!$is_first_multi_exec) { + // Sleep for 1 millisecond to avoid a busy loop that would chew CPU + // See ORA2PG-498 + usleep(1000); + } $status = curl_multi_exec($multihandle, $active); + $is_first_multi_exec = false; } while ($status === CURLM_CALL_MULTI_PERFORM); + // curl_multi_select will sleep for at most 50 milliseconds before returning. + curl_multi_select($multihandle, 0.05); + $microseconds_taken_by_last_curl_operation = (microtime(true) - $operation_start) * 100000; + $to_process = array(); // Read the information as needed @@ -234,6 +264,7 @@ public function request_multiple($requests, $options) { // Parse the finished requests before we start getting the new ones foreach ($to_process as $key => $done) { + $made_progress = true; $options = $requests[$key]['options']; if (CURLE_OK !== $done['result']) { //get error string for handle. From fade4129f235614c045691836a6a960c3380c734 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Wed, 23 Oct 2019 15:04:50 -0400 Subject: [PATCH 2/4] Comment nit --- library/Requests/Transport/cURL.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index 640e4d0d5..eac056d24 100644 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -224,7 +224,7 @@ public function request_multiple($requests, $options) { // For a small fraction of slow requests, curl_multi_select will busy loop and return immediately with no indication of error (it returns 0 immediately with/without setting curl_multi_errno). // (https://github.com/rmccue/Requests/pull/284) // To mitigate this, add our own sleep if curl returned but no requests completed (failing or succeeding) - // - The amount of time to sleep is the smaller of 1ms or 10% of total time spent waiting for curl requests + // - The amount of time to sleep is the smaller of 2ms or 10% of total time spent waiting for curl requests // (10% was arbitrarily chosen to slowing down requests that would normally take less than 1 millisecond) // - The amount of time that was already spent in curl_multi_exec+curl_multi_select in the previous request is subtracted from that. // (E.g. if curl_multi_select already slept for 2 milliseconds, curl_multi_select might be operating normally) From f9c142ac08c7e4d1368ba0d6f708a69cd04cf18b Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Wed, 23 Oct 2019 16:33:59 -0400 Subject: [PATCH 3/4] Sleep for less time. It seems like long-lived request_multiple operations are more affected by the sleep statements that were added. --- library/Requests/Transport/cURL.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index eac056d24..3bc3b5b98 100644 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -224,12 +224,12 @@ public function request_multiple($requests, $options) { // For a small fraction of slow requests, curl_multi_select will busy loop and return immediately with no indication of error (it returns 0 immediately with/without setting curl_multi_errno). // (https://github.com/rmccue/Requests/pull/284) // To mitigate this, add our own sleep if curl returned but no requests completed (failing or succeeding) - // - The amount of time to sleep is the smaller of 2ms or 10% of total time spent waiting for curl requests + // - The amount of time to sleep is the smaller of 1ms or 10% of total time spent waiting for curl requests // (10% was arbitrarily chosen to slowing down requests that would normally take less than 1 millisecond) // - The amount of time that was already spent in curl_multi_exec+curl_multi_select in the previous request is subtracted from that. - // (E.g. if curl_multi_select already slept for 2 milliseconds, curl_multi_select might be operating normally) + // (E.g. if curl_multi_select already slept for 1 milliseconds, curl_multi_select might be operating normally) $elapsed_microseconds = (microtime(true) - $all_requests_start) * 1000000; - $microseconds_to_sleep = min($elapsed_microseconds / 10, 2000) - $microseconds_taken_by_last_curl_operation; + $microseconds_to_sleep = min($elapsed_microseconds / 10, 1000) - $microseconds_taken_by_last_curl_operation; if ($microseconds_to_sleep >= 1) { usleep($microseconds_to_sleep); } @@ -250,7 +250,6 @@ public function request_multiple($requests, $options) { // curl_multi_select will sleep for at most 50 milliseconds before returning. curl_multi_select($multihandle, 0.05); - $microseconds_taken_by_last_curl_operation = (microtime(true) - $operation_start) * 100000; $to_process = array(); @@ -292,6 +291,7 @@ public function request_multiple($requests, $options) { } $completed++; } + $microseconds_taken_by_last_curl_operation = (microtime(true) - $operation_start) * 100000; } while ($active || $completed < count($subrequests)); From 34684b1d279715b89593fc358fff8225a68603a8 Mon Sep 17 00:00:00 2001 From: Tyson Andre Date: Wed, 23 Oct 2019 16:53:36 -0400 Subject: [PATCH 4/4] Check return value of curl_multi_select --- library/Requests/Transport/cURL.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/Requests/Transport/cURL.php b/library/Requests/Transport/cURL.php index 3bc3b5b98..f57ca0bff 100644 --- a/library/Requests/Transport/cURL.php +++ b/library/Requests/Transport/cURL.php @@ -235,7 +235,6 @@ public function request_multiple($requests, $options) { } } $operation_start = microtime(true); - $made_progress = false; $is_first_multi_exec = true; do { if (!$is_first_multi_exec) { @@ -249,7 +248,12 @@ public function request_multiple($requests, $options) { while ($status === CURLM_CALL_MULTI_PERFORM); // curl_multi_select will sleep for at most 50 milliseconds before returning. - curl_multi_select($multihandle, 0.05); + // Note that in php 7.1.23+, curl_multi_select can return immediately but return 0 with no error in some edge cases. + $select_result = curl_multi_select($multihandle, 0.05); + // A return value of 0 means that nothing interesting happened. + // A return value of -1 means that either (1) nothing interesting happened or (2) there was an error. + // A return value of 1 or more means that interesting things happened to that many connections. + $made_progress = $select_result > 0; $to_process = array(); @@ -263,7 +267,7 @@ public function request_multiple($requests, $options) { // Parse the finished requests before we start getting the new ones foreach ($to_process as $key => $done) { - $made_progress = true; + $made_progress = true; // Set this just in case progress was made despite curl_multi_select saying otherwise. $options = $requests[$key]['options']; if (CURLE_OK !== $done['result']) { //get error string for handle.