Add Warning Http Header when one ore more documents fails in bulk

[snakefoot]

Have this BytesResponse from a bulk-operation, where Success = true:

Successful (200) low level call on POST: /_bulk
{
"took":0,
"errors":true,
"items":[{"index":{"_index":"logstash-2021.07.12","_type":"_doc","_id":"OTgmmnoBq3Eup03vE24t","status":403,"error":{"type":"security_exception","reason":"action [indices:data/write/bulk[s]] is unauthorized for API key id [NzgjmnoBq3Eup03vg25H] of user [elastic] on indices [logstash-2021.07.12], this action is granted by the index privileges [create_doc,create,delete,index,write,all]"}

And when checking Http-Headers then there is no "Warning" header:

responseMessage.Headers.TryGetValues("Warning", out warnings);

Please add "Warning"-header to also signal that "One ore more items with errors", when no other "deprecated warnings". Then one can react on this and deserialize the entire response to diagnose the issue (Avoid spending time on deserializing succesful responses, when no warnings).

See also: elastic/elastic-transport-net#30

[elasticmachine]

Pinging @elastic/es-distributed (Team:Distributed)

[DaveCTurner]

IMO the question of whether to add a header in this case is much more general than just for POST /_bulk: many APIs will return 200 OK to indicate that the request was coordinated successfully even if it encountered one or more failures after coordination, and we could reasonably indicate whether we were completely successful or not in the headers returned from all of them. It's also a question for the client folks whether/how users could indicate via the client APIs that they only care about the response body if it contains this sort of failure.

As it stands today we deliberately include status info within the first few bytes (e.g. "errors":true or the number of failed nodes or shards) so a streaming parser can start to read the body and bail out quickly if it decides it doesn't care any more. I've no idea if any of the official clients actually use this fact but I've certainly seen folks doing this with a low-level HTTP client.

As such I'd like the clients team to discuss this idea instead.

[DaveCTurner]

An alternative idea, specifically for the _bulk API, would be to add a parameter that suppresses all the document-level responses if there's no errors. I see a lot of clients that check the errors field and if it's false they simply discard the rest; if we had a way for the client to tell us it's going to do that we could save ourselves the bother of even sending all that junk over the wire.

[Mpdreamz]

@DaveCTurner #55088 😄

[snakefoot]

I'm mostly interested in having the feature in the LowLevel Client, so one can use BytesResponse, and only deserialize when the reply-payload is worth deserializing.

[DaveCTurner]

There's no need to change anything in Elasticsearch to solve this in the Java low-level REST client, you can just start to parse the body and stop at the errors field if it's false. It's the 2nd or 3rd field, you shouldn't need to look at more than ~60 bytes of body.

[snakefoot]

I guess if the BytesResponse in the low-level-client received a nice method that could do that quick-scan without loading up memory-stream, stream-readers with decoding etc. And then quickly return whether the reply-payload should be fully deserialized to reveal the actual warnings.

[DaveCTurner]

We (the @elastic/es-distributed team) discussed this today and want to propose an alternative mode(*) in which we only return the errors in all cases, because this would actually let us add some nice optimisations (data-transfer reductions) within Elasticsearch itself, and would save data transfer and client-side effort in the very common case that you only care about the errors. The main question is how to correlate the errors with the original docs, we could either pad the items array with null or replace it with a map keyed by the index of the doc that had the error (numbers in strings woo) add another key to each per-item map containing its original index. Both should work but neither sounds super-neat. I've asked the clients folks whether they have a preference, but I like the extra-key idea.

(*) edit to add that the default behaviour would remain as it is today, clients would need to opt-in to this alternative mode

[snakefoot]

I'm happy if the "alterntive mode" becomes available through the DotNet LowLevel-client. And I would be really happy if BytesResponse provided a userfriendly (and fast) way to check for any errors.

[DaveCTurner]

The alternative mode would certainly be available in the low-level client (you can do anything there) but I expect we'd add it to the high-level client too. I also expect that the method for checking for any errors would remain the same as it is today, but it'd be much faster since in the no-errors case the response would be tiny and therefore trivial to parse.