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

Should Elasticsearch return a non-200 response if there are shard failures? #18978

Closed
nik9000 opened this issue Jun 20, 2016 · 8 comments
Closed
Labels
:Core/Infra/REST API REST infrastructure and utilities discuss v5.0.0-alpha4

Comments

@nik9000
Copy link
Member

nik9000 commented Jun 20, 2016

Lots of Elasticsearch tasks are forked onto a bunch of shards. When those shards fail, Elasticsearch returns the failures in the json at _shards.failures including the HTTP response code that that failure deserves but it will return an HTTP 200 response code so long as a single shard succeeds. I think it should return a non-200 HTTP status if any shard fails.

Do we want to:

  1. Continue as we are and return a 200 code if any shard succeeds.
  2. Return a non-200 code if any shards fail. We'd return the highest numbered failure because that is what we do now if all shards fail. You can get each response code in the failures array in the response. We could also talk about RFC 4918's 207 Multi-Status but at first glance that specifies some XML response we aren't going to implement.
  3. Add a boolean to the request to toggle between the two behaviors. We'd have to pick a default but we could default to the old behavior for 5.0 if we didn't want the change to be breaking.
@clintongormley
Copy link
Contributor

Add a boolean to the request to toggle between the two behaviors.

Please no.

Return a non-200 code if any shards fail.

This would cause all of the clients to report an exception instead of returning partial results. This is not ideal, eg I'd rather see some results than none at all.

Given that there isn't an HTTP status that indicates partial success, I vote to keep it as 200

@jpountz
Copy link
Contributor

jpountz commented Jun 20, 2016

This is something that bugged me in the past as well, but I do not feel too strongly about it since we give users all the information they need to do whathever they want to do on application side (raise an error, show a big red warning that there is a partial failure, etc.). I am a bit torn as to whether it would be worth maitaining a parameter or setting for this. So I think I am leaning towards 1.

@bleskes
Copy link
Contributor

bleskes commented Jun 20, 2016

+1 to 1 . So sad we don’t have a partial response status code.

On 20 Jun 2016, at 18:00, Adrien Grand [email protected] wrote:

This is something that bugged me in the past as well, but I do not feel too strongly about it since we give users all the information they need to do whathever they want to do on application side (raise an error, show a big red warning that there is a partial failure, etc.). I am a bit torn as to whether it would be worth maitaining a parameter or setting for this. So I think I am leaning towards 1.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@nik9000
Copy link
Member Author

nik9000 commented Jun 20, 2016

207 is a partial response code but it is all mixed up with WebDAV....

I still think it'd be much more intuitive to return a non-200 and have the client check if the request looks like a partial failure rather than a complete one....

I know we don't check it all the right places in our tests.

@nik9000 nik9000 closed this as completed Jun 20, 2016
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Jun 20, 2016
ES only sends a non-200 response all shards fail but we should
fail the tests generated by docs if any of them fail.

Depending on the outcome of elastic#18978 this might be a temporary
workaround.
@BigBrain-Industries
Copy link

This is IMHO, a poor design decision and goes against a fail-fast system.
Imagine what happens when people only sometimes see the complete search results. They'll be spending a long time to figure out what is wrong.

@clintongormley
Copy link
Contributor

@TheGreatYamcha this is intentional, eg we'd rather show you some of your friends' updates instead of none. The response includes the _shards parameter which tells you how many shards failed or succeeded which you can use if you prefer to only show complete results.

@nezda
Copy link

nezda commented Nov 11, 2018

This was added after all as boolean query parameter allow_partial_search_results See https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-body.html) via issue #27435 and comments indicate default may become true!

@danielsnider
Copy link

The allow_partial_search_results setting must be passed as query-string parameters but this isn't possible with the tool that I'm using (ReactiveSearch) A global config option would be better for me. Or an option in the request body.

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 discuss v5.0.0-alpha4
Projects
None yet
Development

No branches or pull requests

7 participants