Page MenuHomePhabricator

Find an alternative to HHVM curl connection pooling for PHP 7
Closed, ResolvedPublic

Description

When activating HTTPS between MW application servers and elasticsearch we implemented connection pooling (ref T130219) to limit the latency overhead introduced by SSL.
PHP7.x with php-fpm do not provide any connection pools we could use and thus we need to find an alternative to the technique we use with HHVM.
This is particularly important in fail-over mode when MW and elasticsearch are in different DCs.

Details

Related Gerrit Patches:

Event Timeline

dcausse created this task.Nov 29 2018, 11:24 AM
Restricted Application added a project: Discovery-Search. · View Herald TranscriptNov 29 2018, 11:24 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
EBernhardson added a comment.EditedNov 29 2018, 6:41 PM

Throwing some ideas out there:

  • PHP requests are stateless, and trying to share something, even an open socket, is painful.
  • It seems both apache and nginx are running on application servers (i only checked mw1330, but seems reasonable guess)
  • We could proxy through apache or nginx on localhost, letting them handle maintaining open connections to the search clusters. The connection pooling implementation in either of these is likely significantly more robust than trying to handle it in php, even from a C extension.
jijiki triaged this task as Normal priority.Dec 4 2018, 10:23 PM
Joe added a comment.Dec 5 2018, 7:03 AM

Throwing some ideas out there:

  • PHP requests are stateless, and trying to share something, even an open socket, is painful.
  • It seems both apache and nginx are running on application servers (i only checked mw1330, but seems reasonable guess)
  • We could proxy through apache or nginx on localhost, letting them handle maintaining open connections to the search clusters. The connection pooling implementation in either of these is likely significantly more robust than trying to handle it in php, even from a C extension.

I'm not super-happy with the idea of having nginx or apache work as forward proxies while also serving as reverse proxies, to be completely honest, but frankly it seems something along those lines is the better solution, if either server can do connection pooling and TLS session reuse. I have no idea about how performant/well done that is.

The alternative, which is admittedly a bit far fetched, would be to look at a TLS terminator that is also used for forward proxying like envoy, which has a series of very desirable properties when used as a forward proxy, including logic for fallback requests, exponential backoff, concurrency limits...

We've built it for use inside our kubernetes installation, but it could make sense exploring its installation on the application servers substituting nginx.

Krinkle renamed this task from Find an alternative to curl connection pooling available in HHVM to Find an alternative to HHVM curl connection pooling for PHP 7.Dec 9 2018, 12:39 AM
Paladox added a subscriber: Paladox.Dec 9 2018, 1:39 AM
Joe added a comment.EditedDec 17 2018, 9:18 AM

I see another problem here:

say we do what makes sense and make MediaWiki connect to -say - localhost:10001 for the search cluster. That's easy to do wherever we have tlsproxy installed, but would mean any server with mediawiki would need the same setup to be present.

I see another problem here:
say we do what makes sense and make MediaWiki connect to -say - localhost:10001 for the search cluster. That's easy to do wherever we have tlsproxy installed, but would mean any server with mediawiki would need the same setup to be present.

Perhaps a bit off the wall, but one potential solution:

  • On servers that have the appropriate localhost proxy installed puppet also creates a file /etc/some_marker_file_that_needs_a_better_name.
  • mw-config can then vary it's decision to use localhost or search.svc.<x>.wmnet on file_exists('/etc/...')
  • php keeps a file stat cache on success, the filesystem will not be checked on each request for the application servers

Will ponder other possibilities.

I suppose more generally, the previous would be cleaner if we had some way to "tag" servers and check those server tags from mediawiki-config. Puppet would setup the proxy and mark that this servers has the localhost-reverse-proxy tag or whatever, and mediawiki-config could check the server tags and vary appropriately.

Even more generally, it we install a reverse proxy for local TLS connection pooling on the application servers, does mediawiki even need to know directly? If we set https_proxy env var for the php application curl will pick that up and proxy all https requests through it. This would catch more than just elasticsearch requests, it would apply to anything making https requests from the application servers. In general it seems desirable that other requests would also get connection pooling?

