[HttpClient] Allow setting max persistent connections in `CurlHttpClient` #58270

HypeMC · 2024-09-16T00:53:22Z

Q	A
Branch?	7.2
Bug fix?	no
New feature?	yes
Deprecations?	no
Issues	-
License	MIT

Currently, it's not possible to set the maximum number of simultaneously open connections in the CurlHttpClient. The maxHostConnections option potentially sets CURLMOPT_MAXCONNECTS, but only if CURLMOPT_MAX_HOST_CONNECTIONS isn't defined:

symfony/src/Symfony/Component/HttpClient/Internal/CurlClientState.php

Lines 52 to 57 in d575fa5

    
           if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) { 
        
               $maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections; 
        
           } 
        
           if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) { 
        
               curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections); 
        
           }

This, of course, isn't helpful if you want CURLMOPT_MAXCONNECTS to always be set.

In my use case, I have workers that processes queued URLs and sends purge requests to our multiple Varnish servers. The workers are long-running processes that processes the queue one item at a time, as each request must be handled in real time, making it impossible to delay or batch the URLs. This means that the number of simultaneously open cURL handlers per worker is usually just one. Given that the default configuration for CURLMOPT_MAXCONNECTS is 4 times the number of added handlers, this configuration is insufficient, as we have more than 4 Varnish servers. As a result, connections are constantly being opened and closed by each worker. Keeping these connections persistently open significantly reduces the time required to make each request.

I've tested the performance using the following hack:

$reflectionMethod = new \ReflectionMethod(CurlHttpClient::class, 'ensureState');
$curlClientState = $reflectionMethod->invoke($this->client);

curl_multi_setopt($curlClientState->handle, \CURLMOPT_MAXCONNECTS, 500);

Here are some graphs that show the results (the dotted vertical line represents the time of deployment):

The first graph shows the number of new connections being established by the workers. As you can see, it dropped to almost zero.
The second graph shows the time it takes to perform the requests. After the initial persistent connections are established, the time significantly decreases.

Even though this hack works, being able to configure this through an option would be much better.

…ent`

HypeMC · 2024-09-16T00:54:46Z

.github/workflows/integration-tests.yml

-          SERVER_NAME: 'http://localhost'
+          SERVER_NAME: 'http://localhost http://localhost:81 http://localhost:82'
          CADDY_SERVER_EXTRA_DIRECTIVES: |
+            route /http-client* {
+              root * /symfony/src/Symfony/Component/HttpClient/Tests/Fixtures/response-functional/
+              php_server
+            }


The PHP built-in server doesn't support persistent connections.

HypeMC · 2024-09-16T00:55:28Z

src/Symfony/Component/HttpClient/CurlHttpClient.php

@@ -45,6 +45,7 @@ final class CurlHttpClient implements HttpClientInterface, LoggerAwareInterface,
                             //   password as the second one; or string like username:password - enabling NTLM auth
        'extra' => [
            'curl' => [],    // A list of extra curl options indexed by their corresponding CURLOPT_*
+            'max_connections' => null, // The maximum amount of simultaneously open connections, corresponds to CURLMOPT_MAXCONNECTS


I didn't put it under curl as this is a curl multi-handler option.

nicolas-grekas · 2024-09-16T07:36:57Z

src/Symfony/Component/HttpClient/Internal/CurlClientState.php

-        if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
-            curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
+        if (\defined('CURLMOPT_MAXCONNECTS') && null !== $maxConnections ??= 0 < $maxHostConnections ? $maxHostConnections : null) {
+            curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxConnections);


instead of adding a new option, can you try removing this line and see if curl doesn't do better by default?

OK, I re-read your description, you tell about it already.

Then what about setting $maxHostConnections to 500?

@nicolas-grekas Unfortunately, setting $maxHostConnections only sets CURLMOPT_MAX_HOST_CONNECTIONS. This doesn't change CURLMOPT_MAXCONNECTS which by default is 4 times the number of added handlers. I've already tried this, and it didn't work.

nicolas-grekas

What about making this a constructor argument with a corresponding configuration option in framework-bundle?
This would require implementing the limiting for Amp too.
Up to have a look?

HypeMC · 2024-09-16T08:23:41Z

What about making this a constructor argument with a corresponding configuration option in framework-bundle? This would require implementing the limiting for Amp too. Up to have a look?

@nicolas-grekas I'll se what I can do. I thought about that initially and played a bit with the NativeHttpClient, but as far as I was able to tell, the NativeHttpClient doesn't keep a persistent connection open. I haven't checked the Amp one though.

nicolas-grekas · 2024-09-16T08:26:27Z

src/Symfony/Component/HttpClient/Internal/CurlClientState.php

@@ -52,8 +52,8 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes)
        if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
            $maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;


I think 0 is the issue. It's not documented as meaning "infinite", unlike many other curl options.

Suggested change

$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;

$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? \PHP_INT_MAX : $maxHostConnections;

@nicolas-grekas Yes, you're right, see #58278. I opened it as a bug fix, if it's not, please let me know.

nicolas-grekas · 2024-09-16T14:00:23Z

Closing in favor of #58278
Good catch for the perf issue and thanks for the graphs!

This PR was merged into the 5.4 branch. Discussion ---------- [HttpClient] Fix setting `CURLMOPT_MAXCONNECTS` | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT An alternative approach for #58270, see #58270 (comment). Commits ------- 4f9b5de [HttpClient] Fix setting CURLMOPT_MAXCONNECTS

[HttpClient] Allow setting max persistent connections in `CurlHttpCli…

