Skip to content

Fix BwC Snapshot INIT Path (#60006)#60038

Merged
original-brownbear merged 1 commit intoelastic:7.9from
original-brownbear:60006-7.9
Jul 22, 2020
Merged

Fix BwC Snapshot INIT Path (#60006)#60038
original-brownbear merged 1 commit intoelastic:7.9from
original-brownbear:60006-7.9

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

There were two subtle bugs here from backporting #56911 to 7.x.

  1. We passed null for the shards map which isn't nullable any longer
    when creating SnapshotsInProgress.Entry, fixed by just passing an empty map
    like the null handling did in the past.
  2. The removal of a failed INIT state snapshot from the cluster state tried
    removing it from the finalization loop (the set of repository names that are
    currently finalizing). This will trip an assertion since the snapshot failed
    before its repository was put into the set. I made the logic ignore the set
    in case we remove a failed INIT state snapshot to restore the old logic to
    exactly as it was before the concurrent snapshots backport to be on the safe
    side here.

Also, added tests that explicitly call the old code paths because as can be seen
from initially missing this, the BwC tests will only run in the configuration new
version master, old version nodes ever so often and having a deterministic test
for the old state machine seems the safest bet here.

Closes #59986

backport of #60006

There were two subtle bugs here from backporting #56911 to 7.x.

1. We passed `null` for the `shards` map which isn't nullable any longer
when creating `SnapshotsInProgress.Entry`, fixed by just passing an empty map
like the `null` handling did in the past.
2. The removal of a failed `INIT` state snapshot from the cluster state tried
removing it from the finalization loop (the set of repository names that are
currently finalizing). This will trip an assertion since the snapshot failed
before its repository was put into the set. I made the logic ignore the set
in case we remove a failed `INIT` state snapshot to restore the old logic to
exactly as it was before the concurrent snapshots backport to be on the safe
side here.

Also, added tests that explicitly call the old code paths because as can be seen
from initially missing this, the BwC tests will only run in the configuration new
version master, old version nodes ever so often and having a deterministic test
for the old state machine seems the safest bet here.

Closes #59986
@original-brownbear original-brownbear added :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs backport labels Jul 22, 2020
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Jul 22, 2020
@original-brownbear original-brownbear merged commit 3ccfebe into elastic:7.9 Jul 22, 2020
@original-brownbear original-brownbear deleted the 60006-7.9 branch July 22, 2020 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed Meta label for distributed team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants