-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Fix Bug with Concurrent Snapshot and Index Delete #73456
Fix Bug with Concurrent Snapshot and Index Delete #73456
Conversation
Fix state machine bug that fixes the incorrect assumption a finished snapshot delete could only start shard snapshots when in fact it can also move snapshots to a completed state.
Pinging @elastic/es-distributed (Team:Distributed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested a couple of small tidy-ups but LGTM.
.../internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java
Outdated
Show resolved
Hide resolved
.../internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java
Outdated
Show resolved
Hide resolved
.../internalClusterTest/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java
Outdated
Show resolved
Hide resolved
changed = true; | ||
if (updatedEntry.state().completed()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry if this is a stupid remark but should we do the same thing for the cloning path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries :) The clone path isn't affected by this, the logic there is different and always sets the snapshot overall state correctly. Also, there cannot be a concurrent removal of shards for clones anyway :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦🏻 makes sense, thanks!
Fixes the incorrect assumption in the snapshot state machine that a finished snapshot delete could only start shard snapshots: in fact it can also move snapshots to a completed state.
Fixes the incorrect assumption in the snapshot state machine that a finished snapshot delete could only start shard snapshots: in fact it can also move snapshots to a completed state.
Adds docs to the release notes for 7.9-7.12 recording elastic#73456 as a known issue.
Thanks David!! |
* master: (1643 commits) Make DataStreamsSnapshotsIT resilient to failures because of local time. (elastic#73516) Upgrade netty to 4.1.63 (elastic#73011) [DOCS] Create a new page for dissect content in scripting docs (elastic#73437) Deprecate freeze index API (elastic#72618) [DOCS] Remove 'closed data stream' reference [DOCS] Update alias references (elastic#73427) [DOCS] Create a new page for grok content in scripting docs (elastic#73118) Remove dependency on azure shadowjar since it's no longer required [DOCS] Update backport policy for known issues (elastic#73489) Shadowed dependencies should be hidden from pom dependencies (elastic#73467) Disable transitive dependencies when resolving bwc JDBC driver artifact (elastic#73448) Print full JVM implementation version at start of build (elastic#73439) [DOCS] Update snapshot/restore for data stream aliases (elastic#73438) Upgrade Azure SDK and Jackson (elastic#72833) (elastic#72995) [DOCS] Fix typo (elastic#73337) (elastic#73474) [DOCS] Fix typo (elastic#73444) (elastic#73472) [DOCS] Update alias security for data stream aliases (elastic#73436) Fix Bug with Concurrent Snapshot and Index Delete (elastic#73456) [DOCS] Move common scripting use cases up a level (elastic#73445) Add more validation for data stream aliases. (elastic#73416) ...
The known-issue docs give the impression that an upgrade will restore the lost data in the repository. This isn't the case, so this commit clarifies this in the docs. Relates elastic#73456 Relates elastic#75598 Relates elastic#79221
Fix state machine bug that fixes the incorrect assumption a finished snapshot delete
could only start shard snapshots when in fact it can also move snapshots to a
completed state.