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

Assert aborted snapshots have no pending shard work #102621

Conversation

DaveCTurner
Copy link
Contributor

When a snapshot is aborted, all shard snapshots must either be complete
(SUCCESS, FAILED or MISSING) or else they must be in state
ABORTED while the data node finishes its work. It seems we do not
check this invariant anywhere today, so this commit adds an assertion of
this invariant.

When a snapshot is aborted, all shard snapshots must either be complete
(`SUCCESS`, `FAILED` or `MISSING`) or else they must be in state
`ABORTED` while the data node finishes its work. It seems we do not
check this invariant anywhere today, so this commit adds an assertion of
this invariant.
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.12.0 labels Nov 26, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team label Nov 26, 2023
Copy link
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

LGTM

Just need to fix the test failure which is due to the randomized State for SnapshotsInProgress#Entry not respecting the new assertion.

@DaveCTurner DaveCTurner added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 27, 2023
@DaveCTurner DaveCTurner removed the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 27, 2023
@DaveCTurner DaveCTurner added the auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Nov 27, 2023
@elasticsearchmachine elasticsearchmachine merged commit 4dd17dc into elastic:main Nov 27, 2023
14 checks passed
@DaveCTurner DaveCTurner deleted the 2023/11/26/assert-no-pending-work-in-aborted-snapshot branch November 27, 2023 11:01
timgrein pushed a commit to timgrein/elasticsearch that referenced this pull request Nov 30, 2023
When a snapshot is aborted, all shard snapshots must either be complete
(`SUCCESS`, `FAILED` or `MISSING`) or else they must be in state
`ABORTED` while the data node finishes its work. It seems we do not
check this invariant anywhere today, so this commit adds an assertion of
this invariant.
@DaveCTurner DaveCTurner restored the 2023/11/26/assert-no-pending-work-in-aborted-snapshot branch June 17, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Meta label for distributed team >test Issues or PRs that are addressing/adding tests v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants