module http_client - support for keeping connections open

[filiplx]

The current implementation uses curl_easy_init() to create a CURL easy handle and curl_easy_cleanup() to destroy the handle, including all open connections within it.

If curl_easy_cleanup() isn't called, it's possible to reuse an open connection for the next http call. To reset the CURL options, curl_easy_reset() can be used instead.

To use curl_easy_cleanup() or curl_easy_reset() should probably be configurable.

http_client_keep_connections_open.diff

[oej]

Discussed this with Daniel and in general it's ok. What I wonder is if we in order to support this properly in the case if we access two or more servers need to move the connection data structure somewhere else, like in process memory but indexed on http_con objects or simply reset it if the http_con object is different from the previous one used in the same process. If you don't understand this confusing message, please report so and I'll try to explain better - just wanted to write down this thought before it disappeared in the mists of a confused mind...

[que273]

The purpose of this feature is to eliminate the TCP/TLS handshake for every query. I wouldn't think calling reset + set_options was the significant bit.
The tricky bit comes when two servers are used and the process is constantly alternating between them. Do we need to store a 'CURL' object (aka easy handle) for each http_con in this case?
Or can the "CURL" object maintain a pool of open connections and select the correct one based on the specified URL + options?

[oej]

Exactly. We may have to have a pkg-memory array of connections related to the connection objects. This needs to be reset if there's a config reload at some point. The benefits are huge, so this is an important enhancement.

[filiplx]

According to this: https://curl.haxx.se/libcurl/c/CURLOPT_MAXCONNECTS.html , libcurl can per default keep up to 5 open persistent connections in the handle. So as long as 5 or fewer connections are used, everything is fine.

Perhaps there should be a module option to set this to a higher value?

[oej]

Sorry, this won't make it into 4.4. I think this would be a very useful addition though and hope to get time to get back to it.

[oej]

Well if we have one handle per curl con everything will be fine. I have some new code in master that splits up configuration in shared memory and some data like last result code and redirect URL in package memory which is per process. If we move the curl handle into package memory we can have one handle per connection. We just need to make sure we clean up properly. I think the infrastructure for getting this done is on it's way by the latest commits, just need to run more tests and make sure the memory handling works as planned.

As I want to be able to reload the configuration file at some point, we need to figure out how to handle that too.

[oej]

Committed code to MASTER. When testing with a TLS server connection reuse saves a lot of time. Thank you for this suggestion. Please try it out.

[filiplx]

I've tried to use the modparam "keep_connections", and it works!

But like the other default parameters, it must be specified before the connection definition to have effect, which I still find unintuitive. It would be neat if this was documented.

[oej]

You are right. Either documentation or a fix.

[oej]

Thank you for testing!