Do not skip not available shard exception in search response#64337
Merged
jimczi merged 9 commits intoelastic:masterfrom Nov 24, 2020
Merged
Do not skip not available shard exception in search response#64337jimczi merged 9 commits intoelastic:masterfrom
jimczi merged 9 commits intoelastic:masterfrom
Conversation
Today search responses do not report failures for shard that were not available
for the search.
So if one shard is not assigned on a search over 5 shards, the
search response will report:
```
"_shards": {
"total": 5,
"successful": 4,
"skipped": 0,
"failed": 0
}
```
If all shards are unassigned, we report a generic search phase exception with no cause.
It's easy to spot that `successful` is less than `total` in the response but not reporting
the failure is misleading for users.
This change removes the special handling of not available shards exception in search responses
and treat them as any other failure that could occur on a shard.
These exceptions will count in the `failed` section and will be reported in details in
the `shard_failures` section.
If all shards are unavailable, the search API will now return 404 NOT_FOUND as an indication
that the search failed because it couldn't find any of the resources.
Closes elastic#47700
Collaborator
|
Pinging @elastic/es-search (:Search/Search) |
Collaborator
|
Pinging @elastic/es-distributed (:Distributed/Distributed) |
Contributor
henningandersen
left a comment
There was a problem hiding this comment.
Thanks @jimczi for addressing this. Besides the comments below, I wonder if we should also add relevant shard-failures to the allowPartialSearchResults=false case in AbstractSearchAsyncAction.run?
I think we can also remove the discrepancy check in executeNextPhase?
server/src/main/java/org/elasticsearch/action/search/AbstractSearchAsyncAction.java
Show resolved
Hide resolved
server/src/internalClusterTest/java/org/elasticsearch/search/basic/SearchRedStateIndexIT.java
Show resolved
Hide resolved
Contributor
|
One more minor nit is the comment here. |
Contributor
henningandersen
left a comment
There was a problem hiding this comment.
One more comment.
server/src/main/java/org/elasticsearch/action/search/SearchResponse.java
Show resolved
Hide resolved
jimczi
added a commit
that referenced
this pull request
Nov 24, 2020
Today search responses do not report failures for shard that were not available
for the search.
So if one shard is not assigned on a search over 5 shards, the
search response will report:
```
"_shards": {
"total": 5,
"successful": 4,
"skipped": 0,
"failed": 0
}
```
If all shards are unassigned, we report a generic search phase exception with no cause.
It's easy to spot that `successful` is less than `total` in the response but not reporting
the failure is misleading for users.
This change removes the special handling of not available shards exception in search responses
and treat them as any other failure that could occur on a shard.
These exceptions will count in the `failed` section and will be reported in details in
the `shard_failures` section.
If all shards are unavailable, the search API will now return 404 NOT_FOUND as an indication
that the search failed because it couldn't find any of the resources.
Closes #47700
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today search responses do not report failures for shard that were not available
for the search.
So if one shard is not assigned on a search over 5 shards, the
search response will report:
If all shards are unassigned, we report a generic search phase exception with no cause.
It's easy to spot that
successfulis less thantotalin the response but not reportingthe failure is misleading for users.
This change removes the special handling of not available shards exception in search responses
and treat them as any other failure that could occur on a shard.
These exceptions will count in the
failedsection and will be reported in details inthe
shard_failuressection.Closes #47700