Joe added a comment.Jan 8 2019, 6:47 AM

Even more generally, it we install a reverse proxy for local TLS connection pooling on the application servers, does mediawiki even need to know directly? If we set https_proxy env var for the php application curl will pick that up and proxy all https requests through it. This would catch more than just elasticsearch requests, it would apply to anything making https requests from the application servers. In general it seems desirable that other requests would also get connection pooling?

We definitely DON'T want to use a forward proxy in general, and probably even for elasticsearch we will prefer to use nginx as a reverse proxy.

Joe added a comment.Jan 8 2019, 6:48 AM

I think I have a decent idea of how to implement a basic version of what we want via nginx. I'll work on it this week hopefully.

Just one thought as I discovered this last week. A non negligible time spent by curl is by reading /etc/ssl/certs/ca-certificates.crt.
Trying to avoid any network round trip I found that the SSL overhead is between 10 to 15ms:

dcausse@elastic1020:~$ curl --cacert /etc/ssl/certs/ca-certificates.crt -w "ns: %{time_namelookup}, conn: %{time_connect}, ssl: %{time_appconnect}, start: %{time_starttransfer}\n" -o /dev/null -s https://search.svc.eqiad.wmnet:9243/ --resolve "search.svc.eqiad.wmnet:9243:127.0.0.1" 

ns: 0.000019, conn: 0.000088, ssl: 0.012000, start: 0.012717

If I force the use of /usr/local/share/ca-certificates/wmf_ca_2017_2020.crt I'm down to 2-3ms:

dcausse@elastic1020:~$ curl  --cacert /usr/local/share/ca-certificates/wmf_ca_2017_2020.crt -w "ns: %{time_namelookup}, conn: %{time_connect}, ssl: %{time_appconnect}, start: %{time_starttransfer}\n" -o /dev/null -s https://search.svc.eqiad.wmnet:9243/ --resolve "search.svc.eqiad.wmnet:9243:127.0.0.1" 

ns: 0.000021, conn: 0.000099, ssl: 0.002891, start: 0.003729

I think this explain why we've seen a +15ms when I broke connection pooling (T212768).
Will there be a way to use plain HTTP with the reverse proxy on localhost so that we avoid completely SSL initialization on the php side?

Joe added a comment.Jan 8 2019, 8:26 AM

I think this explain why we've seen a +15ms when I broke connection pooling (T212768).

That file is large, but 10-15 ms seem like a lot! Anyways

Will there be a way to use plain HTTP with the reverse proxy on localhost so that we avoid completely SSL initialization on the php side?

This is exactly my plan, yes.

Joe added a comment.Jan 11 2019, 11:15 AM

I did some *very* lame benchmarking of the response of the banner url for elasticsearch (/), with the following code:

<?php

$time = 0; 
if (function_exists('curl_init_pooled')) {
for ( $i = 0; $i < 500; ++$i) 
{ 
$ch = curl_init_pooled("cirrus-codfw", "https://search.svc.codfw.wmnet:9243/"); 
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); curl_exec($ch); $time += curl_getinfo($ch, CURLINFO_TOTAL_TIME);
} 
echo "Pooled: " . $time/500 . "\n";
}
$time = 0;
for ( $i = 0; $i < 500; ++$i) 
{ 
$ch = curl_init("https://search.svc.codfw.wmnet:9243/"); 
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); curl_exec($ch); $time += curl_getinfo($ch, CURLINFO_TOTAL_TIME);
} 

echo "Non pooled: " . $time/500 . "\n";
$time = 0;
for ( $i = 0; $i < 500; ++$i) 
{ 
$ch = curl_init("http://localhost:10002/"); 
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); curl_exec($ch); $time += curl_getinfo($ch, CURLINFO_TOTAL_TIME);
} 

echo "Nginx: " . $time/500 . "\n";

(and with modified values to connect to eqiad), ran it with both HHVM and php 7.2, and here are the results (numbers are the mean response time in milliseconds):

InterpreterCurl poolCurl plainnginx
HHVM local2.316.76
PHP local-12.21.5
HHVM crossdc3917043
PHP crossdc-16239

