770331 - Remove HTTP Keep-Alive disable pref

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement)

Description

[Patrick McManus [:mcmanus]]

This patch removes the following prefs 

pref("network.http.keep-alive", true); // set it to false in case of problems
pref("network.http.proxy.keep-alive", true);
pref("network.http.max-connections-per-server", 15);

gecko will always try and use persistent connections, though the server is not obligated to do so to work with us nor is the way that is determined at the protocol level changed. no interop should be impacted - the change just prevents the gecko user from arbirtrarily disabling this feature, and it simplifies our code in that it does not have to maintain two different levels of max-connections (15 and 6) depending on the pconn state.

Some sites, such as those using NTLM, will not even work if the browser does not do keep-alive.

An individual transaction can disable K-A as usual with capability flags.

I want this because:

1] persistent connections are one of the most important features for HTTP/1 performance - turning them off adds an extra RTT to most transactions and serves mostly to shoot yourself in the foot and slow things down

2] I'd like to make some changes to the way we do connection limits (i.e. preconnections, and pools of established but non-transferring) connections, and having two sets of existing limits complicates that un-necessarily. Its easier to cut the cruft.

Comment 1

[Patrick McManus [:mcmanus]]

Attachment: patch 0

Comment 2

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d3ba16c3c8a0

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d3ba16c3c8a0

Comment 4

[Ed Morley [:emorley]]

The copies of httpd.js under services/sync/tps/extensions/mozmill/ and testing/peptest/peptest/extension/resource/mozmill/ are imported from https://github.com/mozilla/mozmill/commits/master/mozmill/mozmill/extension/resource/stdlib/httpd.js

The mozmill copy has since been updated (by chance), but if any further changes are required, please can you file upstream in mozmill so they can update their copy, rather than changing directly in-tree. :-)

Comment 5

[Georg Koppen]

