launchpadlib attempts to use TE (which does not work well in our environment)

Bug #574697 reported by Leonard Richardson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
launchpadlib
Triaged
High
Unassigned

Bug Description

launchpadlib sends the TE header to indicate that it would like compressed versions of resources. If the TE header is present, lazr.restful will process it correctly. But launchpad is not directly connected to clients, because there are intermediaries present, so TE compression cannot be used without the intermediaries all being aware back into the datacentre (and to our outer most intermediary which would have to do the compression).

CE is much simpler and more robust, we should use it. This requires WADL serving to be simplified (removed from the appservers, so that the apache stack, which will do the just-in-time CE for us, handles ETag validation).

Gary Poster (gary)
Changed in launchpad-foundations:
status: New → Triaged
importance: Undecided → High
Revision history for this message
Leonard Richardson (leonardr) wrote :

As an experiment, I told launchpadlib to include a 'Foo-Bar' header. This showed up in the appropriate place as HTTP_FOO_BAR. Something seems to be specifically stripping off the TE header. Apache?

Revision history for this message
Leonard Richardson (leonardr) wrote :

I believe Apache's mod_proxy is stripping the TE header, and I don't see an easy way to fix it. Here's what I believe is happening:

1. launchpadlib sends a request to https://api.launchpad.dev/ that includes the TE header.
2. Apache prepares to handle the request by proxying it to http://localhost:8086/.
3. The TE header is a hop-by-hop header, and the HTTP proxy connection between https://api.launchpad.dev/ and http://localhost:8086/ counts as a "hop". So mod_proxy removes the TE header before proxying the request.

My skills with mod_proxy are admittedly poor, but I don't see any way to say "pass through the TE header". My skills with mod_rewrite are a little better, so I investigated the possibility of ensuring TE's survival by hacking the header value into the query string,

RewriteCond %{HTTP:TE} .+
RewriteRule (.*) $1?HTTP_TE=%{HTTP:TE} [QSA]

Strangely enough, requests that triggered this condition and were subject to this rewrite rule were *not* subject to the "ProxyPass" directive immediately below. The requests were not proxied and I got Apache 404 errors for my trouble. I don't know why, and this is where I gave up.

If we do get Apache to pass through TE, there's a real possibility that mod_proxy will just strip the Transfer-Encoding _response_ header and decompress the data before sending it over the network, since Transfer-Encoding is also a hop-by-hop header. Basically, I suspect we have this problem in both request and response, and I see even less hope for fixing this on the response side. (I ran a hacky experiment to see if this was true, but the results were inconclusive.)

