fix: Reduce the chance of CurlEventPublisher exhausting file descriptors on high throughput PHP-FPM servers

[Watercycle]

Requirements

• I have added test coverage for new or changed functionality
• I have followed the repository's pull request submission guidelines
• I have validated my changes against all supported platform versions

Related issues

• Unable to send events in CurlEventPublisher because of too many events #185

Describe the solution you've provided

TLDR: --connect-timeout vs --max-time (docs)

---

Consider the following code:

function curl()
{
    shell_exec("/usr/bin/env curl -X POST --connect-timeout 3 -H 'Content-Type: application/json' -H 'Accept: application/json' https://reqres.in/api/users?delay=30" . " >> /dev/null 2>&1 &");
}

+

while true; do   echo "$(date +"%H:%M:%S") - Open files: $(lsof | wc -l)";   sleep 0.5; done

However long the request takes is how long file descriptors hang open for.

Web apps like Laravel frequently use PHP-FPM as part of their server infrastructure where each request is a new process. As a result, on high throughput servers, EventProcessor.php::__destruct gets invoked quite frequently. Having many child worker processes seems to also mean new sub-processes consuming many file descriptors. While the Relay Proxy is great at allowing the PHP SDK to quickly read from data stores like Redis, the impressions event processing system ends up quickly bottle-necking if there's any slow-down in publishing events.

Describe alternatives you've considered

I think ideally:

1. CurlEventPublisher should use the actual curl client or something like Guzzle for making async requests instead of using shell_exec.
2. That CurlEventPublisher should possibly explore batch requesting strategies that won't choke out technologies like PHP-FPM (e.g. using the file system + a separate worker thread). With more consideration for making sure these processes terminate - like logging warnings when it happens.

I ultimately stuck with a simple change to minimize the chance of it being a breaking change.

Additional context

This is related to an issue I filed a bit ago: https://support.launchdarkly.com/hc/en-us/requests/87721

[keelerm84]

The LDClient config already allows specifying both a connect_timeout and a max length timeout parameter. We use that timeout parameter in the existing Guzzle client.

Can you update this so that we still support the connect time parameter as before, and also add the max time flag hooked up to that value?

[keelerm84]

Thank you @Watercycle for the contribution. I went ahead and made that minor modification. I appreciate your time and effort on this work.