(In reply to Patrick McManus [:mcmanus] from comment #0)
> the change just prevents the gecko user from arbirtrarily disabling this feature
And what if the user has done this deliberately? Which option does he still have to forbid e.g. a keep-alive if a proxy is used?

I just found this "bug" in Firefox Beta and bisected my way down to this ticket. Well, we have an anonymization service with several thousands of users that relies (due to its architecture) on network.http.proxy.keep-alive set to false in order to not let the last server in a so-called cascade correlate traffic of users. How can we still reach still goal given this ticket (apart from undoing the patches in a custom Firefox build)? Setting the "Proxy-Connection" header in an extension to "close" does not seem to do the trick.

Comment 6

[Patrick McManus [:mcmanus]]

an extension can set connection: close in response to on-http-modify-request observations if it really must do so.

connection is a hop-to-hop header - so it applies to the proxy in the case one is being used.

but single transaction per tcp connection is harmful to the internet; that's the root of the change.

Comment 7

[Georg Koppen]

(In reply to Patrick McManus [:mcmanus] from comment #6)
> an extension can set connection: close in response to on-http-modify-request
> observations if it really must do so.
> 
> connection is a hop-to-hop header - so it applies to the proxy in the case
> one is being used.
But the problem is that it is possible to already have network activity before extensions are even available since Gecko2 (e.g. in the add-ons version check after an application upgrade).
> but single transaction per tcp connection is harmful to the internet; that's
> the root of the change.
Yeah, and that's why the prefs were enabled by default, I guess...

Comment 8

[Chris]

I request a option to disable keepalive, basically I requesting this is undone.  It seems the submitted patch was done with some arrogance with not realising the consequence of the change.  There is 2 reasons.  Disabling keepalive is good for debugging behaviour of webservers and sites with it off as eg. som web browsers dont use keepalive, some http servers dont support keepalive, in addition firefox is actually buggy with keepalive on, it keeps improperly connections open fireever when this didnt happen with keepalive off.  Please dont dumb the browser down to chrome levels, even IE allows this to be toggled.

Comment 9

[Chris]

in addition network.http.max-connections-per-server could stop a single host from saturating the entire firefox connection limit, some sites eg. constantly refresh themselves, and when combined with the bug that connections stay open forever then connections can build up.

eg. right now I have 160 connections open to google via firefox.

Comment 10

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Chris from comment #9)
> eg. right now I have 160 connections open to google via firefox.

I would expect we would have MANY fewer connections to Google, because Google implements SPDY and we do SPDY connection coalescing. Do you have SPDY enabled? If you have SPDY enabeld, please file a bug to investigate that, and if you can, please enumerate how many connections to each Google hostname you have.

Comment 11

[Patrick McManus [:mcmanus]]

(In reply to Chris from comment #9)

> eg. right now I have 160 connections open to google via firefox.

to a collection of hosts in google.com or to one host? There shouldn't be anywhere near 160 to a single host unless you have non default preferences, which I don't recommend.

Comment 12

[Patrick McManus [:mcmanus]]

(In reply to Chris from comment #8)
> I request a option to disable keepalive, basically I requesting this is
> undone.  It seems the submitted patch was done with some arrogance with not

the patch simplified the code base.

> realising the consequence of the change.  There is 2 reasons.  Disabling
> keepalive is good for debugging behaviour of webservers and sites with it
> off as eg. som web browsers dont use keepalive, some http servers dont
> support keepalive,

it's 2013 - not supporting keep alive is detrimental to the Internet and even products that do not do so absolutely have to interact with ones that do.

> in addition firefox is actually buggy with keepalive on,

I don't believe so

> it keeps improperly connections open fireever when this didnt happen with
> keepalive off. 

I don't believe this is true (and even if it were true that's not a violation of HTTP and is therefore not buggy). An STR would be welcome if you're seeing it with a default set of preferences.

Comment 13

[Chris]

you dont think so?

what testing was done prior to this patch been applied? we have one person making a patch, its patched to code and is put in a major release.

Here is the output from my dd-wrt router, this output is after the page was left open for 15 minutes auto refreshing its output every 60 seconds.  There is 12 established connections open (12 per server limit set in firefox).  When I test with an old version of firefox with keepalive off, there is only time wait's.  I repeat this patch has been submitted without much thought simply because the author thinks "its good for the internet", if he knew anything about system admninistration he would know the majority of server admins force keepalive off on webservers, for the same reason, connections often dont get closed properly and timely.  Aside from deliberatly removing choice because a dev knows better than the enduser right? or copying chrome for the 1000th time why remove this option?

Also SPDY is disabled.

192.168.1.1.24 is the lan ip of this machine running firefox, the .254 is the router.

1	TCP	68	192.168.1.124	192.168.1.254	80	TIME_WAIT
2	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
3	TCP	17	192.168.1.124	192.168.1.254	80	TIME_WAIT
4	TCP	22	192.168.1.124	192.168.1.254	80	TIME_WAIT
5	TCP	88	192.168.1.124	192.168.1.254	80	TIME_WAIT
6	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
7	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
8	TCP	16	192.168.1.124	192.168.1.254	80	TIME_WAIT
9	TCP	2	192.168.1.124	192.168.1.254	80	TIME_WAIT
10	TCP	108	192.168.1.124	192.168.1.254	80	TIME_WAIT
11	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
12	TCP	91	192.168.1.5	173.194.34.104	80	TIME_WAIT
13	TCP	78	192.168.1.124	192.168.1.254	80	TIME_WAIT
14	TCP	98	192.168.1.124	192.168.1.254	80	TIME_WAIT
15	TCP	93	192.168.1.124	192.168.1.254	80	TIME_WAIT
16	TCP	53	192.168.1.124	192.168.1.254	80	TIME_WAIT
17	TCP	32	192.168.1.124	192.168.1.254	80	TIME_WAIT
18	TCP	7	192.168.1.124	192.168.1.254	80	TIME_WAIT
19	TCP	12	192.168.1.124	192.168.1.254	80	TIME_WAIT
20	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
21	TCP	63	192.168.1.124	192.168.1.254	80	TIME_WAIT
22	TCP	83	192.168.1.124	192.168.1.254	80	TIME_WAIT
23	TCP	113	192.168.1.124	192.168.1.254	80	TIME_WAIT
24	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
25	TCP	42	192.168.1.124	192.168.1.254	80	TIME_WAIT
26	TCP	118	192.168.1.124	192.168.1.254	80	TIME_WAIT
27	TCP	105	192.168.1.124	192.168.1.254	80	TIME_WAIT
28	TCP	73	192.168.1.124	192.168.1.254	80	TIME_WAIT
29	TCP	58	192.168.1.124	192.168.1.254	80	TIME_WAIT
30	TCP	45	192.168.1.124	192.168.1.254	80	TIME_WAIT
31	TCP	75	192.168.1.124	192.168.1.254	80	TIME_WAIT
32	TCP	37	192.168.1.124	192.168.1.254	80	TIME_WAIT
33	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
34	TCP	103	192.168.1.124	192.168.1.254	80	TIME_WAIT
35	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
36	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
37	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
38	TCP	27	192.168.1.124	192.168.1.254	80	TIME_WAIT
39	TCP	90	192.168.1.5	173.194.34.96	80	TIME_WAIT
40	TCP	47	192.168.1.124	192.168.1.254	80	TIME_WAIT
41	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED
42	TCP	1799	192.168.1.124	192.168.1.254	80	ESTABLISHED

Comment 14

[diyism]

Rollback request, "network.http.keep-alive" is important for web developers.

Comment 15

[Chris]

to diyism and thers I submitted bug and patch to reverse this bad change.

879002

https://bugzilla.mozilla.org/show_bug.cgi?id=879002

Comment 16

[Chris]

is this change sponsored by google? google coded websites love to hog connections. eg. loading youtube hogs over 20 connections (they stay open persistent).  This code as put into live firefox 17 days after submission, fast track is an understatement, thoughts?

if chrome add option to disable keepalive, will firefox copy and readd? "because its good for the internet"

Comment 17

[detlef.eppers]

(In reply to diyism from comment #14)
> Rollback request, "network.http.keep-alive" is important for web developers.

network.http.proxy.keep-alive is not just important for web developers, switching the option to false helps to stay anonymous with tools like Jondonym

http://ip-check.info/description.php?lang=en