HTTP Pipelining causes responses to mixed up. #2665

Closed
nkvoll opened this Issue Feb 19, 2013 · 23 comments

Projects

None yet
@nkvoll
Member
nkvoll commented Feb 19, 2013

ElasticSearch seems to advertise HTTP pipelining support by using HTTP 1/1 and not supplying a Connection-header in the response, but fails to deliver on the promises of responding to the requests in the same order they are sent, which means that clients might get the response from an unexpected request.

Example reproduction:

It sometimes works es expected

$ printf "GET /_nodes HTTP/1.1\r\n\r\nGET / HTTP/1.1\r\n\r\n" | nc -i 1 127.0.0.1 9200
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 222

{"ok":true,"cluster_name":"elasticsearch","nodes":{"MVf7UrJJRyaOJj35MAdODg":{"name":"Caiera","transport_address":"inet[/10.0.0.6:9300]    ","hostname":"machine.local","version":"0.20.4","http_address":"inet[/10.0.0.6:9200]"}}}HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 169

{
  "ok" : true,
  "status" : 200,
  "name" : "Caiera",
  "version" : {
    "number" : "0.20.4",
    "snapshot_build" : false
  },
  "tagline" : "You Know, for Search"
}

But sometimes, given the exact same request, changes the order of the responses:

$ printf "GET /_nodes HTTP/1.1\r\n\r\nGET / HTTP/1.1\r\n\r\n" | nc -i 1 127.0.0.1 9200
HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 169

{
  "ok" : true,
  "status" : 200,
  "name" : "Caiera",
  "version" : {
    "number" : "0.20.4",
    "snapshot_build" : false
  },
  "tagline" : "You Know, for Search"
}HTTP/1.1 200 OK
Content-Type: application/json; charset=UTF-8
Content-Length: 222

{"ok":true,"cluster_name":"elasticsearch","nodes":{"MVf7UrJJRyaOJj35MAdODg":{"name":"Caiera","transport_address":"inet[/10.0.0.6:9300]    ","hostname":"machine.local","version":"0.20.4","http_address":"inet[/10.0.0.6:9200]"}}}    
@kimchy
Member
kimchy commented Feb 22, 2013

HTTP 1.1 and HTTP pipeline are two different things, don't confuse them :). Yea, there will be problems today with using HTTP pipeline feature.

@spinscale
Member

maybe worth a look in this regard: https://github.com/typesafehub/netty-http-pipelining

@spinscale
Member

@nkvoll Are you trying to make use of the HTTP pipelining feature in production? I actually was not really able to find a Java library supporting this from the client side in order to conduct some tests. Just trying to understand, what you are trying to archive.

What you can do as a different solution at the moment (which is also returning the responses when they are finished instead of creating the responses in a queue and ensuring their order on server side), is to use the "X-Opaque-Id" header of elasticsearch in order to map back the request/response pairs on the client instead of the server side (which might not be what you want depending on your use-case). You will obviously have a hard time with this inside of a browser (not all of them is supporting pipelining anyway)

printf "GET /_nodes HTTP/1.1\r\nX-Opaque-Id: 2\r\n\r\nGET / HTTP/1.1\r\nX-Opaque-Id: 1\r\n\r\n" | nc -i 1 127.0.0.1 9200
@spinscale spinscale was assigned Jun 6, 2013
@spinscale
Member

@nkvoll could you solve your issue by using the X-Opaque-Id header or do you think there needs more to be done in elasticsearch in order to support your use-case?

I am not sure if we can 'not advertise' pipelining support (as you wrote in the initial ticket), but still have this functionality. Would like to hear your opinions on that!

@tkurki
tkurki commented Feb 18, 2014

Not that I need pipelining with ES for anything, but http 1.1 spec at http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html#sec8.1.2.2 says "A server MUST send its responses to those requests in the same order that the requests were received"

@spinscale spinscale removed their assignment Feb 18, 2014
@kevin-montrose

We just got bit really badly by this issue over at Stack Overflow.

We've got an app over here that does an awful lot of (highly parallel) reads and writes by id over a small number of types. Because the types matched most of the time, we'd normally be able to deserialize and proceed normally; oftentimes even updating the wrong document at the end.

This is exacerbated by .NET's default behavior being to pipeline all web requests.

It took the better part of a week to isolate and debug this issue. It's particularly insidious since capturing requests at a proxy can easily strip out pipelining (as Fiddler does, for example).

We have a workaround (just setting myHttpWebRequest.Pipelined = false), pipelined really should fail if they're not going to be honored to spec.

