Fix Scroll responds with 503 when no shards match #143532

[thiagosteps227]

This change ensures that scroll_id is omitted when totalShards == 0, instead of returning misleading scroll ID.

In my thought, when no shards are involved in the search, there is no valid scroll to continue, so returning a _scroll_id would be 'incorrect'

Why didn't change the 503 behaviour:

Changing 503 handling would have more impact, also, this fix would be backwards safe.
The proposal now is: no shards -> no scroll context

• Have you signed the contributor license agreement?
• Have you followed the contributor guidelines?
• If submitting code, have you built your formula locally prior to submission with gradle check?
• If submitting code, is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed.
• If submitting code, have you checked that your submission is for an OS and architecture that we support?
• If you are submitting this code for a class then read our policy for that.

[elasticsearchmachine]

Pinging @elastic/es-search-foundations (Team:Search Foundations)

[benchaplin]

Sorry I've been so slow to review this @thiagosteps227! I need to think more carefully about the change your proposing. Just wanted to let you know it's on my list, hoping to get to it this week. Thanks for your contribution.

[benchaplin]

@elasticmachine test this please

[benchaplin]

I like the spirit of the change, I agree it seems pointless to return a scroll ID when no shards were searched.

However this would be a breaking API contract change - I don't think it's possible to do a "_search?scroll=..." and not receive a "_scroll_id" in the response. See https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-scroll:

To get the necessary scroll ID, submit a search API request that includes an argument for the scroll query parameter. The scroll parameter indicates how long Elasticsearch should retain the search context for the request. The search response returns a scroll ID in the _scroll_id response body parameter.

We're going to need to continue returning a scroll ID here, just with a 200 and empty hits.

[thiagosteps227]

Hey @benchaplin completely understand, and I imagined we could perhaps update the API docs, but that is fine too. I'll get back to the changes following your feedback, thanks very much for that and for letting me pick this issue. Really help to help. Cheers!!