318822a

…ent`

carsonbot added Status: Needs Review Feature HttpClient labels Sep 16, 2024

carsonbot added this to the 7.2 milestone Sep 16, 2024

HypeMC commented Sep 16, 2024

View reviewed changes

nicolas-grekas reviewed Sep 16, 2024

View reviewed changes

HypeMC mentioned this pull request Sep 16, 2024

[HttpClient] Fix setting CURLMOPT_MAXCONNECTS #58278

Merged

nicolas-grekas closed this Sep 16, 2024

HypeMC deleted the http-client-max-connections branch September 16, 2024 14:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HttpClient] Allow setting max persistent connections in `CurlHttpClient` #58270

[HttpClient] Allow setting max persistent connections in `CurlHttpClient` #58270

HypeMC commented Sep 16, 2024

HypeMC Sep 16, 2024

HypeMC Sep 16, 2024

nicolas-grekas Sep 16, 2024

nicolas-grekas Sep 16, 2024 •

edited

Loading

HypeMC Sep 16, 2024

nicolas-grekas left a comment

HypeMC commented Sep 16, 2024

nicolas-grekas Sep 16, 2024

HypeMC Sep 16, 2024

nicolas-grekas commented Sep 16, 2024

	if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
	$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;
	}
	if (\defined('CURLMOPT_MAXCONNECTS') && 0 < $maxHostConnections) {
	curl_multi_setopt($this->handle, \CURLMOPT_MAXCONNECTS, $maxHostConnections);
	}

		@@ -52,8 +52,8 @@ public function __construct(int $maxHostConnections, int $maxPendingPushes)
		if (\defined('CURLMOPT_MAX_HOST_CONNECTIONS')) {
		$maxHostConnections = curl_multi_setopt($this->handle, \CURLMOPT_MAX_HOST_CONNECTIONS, 0 < $maxHostConnections ? $maxHostConnections : \PHP_INT_MAX) ? 0 : $maxHostConnections;

[HttpClient] Allow setting max persistent connections in CurlHttpClient #58270

[HttpClient] Allow setting max persistent connections in CurlHttpClient #58270

Conversation

HypeMC commented Sep 16, 2024

HypeMC Sep 16, 2024

Choose a reason for hiding this comment

HypeMC Sep 16, 2024

Choose a reason for hiding this comment

nicolas-grekas Sep 16, 2024

Choose a reason for hiding this comment

nicolas-grekas Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

HypeMC Sep 16, 2024

Choose a reason for hiding this comment

nicolas-grekas left a comment

Choose a reason for hiding this comment

HypeMC commented Sep 16, 2024

nicolas-grekas Sep 16, 2024

Choose a reason for hiding this comment

HypeMC Sep 16, 2024

Choose a reason for hiding this comment

nicolas-grekas commented Sep 16, 2024

[HttpClient] Allow setting max persistent connections in `CurlHttpClient` #58270

[HttpClient] Allow setting max persistent connections in `CurlHttpClient` #58270

nicolas-grekas Sep 16, 2024 •

edited

Loading