Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider using 207 Multi-Status for responses with varying operation results #60442

Open
russcam opened this issue Jul 30, 2020 · 7 comments
Open
Labels
:Core/Infra/REST API REST infrastructure and utilities >enhancement help wanted adoptme Team:Core/Infra Meta label for core/infra team team-discuss

Comments

@russcam
Copy link
Contributor

russcam commented Jul 30, 2020

Related issue: #41434

The bulk API returns a HTTP 200 status response in the case that all operations have successfully completed, but also in the case where one or more of the operations has not. In the case that one or more operations did not complete successfully, the response body will return an "error": true property in the result, along with an "items" array that contains the result of each operation, where a result contains a "status" property with the HTTP status code that aligns with the operation result.

Currently, it is not possible to determine if any of the operations have failed without reading at least part of the response body to ascertain if it contains "error": true. For some client implementations, the client API may not expose a convenient way to read part of the body, requiring the whole response body to be read and deserialized.

Assuming

  • the bulk API is largely used to send a homogeneous collection of index operations
  • under normal operating conditions, the majority of operations are successful

Needing to read at least part of the response body to determine if there are errors can end up being inefficient for high volume ingestion.

Proposal

Consider using 207 Multi-Status HTTP code to indicate when the response contains heterogeneous operation results.

Scenario HTTP status code
All successful index operations 200
Mix of successful and failed index operations 207
All failed index operations 207
All successful update operations 200
Mix of successful and failed update operations 207
All failed update operations 207
All successful create operations 201
Mix of successful and failed create operations 207
All failed create operations 207
All successful delete operations 200
Mix of successful and failed delete operations 207
All failed delete operations 207
Mix of successful index, update, delete operations 200
Mix of successful index, update, delete, create operations 207
Mix of successful and failed index, update, delete, create operations 207
All failed index, update, delete, create operations 207

With such an approach, it is easy to distinguish between a bulk response of homogeneous operations containing all successful results or containing at least one error using the response status code, and using this to determine whether to read the response body.

Note that a bulk response of All successful create operations would return a HTTP 201 response, since the status of all operations is 201. Thus, any mix of at least one create operation with index, update or delete operations would return HTTP 207 response whether all operations are successful or not, because a successful create operation returns 201 and a successful index, update and delete operation returns 200.

@russcam russcam added >enhancement needs:triage Requires assignment of a team area label labels Jul 30, 2020
@Mpdreamz
Copy link
Member

Love this idea!

I think this a great idea!

I do wonder if we need 201 as a special edge case for all create operations that succeeded. Ideally the bulk indicates all bulk requests were accepted and executed was well (200) or all operations accepted but one or more did not succeed (207).

This also goes hand in hand nicely with #55088

@russcam
Copy link
Contributor Author

russcam commented Jul 31, 2020

I do wonder if we need 201 as a special edge case for all create operations that succeeded. Ideally the bulk indicates all bulk requests were accepted and executed was well (200) or all operations accepted but one or more did not succeed (207).

It may be a better fit to use a 207 response for Elasticsearch as you suggest since a successful index operation can return a 201 response in the case the document did not exist, and a 200 response in the case it did, which I originally overlooked.

In the case of all successful index operations where some may return 201 responses and some may return 200 responses, my understanding of the Multi-Status Response RFC is that the overall response code in this case should be 207 because there are multiple status codes. In following this however, it no longer becomes possible to distinguish between a response with all successful operations but multiple statuses and a response with some successful and some failed operations, which to me seems much more useful. Using

Scenario HTTP status code
All successful operations 200
One or more failed operations 207

might perhaps be considered to change the semantic meaning of a 207 response, but this issue already proposes using the code outside of its intended WebDAV usage anyway.

@gwbrown gwbrown added :Core/Infra/REST API REST infrastructure and utilities and removed needs:triage Requires assignment of a team area label labels Aug 5, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/REST API)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Aug 5, 2020
@colings86 colings86 removed the discuss label Oct 1, 2020
@rjernst rjernst added the needs:triage Requires assignment of a team area label label Dec 3, 2020
@jaymode jaymode added help wanted adoptme and removed needs:triage Requires assignment of a team area label labels Dec 10, 2020
@ezimuel
Copy link
Contributor

ezimuel commented Aug 10, 2021

@russcam I like the idea but unfortunately if we want to support 207 Multi-Status we need to follow the RFC 4918 that is specific for WebDAV, including the use of XML format for the body.
It seems not possible to use JSON to specify multiple response codes, as discussed here.

@hrishikeshdkakkad
Copy link

hrishikeshdkakkad commented Aug 8, 2023

Hi, if this is open can I take it up? @jaymode

@mosche
Copy link
Contributor

mosche commented Dec 13, 2024

Requires more discussion if 207 Multi-Status isn't an applicable approach

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/REST API REST infrastructure and utilities >enhancement help wanted adoptme Team:Core/Infra Meta label for core/infra team team-discuss
Projects
None yet
Development

No branches or pull requests