Use connection keep-alive

[tiran]

Do not forcefully close the connection after every request. This enables
HTTP connection keep-alive, also known as persistent TCP and TLS/SSL
connection. Keep-alive speed up consecutive HTTP requests by 15% (for
local, low-latency network connections to a fast server) to multiple
times (high latency connections or remote peers).

pache has a default keep alive timeout of 5 seconds. That's too low for
interactive commands, e.g. password prompts. 30 seconds sounds like a
good compromise.

https://pagure.io/freeipa/issue/6641

[tkrizek]

I don't agree with this change. Since we're no longer closing connections explicitly, we rely on the server to close them. Keeping all the connections open for 30 seconds for a few use cases does not seem like an acceptable trade off.

I suggest we keep the default 5 seconds to ease the load on the server. I think making one extra round trip to establish TLS once again in cases when user is prompted for password is preferable.

[tiran]

Your assumption is incorrect. For ipa client, the connection is automatically closed by ipaclient.api.Backend.rpcclient.disconnect() or when the client process exits. The 30 seconds keepalive timeout optimizes both ipa and browser sessions. For browser session I would even increase the keepalive timeout for 60 seconds in order to reduce the load on the server.

Modern webservers like Apache use high performance socket handling features of modern operating systems. epoll can easily handle thousands of connections. Repeating TCP handshake and TLS handshake over and over again cost magnitudes more performance than watching a bunch of additional sockets for I/O. A longer keepalive period won't be a problem until we have to handle thousands of client connections simultaneously.

[tkrizek]

Ok, didn't notice that the connections are actually closed elsewhere in ipa client.

[tkrizek]

Please add comment explicitly stating that the connection is not closed on purpose to enable keep-alive.

[tiran]

There are comments and debug logging calls.

[tkrizek]

I examined this in wireshark. Without this patch, ipa vault-add would establish 7 TCP connections to apache, while it establishes only 3 with this patch. I wasn't able to track down where are the 2 rogue connections opened and why. The situation is similar for other commands.

The question is whether this improvement is good enough or whether we want to optimize the RPC to actually use just a single connection.

Also, please follow the development process next time and assign yourself to the ticket when you start working on it, so other don't have to invest time into solving the same issue.

[tkrizek]

The extra connections seem to come from the internals of httplib library. If the hostname resolves to both IPv4 and IPv6 address, one connection is established to IPv4 and two to IPv6. I wasn't able to find the reason for this, but it doesn't seem to be related to our code.

connect(4, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("10.0.0.1")}, 16) = 0
connect(4, {sa_family=AF_INET6, sin6_port=htons(443), inet_pton(AF_INET6, "dead:beef::cafe", &sin6_addr), sin6_flowinfo=htonl(0), sin6_scope_id=0}, 28) = 0
connect(4, {sa_family=AF_INET6, sin6_port=htons(443), inet_pton(AF_INET6, "dead:beef::cafe", &sin6_addr), sin6_flowinfo=htonl(0), sin6_scope_id=0}, 28) = 0

[tiran]

This behavior could be caused by https://github.com/python/cpython/blob/master/Lib/socket.py#L688 . What's socket.getaddrinfo(host, 443, 0, socket.SOCK_STREAM) for your host?

[tkrizek]

@tiran I checked that code as well, getaddrinfo returns both IPv6 and IPv4. That could explain two connections, but I'm not sure where the third one comes from.

[tkrizek]

master:

• 7beb6d1 Use connection keep-alive

• b2bdd2e Add debug logging for keep-alive

• 7f56728 Increase Apache HTTPD's default keep alive timeout
  ipa-4-5:

• 25cf4a2 Use connection keep-alive

• f784394 Add debug logging for keep-alive

• 4b426fb Increase Apache HTTPD's default keep alive timeout