From 638701497680bd177494de70895849ad02d3d7a1 Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Wed, 6 Feb 2019 11:08:59 +0100 Subject: [PATCH 1/2] Fix Issue with Concurrent Snapshot Init + Delete * Closes #38489 --- .../snapshots/SnapshotsService.java | 19 ++++++++++++++++--- .../DedicatedClusterSnapshotRestoreIT.java | 1 - 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index e2628fda991bd..4dbdc27e18c4a 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -331,7 +331,6 @@ public void onFailure(final Exception e) { public TimeValue timeout() { return request.masterNodeTimeout(); } - }); } @@ -394,6 +393,8 @@ private void beginSnapshot(final ClusterState clusterState, boolean snapshotCreated; + boolean hadAbortedInitializations; + @Override protected void doRun() { assert initializingSnapshots.contains(snapshot.snapshot()); @@ -433,6 +434,8 @@ public ClusterState execute(ClusterState currentState) { if (entry.state() == State.ABORTED) { entries.add(entry); + assert entry.shards().isEmpty(); + hadAbortedInitializations = true; } else { // Replace the snapshot that was just initialized ImmutableOpenMap shards = @@ -491,6 +494,16 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS // completion listener in this method. For the snapshot completion to work properly, the snapshot // should still exist when listener is registered. userCreateSnapshotListener.onResponse(snapshot.snapshot()); + + if (hadAbortedInitializations) { + final SnapshotsInProgress snapshotsInProgress = newState.custom(SnapshotsInProgress.TYPE); + if (snapshotsInProgress != null) { + final SnapshotsInProgress.Entry entry = snapshotsInProgress.snapshot(snapshot.snapshot()); + if (entry != null) { + endSnapshot(entry); + } + } + } } }); } @@ -701,8 +714,8 @@ public void applyClusterState(ClusterChangedEvent event) { // 3. Snapshots in any other state that have all their shard tasks completed snapshotsInProgress.entries().stream().filter( entry -> entry.state().completed() - || entry.state() == State.INIT && initializingSnapshots.contains(entry.snapshot()) == false - || entry.state() != State.INIT && completed(entry.shards().values()) + || initializingSnapshots.contains(entry.snapshot()) == false + && (entry.state() == State.INIT || completed(entry.shards().values())) ).forEach(this::endSnapshot); } if (newMaster) { diff --git a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java index b118d3a3d4933..433cbe2fdaa6c 100644 --- a/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java +++ b/server/src/test/java/org/elasticsearch/snapshots/DedicatedClusterSnapshotRestoreIT.java @@ -855,7 +855,6 @@ public void testMasterShutdownDuringSnapshot() throws Exception { assertEquals(0, snapshotInfo.failedShards()); } - public void testMasterAndDataShutdownDuringSnapshot() throws Exception { logger.info("--> starting three master nodes and two data nodes"); internalCluster().startMasterOnlyNodes(3); From 5aafa447e78e16b3d1caa7c99fcd20dc9c54e09d Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Thu, 7 Feb 2019 10:07:19 +0100 Subject: [PATCH 2/2] assert instead of if --- .../org/elasticsearch/snapshots/SnapshotsService.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index 4dbdc27e18c4a..f6ed3eb75d859 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -497,12 +497,10 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS if (hadAbortedInitializations) { final SnapshotsInProgress snapshotsInProgress = newState.custom(SnapshotsInProgress.TYPE); - if (snapshotsInProgress != null) { - final SnapshotsInProgress.Entry entry = snapshotsInProgress.snapshot(snapshot.snapshot()); - if (entry != null) { - endSnapshot(entry); - } - } + assert snapshotsInProgress != null; + final SnapshotsInProgress.Entry entry = snapshotsInProgress.snapshot(snapshot.snapshot()); + assert entry != null; + endSnapshot(entry); } } });