Skip to content

Add phase failures to search responses #122788

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

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Feb 17, 2025

Second part of #116796, following on from #121784

Expose reranker failure information as part of a search response, in a new phase_failures field

Resolves #116796

@thecoop thecoop force-pushed the reranking-failure-response branch from ca28a76 to c26330c Compare February 17, 2025 16:12
@thecoop thecoop added >enhancement :Search Relevance/Ranking Scoring, rescoring, rank evaluation. labels Feb 17, 2025
@elasticsearchmachine
Copy link
Collaborator

Hi @thecoop, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Hi @thecoop, I've updated the changelog YAML for you.

@benwtrent
Copy link
Member

I think this idea has merit.

I would like to "see" what it looks like in an API response. I also don't think "subsidiaryFailures" is a good name. They key is that these are failures that would not "fail" the result that are unrelated to remote clusters or individual shards.

Looking at the API as it is now, I think the response would look like:

{
  "took": 20,
  "timed_out": false,
  "_shards": {
...
  },
  "subsidiaryFailures": [ { "phase": "rerank", "exception": "blah blah"} ],
  "hits": {
...
  }
}

maybe it should be called "phase_failures" or "additional_failures" or "partial_failures"? The key thing is that the failures:

  • Are unrelated to a shard or cluster
  • Do effect the search response, but not to the extent that the search is considered completely failed.

@thecoop
Copy link
Member Author

thecoop commented Feb 18, 2025

I like phase_failures. Although maybe it should be a bit more general than phase, which has a specific meaning for searches? operation_failures?

An example JSON structure can be seen https://github.com/elastic/elasticsearch/pull/122788/files#diff-e69bede2f385a8806c0d1784ed73c218fc172666cb9fbdf8fdde728bb4f12e57. I'll of course add more tests, docs, and examples once the core idea has been cleared.

@javanna
Copy link
Member

javanna commented Feb 18, 2025

Do you have an example of a failure that would fall in this new bucket? Are these coord level errors that don't fit into shard failures perhaps ? I have a hard time wrapping my head around this new concept. Also, where are these failures expected to be consumed?

@thecoop
Copy link
Member Author

thecoop commented Feb 18, 2025

This is used here. For background, see #116796 and #121784

@thecoop thecoop force-pushed the reranking-failure-response branch from 926a386 to 0873098 Compare February 21, 2025 13:52
@thecoop thecoop changed the title Add subsidiary failures to search responses Add phase failures to search responses Feb 21, 2025
@thecoop thecoop force-pushed the reranking-failure-response branch from d925c23 to 20b2c76 Compare February 21, 2025 16:59
Copy link
Contributor

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

@thecoop thecoop marked this pull request as ready for review February 21, 2025 17:00
@thecoop thecoop requested a review from a team as a code owner February 21, 2025 17:00
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Feb 21, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@thecoop thecoop added the WIP label Feb 21, 2025
Copy link
Contributor

Warning

It looks like this PR modifies one or more .asciidoc files. These files are being migrated to Markdown, and any changes merged now will be lost. See the migration guide for details.

@elasticsearchmachine
Copy link
Collaborator

Hi @thecoop, I've created a changelog YAML for you.

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good. I have some name changes that need addressed and we should have a yaml test.

One other thing is we need to ensure docs & specs are updated to include this change.

Also, could you post what the response looks like when there is a failure?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Relevance/Ranking Scoring, rescoring, rank evaluation. Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.1.0 WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to return non-reranked results if an error occurs during reranking
4 participants