If you haven't followed this whole drama you might ask: "why not use Accept-Encoding and Content-Encoding like a normal web app?" Well, we did that at one time. The problem is that we were using Content-Encoding with mod_compress, which
changes the ETags of the representations it serves from "foo" to "foo-gzip". (The Apache bug https://issues.apache.org/bugzilla/show_bug.cgi?id=39727 explains why.) When launchpadlib sent a conditional request with If-None-Match: "foo-gzip", lazr.restful had no idea what the ETag meant, and the conditional request didn't work, even if it would have otherwise.

I can think of two solutions that don't involve Apache magic:

1. My understanding of HTTP intermediaries such as proxies is pretty poor. There may be something in RFC2616 that allows a client to ask an intermediary to pass on a hop-by-hop header if that's possible. I'll investigate this.

2. We've already implemented the equivalent of mod_compress in lazr.restful; we just made it use TE and Transfer-Encoding instead of Accept-Encoding and Content-Encoding, so we could use the same ETags. But this means lazr.restful now controls every relevant portion of the system. We can go back to using Accept-Encoding and Content-Encoding, and lazr.restful can be programmed to serve and understand ETags like "foo-gzip". The downside of this solution is that it won't benefit existing clients, who will keep uselessly sending TE headers forever.

Revision history for this message
Leonard Richardson (leonardr) wrote :

Re proposed solution #1: What I want is the opposite of the "Connection" header, and HTTP (understandably) doesn't have it.

Revision history for this message
Leonard Richardson (leonardr) wrote :

For reference, bug 261619 is the one covering the original Content-Encoding problem and the changeover to Transfer-Encoding.

Revision history for this message
Leonard Richardson (leonardr) wrote :

After discussing this on the launchpad developers list, I've come to the conclusion that the best solution is to go back to using Content-Encoding. After we switched over to TE, a fix was applied to Apache that makes it look at incoming If-None-Match and If-Match, and transparently strip off the "-gzip" suffix it added to the ETag on the way out. This would have solved our original problem. Assuming that fix is present in the version we're running, we should be able to simply revert our changes to lazr.restfulclient and lazr.restful, and turn mod_compress back on on the 'api' vhost.

In my tests I've discovered a discrepancy between edge and dev. I don't know the significance of this discrepancy. Here we go: first, I hacked httplib2 to print out the content length and the first part of the content so i could easily see whether a given response was compressed.

When I use an unmodified lazr.restfulclient, dev and edge have the same behavior. I never get compression.

RAW CONTENT SIZE: 1190598
RAW CONTENT BEGINS: <?xml version="1.0"?

OK, that's bad, but that's the point of this bug. Then, I made a lazr.restfulclient branch that sends Accept-Encoding instead of TE. Basically, I removed my custom code and let httplib2 do what it does normally. This has no effect on edge,

RAW CONTENT SIZE: 1225680
RAW CONTENT BEGINS: <?xml version="1.0"?

On dev, the representation is compressed with gzip!

header: Etag: "9af6ae672c23d5d7cd2ef19999a13dfed68aac02"
header: Content-Encoding: gzip
header: Transfer-Encoding: chunked
RAW CONTENT SIZE: 108257
RAW CONTENT BEGINS: ^_<8B>^H^@^@^@^@^@^@^C<C4>Ym<8F><E3><B6>^Q<FE><BE><BF>

Awesome! But why? Is mod_compress secretly enabled on the 'api' vhost in dev, but disabled in production? That seems like a strange way to do it. And if mod_compress were running, it would have tacked "-gzip" onto the ETag. It looks like _somebody_ compressed the representation on the content level without changing the ETag (which is wrong). But I don't know who.

Revision history for this message
Leonard Richardson (leonardr) wrote :

Yes, mod_compress is enabled on the 'api' vhost, but disabled in production. We're going to re-enable it in production today.

Revision history for this message
Gary Poster (gary) wrote :

From IRC conversation with Leonard today:

- the apache bug has not been fixed. our version of mod_compress still modifies etags on the way out but not on the way in

- [...] we should not yet turn mod_compress back on in the api vhost

- https://issues.apache.org/bugzilla/show_bug.cgi?id=39727#c31 has roy fielding explaining why the "just strip off -gzip on the way in" fix doesn't work, and that's probably why the fix was never applied

- roy says "The best solution is to implement transfer-encoding as an http protocol filter module", so that intermediaries can understand it and pass it along

We'll discuss next steps soon. They are not clear yet.

Revision history for this message
Leonard Richardson (leonardr) wrote :

After thinking about this on my vacation I've come to the conclusion that the best solution for now is to hack a (configurable) understanding of the current mod_compress behavior into lazr.restful. Anything else is above our pay grade, and this is too big a performance win to ignore.

Revision history for this message
Robert Collins (lifeless) wrote :

So, we should be using CE for this. I'll update the description.

Why? Because we don't want hop by hop negotiation, we can use a compressed representation just fine. The WADL serving needs to move out of the appservers *anyway* (there is a separate bug for that), so the angst over ETAG handling is irrelevant - apache will handle all of that for us.

affects: launchpad → launchpadlib
summary: - Launchpad strips incoming TE header
+ launchpadlib attempts to use TE
description: updated
Revision history for this message
Francis J. Lacoste (flacoste) wrote : Re: launchpadlib attempts to use TE

AFAIK, apache is already serving the WADL file.

Revision history for this message
Robert Collins (lifeless) wrote :

Bug 245368 asked for that, but was closed by serving a static file from within the appserver (so appserver negotiation still happens).
Bug 607961 reprised the request for apache, due to appservers still having issues; its closed as 'ready for apache' with the 'second stage via RT'.

I don't know if the RT has been actioned yet; I vaguely think not, but I may be being pessimistic.

summary: - launchpadlib attempts to use TE
+ launchpadlib attempts to use TE (which does not work well in our
+ environment)
tags: added: easy
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.