Skip to content

Tests demonstrating inconsistencies in error handling when skip_unavailable=true #129957

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

smalyshev
Copy link
Contributor

In relation to the recent skip_unavailable=true change, there's still an inconsistency there. The change in #128163 covers most runtime errors, however it does not cover one code path, namely the one covered by CssPartialErrorsActionListener.onFailure - which handles failures that happen in analyzedPlan . There the code still has this condition:

        if (e instanceof NoClustersToSearchException || ExceptionsHelper.isRemoteUnavailableException(e)) {

which controls the code path where all remotes that we tried when attempting to resolve index failed, and we're about to return an empty result (we're talking about skip_unavailable=true here).
However, if the error that happened on the remote is not a disconnect error but some other error, unlike the runtime branch, skip_unavailable=true would not cover it, and we would still return a failure.

In addition to that, when other indices are present, the same error would instead be hidden by mergedMappings because it is currently only processes unavailability errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants