Skip to content
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

validate snapshot has global state before restoring #82037

Merged
merged 6 commits into from
Jan 17, 2022

Conversation

idegtiarenko
Copy link
Contributor

Currently it is possible to restore with include_global_state: true from a snapshot that does not have a global state.
This behavior will nullify cluster state once #81373 is merged.
This pr adds explicit validation and error message to prevent damaging cluster state.

closes #82019

It is possible to restore from a snapshot with a global
state even if it does not have one. This pr adds validation
for this case.
@idegtiarenko idegtiarenko added >bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. auto-backport-and-merge v8.1.0 labels Dec 22, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@@ -158,7 +158,7 @@ public void testIncludeGlobalState() throws Exception {
logger.info("--> try restoring cluster state from snapshot without global state");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should revisit the log entries as well

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

Thanks Ievgen, this looks good. I would like to add documentation to the restore page explaining that you can only restore global state if the snapshot was taken with global state. Also, I wonder if we should not list it as breaking, only to highlight this to the few users who might be impacted.

@@ -170,6 +170,9 @@ ingest pipelines and {ilm-init} lifecycle policies that exist in your cluster
and replaces them with the corresponding items from the snapshot.

Use the `feature_states` parameter to configure how feature states are restored.

If `include_global_state` is `true` and a snapshot was created without a global
state then restore will fail.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@henningandersen please let me know if you have ideas as for other sections this note should be added to

@henningandersen
Copy link
Contributor

Docs preview link

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

@idegtiarenko idegtiarenko merged commit e7d8991 into elastic:master Jan 17, 2022
@idegtiarenko idegtiarenko deleted the 82019_validate_restore branch January 17, 2022 14:33
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.0 Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 82037

idegtiarenko added a commit to idegtiarenko/elasticsearch that referenced this pull request Jan 17, 2022
It is possible to restore from a snapshot with a global
state even if it does not have one. This pr adds validation
to prevent this from happening.

(cherry picked from commit e7d8991)
elasticsearchmachine pushed a commit that referenced this pull request Jan 17, 2022
It is possible to restore from a snapshot with a global
state even if it does not have one. This pr adds validation
to prevent this from happening.

(cherry picked from commit e7d8991)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking >bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.0.0 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fail with error when restoring global state from a snapshot that does not have one
4 participants