There's a non-trivial performance penalty to disabling pipeling, at least in .NET. I threw together a quick gist in LinqPad against our production cluster. In my testing it's about 3 times faster to pipeline requests on our infrastructure.

@jprante
Contributor
jprante commented Jul 25, 2014

Maybe a priority queue like in https://github.com/typesafehub/netty-http-pipelining could be integrated into ES netty http transport, and enabled by an option, e.g. http.netty.pipelining: true|false.

@gmarz
Contributor
gmarz commented Jul 25, 2014

@kevin-montrose , we've never run into that issue, but it sounds a bit scary. We're considering making a change to the .NET client to disable pipelining by default (exposed as a connection setting) until this is addressed with elasticsearch. However, I'm a bit hesitant after hearing your claim about performance.

For what it's worth though, I ran your gist on my machine to see if there would be a notable difference, and there wasn't. In fact, pipelined was a bit slower more times than non. Maybe totally environment related.

@kimchy
Member
kimchy commented Jul 25, 2014

@jprante looking that the code, I don't think its enough what is done there in the context of ES. The implementation supports returning the results in the same order, while in ES, we need to make sure that we execute it in the same order. For example, if an index request and then a get request for the same id are in the same inflight requests, yet serialized on the client side, they need to be serialized on the request side to be executed one after the other, not just return the results of them in order, since in that case, they might still execute out of order.

I still need to think about this more, but effectively, in order to implement ordered pipeline support, it means it needs to be ordered when handling the upstream events, and progressing with one at all before the the previous one is sent downstream as a response.

@kimchy
Member
kimchy commented Jul 25, 2014

btw, to make things more interesting..., assume you only execute search requests, then HTTP pipelining with just ordering the responses is great, since it allow for concurrent execution of them on the server side. Tricky.... .

@jprante
Contributor
jprante commented Jul 26, 2014

@kimchy maybe it is possible just to timestamp the incoming HTTP request in NettyHttpServerTransport, with a channels handler especially for HTTP pipelining, so the responses can be ordered downstream in a priority queue by timestamp? Queueing up a certain number of responses in a queue in this special HTTP pipeline channels handler would be the downside.

I think executions can be still unordered, even with HTTP pipelining, because these are separate things. If e.g. set/get on the same id fails, there might be other obscure reasons. But it will just become reliably visible, by eliminating Netty's weakness in HTTP pipelining.

@NickCraver

@gmarz Pipelining's main gains are in any environment with non-0 latency: the higher the ratio of network latency vs. time spent inside elastic, the higher the impact. It's definitely going to vary by environment, but there are many cases where this is a huge impact...in others like link-local or on-box it may have little difference. It's important to test the performance impact of pipelining over not on your local machine as well.

@gmarz gmarz referenced this issue in elastic/elasticsearch-net Jul 27, 2014
Closed

Consider disabling HTTP pipelining by default #830

@gmarz
Contributor
gmarz commented Jul 27, 2014

@NickCraver thanks a lot for the info, I figured as such. Definitely plan on doing more testing before making any changes to the .NET client. I just opened #830 for this, suggestions/input very much welcomed.

@hamiltop

We're getting bit by it as well. Using erlang ibrowse client, which uses pipelining by default.

@NickCraver

Has there been any update on this? It was a deciding factor in us dropping ElasticSearch as a viable option for projects at Stack Overflow, but we'd love to see this fixed for other potential uses.

This is a serious bug, and it doesn't seem (at least to our developers) to be treated as serious as the impact it has.

@kimchy
Member
kimchy commented Oct 15, 2014

@NickCraver thanks for giving context into this, I have personally been thinking about how to solve this, I think that potentially we can start with baby steps and make sure that at the very least the response are returned in order, and iterate from there. Will keep you posted.

@clintongormley clintongormley removed the discuss label Oct 16, 2014
@clintongormley clintongormley added the bug label Oct 17, 2014
@asnare
asnare commented Oct 29, 2014

@kimchy I'm not sure that execution order is a problem for pipelined requests, mainly because pipelining is a transport optimisation. Pipelined requests are semantically equivalent to the same requests being issued on separate connections. If client applications care about ordering, they should not issue requests that depend on requests whose response status is still pending.

@NickCraver

To better illustrate for people the impact here, let's compare a real world example. Let's say for example I'm querying our elasticsearch cluster in Oregon from New York. I have 10 requests to make. The trip from NY to OR takes about 40ms, and elasticsearch only takes 10ms to fulfill each request. Let's compare performance with and without pipelining:

Pipelining:

10 requests issued in order immediately
40ms travel time
10 requests processed (any order), responded to (in order) (10+ms, elasticsearch processing)
40ms travel time
10 responses received in order