so while there is a small edge to using curl pooling in HHVM, I think we can safely move to using nginx for this specific use (search) pretty easily. I'd like to add some instrumentation to nginx before we go live though.

Change 483788 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/puppet@production] profile::services_proxy: simple local proxying for remote services

https://gerrit.wikimedia.org/r/483788

Change 483789 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/puppet@production] mediawiki::common: add proxy for services

https://gerrit.wikimedia.org/r/483789

Joe added a comment.EditedJan 23 2019, 1:18 PM

After some work and further research on NGINX to the end of supporting connection pooling for all outgoing connections from MediaWiki, we found NGINX (at least the FLOSS version) to lack some fundamental features.

Specifically: for most external services (with the notable exception of search!) we use DNS discovery urls. This means that we need the proxy to resolve the hostname every time the TTL expires, and to change the LVS IP it connects to accordingly.

Sadly, there is no way to have connection pooling AND proper DNS resolution support in nginx - if not in the commercial version.

Specifically:

if you define an upstream stanza

resolver 1.2.3.4;
upstream parsoid {
   server parsoid.discovery.wmnet:8000
       keepalive 100;
 }
  server {
        listen 18000;
        location / {
                proxy_pass http://parsoid;
                proxy_http_version 1.1;
                proxy_set_header Connection "";
                proxy_read_timeout     300;
                proxy_connect_timeout  300;
        }
}

parsoid.discovery.wmnet will be resolved ONLY ONCE at startup, using the libc resolver instead of the DNS server indicated in the resolver directive.

There is a way to force dns resolution via the resolver, but it prevents us from defining any form of connection pooling (which you do in an upstream stanza by defining the keepalive parameter):

server {
        resolver 208.80.154.254 valid=10s;
        listen 10002;
        set $parsoid "http://parsoid.discovery.wmnet:8000";
        location / {
                proxy_pass $parsoid;
                proxy_http_version 1.1;
                proxy_set_header Connection "";
                proxy_read_timeout     300;
                proxy_connect_timeout  300;
        }

}

in this case, the DNS server indicated in the resolver directive is used, TTL is respected, but no connection pooling happens.

The proper solution is supported in Nginx-plus, but not in the free version, showing once agian why we should never trust open core software.

I tried to write an alternative approach that uses confd to fetch discovery data, but the result is ugly in puppet terms and in actual implementation terms (it implies we'll need an nginx reload every time we change anything).

So I would conclude that we should just support the simple use-case we have now - search - and ditch any support for discovery addresses until we switch to a proxy with better support for such functionality, like Envoy, see https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/service_discovery (either the "strict dns" version or implementing an EDS backend).

fsero added a subscriber: fsero.Jan 23 2019, 7:27 PM

Change 483788 merged by Giuseppe Lavagetto:
[operations/puppet@production] profile::services_proxy: simple local proxying for remote services

https://gerrit.wikimedia.org/r/483788

Change 483789 merged by Giuseppe Lavagetto:
[operations/puppet@production] mediawiki::common: add proxy for services

https://gerrit.wikimedia.org/r/483789

dcausse assigned this task to Joe.Feb 6 2019, 10:04 AM

Thanks @Joe! I'll follow-up on this and prepare mw-config patches to use these new entries.

Change 488373 had a related patch set uploaded (by Giuseppe Lavagetto; owner: Giuseppe Lavagetto):
[operations/puppet@production] role::deployment_server: add the services proxy profile

https://gerrit.wikimedia.org/r/488373

Change 488373 merged by Giuseppe Lavagetto:
[operations/puppet@production] role::deployment_server: add the services proxy profile

https://gerrit.wikimedia.org/r/488373

debt added a subscriber: debt.

Moving to Discovery-Search (Current work) waiting column to see if there is anything else we need to do, or if we can close it out.

Joe added a comment.Feb 11 2019, 5:22 AM
This comment was removed by Joe.
Joe closed this task as Resolved.Feb 21 2019, 6:31 AM

This task is resolved per-se, we still might need the mw-config patches if they weren't merged.