Client-side HTTP pipelining does not work

[akka-ci]

Issue by jrudolph Monday Sep 05, 2016 at 12:39 GMT Originally opened as https://github.com/akka/akka/issues/21368

---

HTTP pipelining doesn't work any more in the low-level client-side implementation.

We do not seem to have any tests that exercise that pipelining actually works over the wire.

This test (to be added in LowLevelOutgoingConnectionSpec) fails on the last expectWireData:

      "supports two pipelined requests" in new TestSetup {
        requestsSub.sendNext(HttpRequest(uri = "/a"))
        expectWireData(
          """GET /a HTTP/1.1
            |Host: example.com
            |User-Agent: akka-http/test
            |
            |""")
        responsesSub.request(10)
        requestsSub.sendNext(HttpRequest(uri = "/b"))
        expectWireData(
          """GET /b HTTP/1.1
            |Host: example.com
            |User-Agent: akka-http/test
            |
            |""")
      }

since

Bisecting: 0 revisions left to test after this (roughly 0 steps)
[5afab017b08c1ed97099ade57aef83e6b9490383] add fusing

(which is not surprising since fusing changed all kinds of buffering behavior).

In the commit where it worked the last time, I tried a similar test:

      responsesSub.request(10)

      def sendOne(id: Int): Unit = {
        println(id)
        requestsSub.sendNext(HttpRequest(uri = s"/$id"))
        expectWireData(
          s"""GET /$id HTTP/1.1
             |Host: example.com
             |User-Agent: akka-http/test
             |
             |""")
      }

      (1 to 1000).foreach(sendOne)

which will fail only on the 49th request when backpressure kicks in (but where and why?).

In ConnectionPoolSpec, we should have these additional tests:

    "pipeline several requests on one slot if all running requests are idempotent" in pending
    "do not pipeline a further request on one slot if one request is non-idempotent" in pending

    "retry idempotent request pipelined after a request with a response that closed the connection" in pending
    "retry idempotent request pipelined on a connection that was aborted" in pending

    "do not retry non-idempotent request pipelined after a request with a response that closed the connection" in pending
    "do not retry non-idempotent request pipelined on a connection that was aborted" in pending

When the issue has been fixed, changes from #21316 to PoolSlot need to be revisited to see if everything still works (should also be caught by the new tests).

[pfcoperez]

@jrudolph , you wrote that it doesn't work any more. Do you know the akka-http version for which it worked? Thanks for the great work and issue tracking.

[ktoso]

Bump, received some feedback based on which we might want to work on this sooner. (Not required for 3.0)

[jrudolph]

Here's a previous PR on akka that tried to fix it but that wasn't merged: https://github.com/akka/akka/pull/21392

[eugenemiretsky]

@jrudolph: The issue is still open. Can you please provide an update? Is it still broken and disabled? Is it broken only for request level api, or for the host level api as well? This is a fairly important feature of HTTP. If it is still broken, would it make sense to remove it from the docs?

[jrudolph]

It's still broken and it might make sense to remove it from the docs for now.

[greenrd]

This bug, and the docs, need to be updated, for two reasons (all this applies to Akka http version 10.1.12):

1. The new HTTP client connection pool, which is the default, does support pipelining - I can see it reusing connections.
2. How is it reusing connections when the default config tells it not to pipeline? Well, it simply ignores the pipelining-limit config - a quick grep through the code seems to show that only the legacy connection pool reads that config.

But the docs claim that "# Client-side pipelining is not currently supported. See https://github.com/akka/akka-http/issues/32"

[raboof]

I've been confused about this before as well, but I think you've got the terminology mixed up here.

I can see it reusing connections.

The 'reusing connections' HTTP feature is called 'keep-alive' (and driven by the "Connection: keep-alive" header), and is indeed supported.

'Pipelining' is a related but different feature: it allows sending multiple requests on the same connection without first waiting for the results. In the happy scenario's that could result in improved latency/parallelism/throughput, but it also means if there's one slow response it can delay all the requests still in the pipeline, and thus ending up hurting latency/throughput. This is generally called 'head-of-line blocking'