Event bridge - Optimization of Connection Persistence in HTTP Client

[lukasojd]

This merge request addresses a significant performance issue in the HTTP client, specifically relating to the management of keep-alive connections. Previously, when only the status code of the HTTP response was retrieved without accessing the content, it inadvertently led to the termination of keep-alive connections on the client side (particularly noticeable with curl). This behavior resulted in the need for re-establishing connections and performing a new handshake for subsequent requests. Such a process introduced a considerable delay ranging between 30-80 ms for each request, substantially impacting the overall efficiency.

The proposed changes in this merge request aim to optimize this behavior. By ensuring that the content of the HTTP response is accessed ($this->httpResponse->getContent(false)) even when a timeout is not specified (null === $timeout), we maintain the persistence of the keep-alive connection. This approach effectively avoids the overhead of re-establishing connections and performing handshakes for subsequent requests, thereby enhancing the performance of the HTTP client.

These changes are crucial for scenarios where high-frequency requests are made, as it significantly reduces the cumulative latency. This optimization is particularly beneficial for applications requiring fast and efficient communication with AWS services.

Included is the updated code snippet for the resolve function, which now ensures that the content of the HTTP response is accessed irrespective of the timeout condition, thereby maintaining the keep-alive connection state effectively.

[GrahamCampbell]

Interesting. Is this still happening if you set the max connections to 1 on the curl http client, without using this change?

[GrahamCampbell]

This line is no longer needed, due to the line you added after.

[lukasojd]

Fix please test cc @lukasojd

[lukasojd]

Interesting. Is this still happening if you set the max connections to 1 on the curl http client, without using this change?

I have conducted tests using the default Symfony client (Symfony\Component\HttpClient\CurlHttpClient), specifically setting the maximum number of host connections to 1. Here's the relevant code snippet:

$client = new CurlHttpClient(maxHostConnections: 1);
$this->client = new EventBridgeClient(
    configuration: [
        'accessKeyId' => $this->accessKeyId,
        'accessKeySecret' => $this->accessKeySecret,
        'region' => $this->region,
    ],
    httpClient: $client,
    logger: $this->logger,
);

Despite this adjustment, the issue persists. The configuration was set up as per the example above, ensuring that only one connection per host is maintained at any time. However, it seems that this configuration change alone does not resolve the problem.

Additionally, I have also monitored the network traffic and confirmed that the issue indeed involves the closing of connections when the response is not fully retrieved. This results in repeated packets associated with connection re-establishment and the SSL handshake process. It's evident from the network analysis that each new request initiates a fresh connection cycle, including the SSL handshake, which aligns with the observed performance issue.

[jderusse]

@nicolas-grekas does it rings a bell?

We don't consume the entire response to save few IO. But it looks like the side effect is, the response is not free for curl, and it don't reuse it.

How can we tell SymfonyClient to stop streaming the response, but keep the connection alive?

[lukasojd]

@GrahamCampbell @jderusse @nicolas-grekas I've continued troubleshooting the issue and discovered that the problem is related to specific versions of curl. It appears that versions prior to 7.87.0 are affected, while the issue has been resolved starting from curl 7.87.0. Considering this, I believe it may not be necessary to insist on this merge request. The resolution might simply involve updating to a newer version of curl. I leave the decision up to you on how to proceed further.

[nicolas-grekas]

Thanks for digging!
Given 7.87.0 is one year old, it might be fine doing nothing indeed.
Unless we're sure that fetching all the body won't generate needless network traffic (because responses are small) in which case why not.

[GrahamCampbell]

Using the amphp http client may be a suitable work-around for you?