Skip to content

Improve get-snapshots behaviour for unreadable repositories #128277

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 2 commits into
base: main
Choose a base branch
from

Conversation

DaveCTurner
Copy link
Contributor

Today if the get-snapshots API cannot access one of the repositories we
return an exception with a fairly low-level message about the problem,
perhaps Could not determine repository generation from root blobs.
This message is shown verbatim in the Kibana UI so users need something
a little more descriptive. With this commit we wrap the exception in one
that indicates the problem in terms that users are more likely to
understand.

Moreover, if the user specifies the ?ignore_unavailable option then we
ignore individual snapshots that are unavailable, treating them as if
they do not exist, but an unavailable repository will still cause the
API to return an exception. With this commit we extend the meaning of
this option to also ignore whole-repository unavailability, treating
unavailable repositories as if they are empty.

Relates #128208

Today if the get-snapshots API cannot access one of the repositories we
return an exception with a fairly low-level message about the problem,
perhaps `Could not determine repository generation from root blobs`.
This message is shown verbatim in the Kibana UI so users need something
a little more descriptive. With this commit we wrap the exception in one
that indicates the problem in terms that users are more likely to
understand.

Moreover, if the user specifies the `?ignore_unavailable` option then we
ignore individual snapshots that are unavailable, treating them as if
they do not exist, but an unavailable repository will still cause the
API to return an exception. With this commit we extend the meaning of
this option to also ignore whole-repository unavailability, treating
unavailable repositories as if they are empty.

Relates elastic#128208
@elasticsearchmachine
Copy link
Collaborator

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

@DaveCTurner
Copy link
Contributor Author

I'm opening this for discussion with the team - it seems that we already have an option that enables (egregiously?) lenient behaviour in this API, so maybe we should just lean into this and make it even more lenient if the caller indicates that they just wants some kind of response regardless of whether it's correct or not.

Note that prior to #107191 we did report on repository-level (but not snapshot-level) failures in the response (see #114496 for the PR which removes this field entirely). We could in principle reinstate this response field, although if we did we should consider whether also to report on snapshot-level failures. It all gets very messy when pagination is involved.

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