We're talking about 90+ ms (the + depends on how well elasticsearch handles the concurrent queries). Let's say it's not even concurrent (worst case), then we're talking 100ms for elastic, so 190ms total.

Now let's turn pipelining off:

1 request issued
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Single request received, next request sent
40ms travel time
10ms 1 request processed, 1 response sent
40ms travel time
Last request received

That took 90ms per request, so at best it's 900ms. Hopefully that better illustrates the performance problem that not having HTTP pipelining working introduces. The same happens at < 1ms latency which adds up for millions of requests per day. We hit some systems like this (redis for example) with 4 billion+ hits a day...without pipelining Stack Overflow would be hosed.

Currently, if one for 10 indexing operations fails (especially on the indexing side) we can get an "okay, indexed!" when in fact it failed...and we don't even know which one failed. So not only is this a major problem (which is why I feel this makes easily the resiliency list), it's a problem a user is incapable of solving. You currently just have to turn off pipelining and take a huge performance hit for high traffic applications.

As for execution order, yes it does matter. If a Stack Overflow post changes, we may issue an index request as soon as that happens - edits can happen rapidly and 2 indexing requests for the same document (with a new post body, title, tags, last activity date, etc. in our case) can easily be in the pipeline. If the last index command doesn't execute last, then an invalid document has been indexed which is out of date.

@nkvoll
Member
nkvoll commented Oct 30, 2014

@NickCraver regarding the execution order -- wouldn't using the external version type for example with the source timestamp work for you? (http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/docs-index_.html#_version_types) I'm assuming that you have an explicit document identifier and explicitly versioning documents makes a lot of sense when working with a distributed system. This way, it wouldn't even matter which server you send the request to and the actual operation speed of that particular server -- much less the exact execution order for requests coming from a given HTTP pipeline -- the latest version would be the one that sticks in any case.

@NickCraver

@nkvoll for that exact case, we could version the documents yes (though we have no other reason to was we only care about the current one). But we'd be explicitly doing so just to work around this bug.

Let's take another example we hit that causes Elasticsearch to be dropped from the project:

  1. Index a document.
  2. Do a GET for that document, by Id.

Since 2 can execute and return before 1, on the same node, even a getting a document you just stored isn't guaranteed, and bit us several times.

Not related to (regardless of) the execution order, let me address transport impact:

The transport switch (even if executed in order) is still a compounding issue. We were doing batch index operations for documents, GETting documents as well (all at high volume/speed) and checking cluster status along every so often as well to check index queue limits and such. We'd get cluster status responses to the document GET by Id requests. I don't think I have to impress just how bad that behavior is - and it's exactly what we hit many times that led us to this Github issue.

For all the documents that where the correct type and didn't throw deserialization errors as a result, we then had no confidence they were the right documents returned to the right requests. We know at least a decent portion of them weren't. As a result we had to throw away all of the data and start over. That's when the decision was made to redesign the data layout and abandon Elasticsearch for the project.

@kimchy
Member
kimchy commented Oct 30, 2014

@NickCraver I read pipelining wrong and actually part of the spec is not to allow to pipeline non-idempotent methods, and its the responsibility of the client not to do so (http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html). This heavily simplifies the implementation on our side, so I hope it happens quicker (@spinscale is on it, we are aiming for 1.4). So the question around order is not relevant.

Regarding the example you gave, of course pipelining will help on a single connection, I don't think there was any denying it.

@nkvoll
Member
nkvoll commented Oct 30, 2014

@NickCraver true, the document will not be available before the Lucene segment is written, which is after the default refresh_interval of 1 second (configurable, of course). You could do a an explicit call to _refresh (http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/indices-refresh.html#indices-refresh), but I'm not sure I'd recommend trying to work around the Near Real Time-properties of Elasticsearch/Lucene as much as work with it. I'm unaware of any planned changes to this part, as it's pretty central to achieving good indexing performance and coordinating shards across nodes, but then again, I'm not an Elasticsearch employee.

Regarding the transport level mix-up, I certainly feel you (which is why I created this issue in the first place). I think the worst part is that pipelining is advertised according to the HTTP 1.1 specification, but the actual results are wrong -- and that this might not be evident when doing small-scale development/testing runs. It can certainly catch unwary developers out quickly.

While maybe not optimal for your case, it's possible to work around this as well running a small proxy on the same server as your Elasticsearch instance that supports pipelining proper on the server side, but is possible to configure to not pipeline when forwarding requests to Elasticsearch. Whether this is a feasible approach depends on how you do operations and what systems you're comfortable with using. It's not at all optimal, but will get you closer to the goal until it's supported properly.

@Mpdreamz
Member

@NickCraver out of interest what are you using to issue a:

POST [index doc]
GET [doc]

On a pipelined http connection ?

To the best of my understanding HttpWebRequest (which the new HttpClient class still uses under cover by default) adheres to the RFC and won't pipeline any request that has a body:

http://referencesource.microsoft.com/#System/net/System/Net/_Connection.cs#800

and force it to wait on a new/free connection of it sees a request with a body on a pipelined connection:
http://referencesource.microsoft.com/#System/net/System/Net/_Connection.cs#614

As @kimchy pointed out The RFC disallows this:

Clients SHOULD NOT pipeline requests using non-idempotent methods or non-idempotent sequences of methods (see section 9.1.2). Otherwise, a premature termination of the transport connection could lead to indeterminate results. A client wishing to send a non-idempotent request SHOULD wait to send that request until it has received the response status for the previous request.

Of course the ordering issue still exists when doing 10 GETS sequentially on a pipelined connection and if they return out of order your application might end up updating the wrong documents (which should be fixed with the new pipelining support)

Please nudge me If my understanding of the HttpWebRequest in this regard is flawed!

@nkvoll elasticsearch should be realtime when indexing a single doc and then doing a single doc get provided the client waits for an ack on the client for the index and they are executed in the right order.

@spinscale spinscale added a commit to spinscale/elasticsearch that referenced this issue Oct 31, 2014
@spinscale spinscale Netty: Add HTTP pipelining support
This adds HTTP pipelining support to netty. Previously pipelining was not
supported due to the asynchronous nature of elasticsearch. The first request
that was returned by Elasticsearch, was returned as first response,
regardless of the correct order.

The solution to this problem is to add a handler to the netty pipeline
that maintains an ordered list and thus orders the responses before
returning them to the client. This means, we will always have some state
on the server side and also requires some memory in order to keep the
responses there.

Pipelining is enabled by default, but can be configured by setting the
http.pipelining property to true|false. In addition the maximum size of
the event queue can be configured.

The initial netty handler is copied from this repo
https://github.com/typesafehub/netty-http-pipelining

Closes #2665
5eeac2f
@spinscale spinscale closed this in #8299 Oct 31, 2014
@spinscale spinscale added a commit that referenced this issue Oct 31, 2014
@spinscale spinscale Netty: Add HTTP pipelining support
This adds HTTP pipelining support to netty. Previously pipelining was not
supported due to the asynchronous nature of elasticsearch. The first request
that was returned by Elasticsearch, was returned as first response,
regardless of the correct order.

The solution to this problem is to add a handler to the netty pipeline
that maintains an ordered list and thus orders the responses before
returning them to the client. This means, we will always have some state
on the server side and also requires some memory in order to keep the
responses there.

Pipelining is enabled by default, but can be configured by setting the
http.pipelining property to true|false. In addition the maximum size of
the event queue can be configured.

The initial netty handler is copied from this repo
https://github.com/typesafehub/netty-http-pipelining

Closes #2665
0b778e1
@spinscale spinscale added a commit that referenced this issue Oct 31, 2014
@spinscale spinscale Netty: Add HTTP pipelining support
This adds HTTP pipelining support to netty. Previously pipelining was not
supported due to the asynchronous nature of elasticsearch. The first request
that was returned by Elasticsearch, was returned as first response,
regardless of the correct order.

The solution to this problem is to add a handler to the netty pipeline
that maintains an ordered list and thus orders the responses before
returning them to the client. This means, we will always have some state
on the server side and also requires some memory in order to keep the
responses there.

Pipelining is enabled by default, but can be configured by setting the
http.pipelining property to true|false. In addition the maximum size of
the event queue can be configured.

The initial netty handler is copied from this repo
https://github.com/typesafehub/netty-http-pipelining

Closes #2665
7119947
@mute mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015
@spinscale spinscale Netty: Add HTTP pipelining support
This adds HTTP pipelining support to netty. Previously pipelining was not
supported due to the asynchronous nature of elasticsearch. The first request
that was returned by Elasticsearch, was returned as first response,
regardless of the correct order.

The solution to this problem is to add a handler to the netty pipeline
that maintains an ordered list and thus orders the responses before
returning them to the client. This means, we will always have some state
on the server side and also requires some memory in order to keep the
responses there.

Pipelining is enabled by default, but can be configured by setting the
http.pipelining property to true|false. In addition the maximum size of
the event queue can be configured.

The initial netty handler is copied from this repo
https://github.com/typesafehub/netty-http-pipelining

Closes #2665
8b0ecf6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment