Create specific exception for when snapshots are in progress

delete and close index actions threw IllegalArgumentExceptions when attempting to run against an index that has a snapshot in progress. This change introduces a dedicated SnapshotInProgressException for these scenarios. This is done to explicitely signal to clients that this is the reason the action failed, and it is a retryable error. relates to elastic#37541.
talevy · Jan 16, 2019 · f713336 · f713336
1 parent ecf0de3
commit f713336
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 8 deletions.
diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java
@@ -1010,7 +1010,9 @@ private enum ElasticsearchExceptionHandle {
         COORDINATION_STATE_REJECTED_EXCEPTION(org.elasticsearch.cluster.coordination.CoordinationStateRejectedException.class,
             org.elasticsearch.cluster.coordination.CoordinationStateRejectedException::new, 150, Version.V_7_0_0),
         CLUSTER_ALREADY_BOOTSTRAPPED_EXCEPTION(org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException.class,
-            org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException::new, 151, Version.V_7_0_0);
+            org.elasticsearch.cluster.coordination.ClusterAlreadyBootstrappedException::new, 151, Version.V_7_0_0),
+        SNAPSHOT_IN_PROGRESS_EXCEPTION(org.elasticsearch.snapshots.SnapshotInProgressException.class,
+            org.elasticsearch.snapshots.SnapshotInProgressException::new, 152, Version.V_7_0_0);
 
         final Class<? extends ElasticsearchException> exceptionClass;
         final CheckedFunction<StreamInput, ? extends ElasticsearchException, IOException> constructor;

diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotInProgressException.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotInProgressException.java
@@ -0,0 +1,51 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.snapshots;
+
+import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.rest.RestStatus;
+
+import java.io.IOException;
+
+/**
+ * Thrown on the attempt to execute an action that requires
+ * that no snapshot is in progress.
+ */
+public class SnapshotInProgressException extends ElasticsearchException {
+
+    public SnapshotInProgressException(String msg) {
+        super(msg);
+    }
+
+    public SnapshotInProgressException(String msg, Throwable cause) {
+        super(msg, cause);
+    }
+
+    public SnapshotInProgressException(StreamInput in) throws IOException {
+        super(in);
+    }
+
+    @Override
+    public RestStatus status() {
+        return RestStatus.BAD_REQUEST;
+    }
+}
+
diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java
@@ -1447,7 +1447,7 @@ private ImmutableOpenMap<ShardId, SnapshotsInProgress.ShardSnapshotStatus> shard
     public static void checkIndexDeletion(ClusterState currentState, Set<IndexMetaData> indices) {
         Set<Index> indicesToFail = indicesToFailForCloseOrDeletion(currentState, indices);
         if (indicesToFail != null) {
-            throw new IllegalArgumentException("Cannot delete indices that are being snapshotted: " + indicesToFail +
+            throw new SnapshotInProgressException("Cannot delete indices that are being snapshotted: " + indicesToFail +
                 ". Try again after snapshot finishes or cancel the currently running snapshot.");
         }
     }
@@ -1459,7 +1459,7 @@ public static void checkIndexDeletion(ClusterState currentState, Set<IndexMetaDa
     public static void checkIndexClosing(ClusterState currentState, Set<IndexMetaData> indices) {
         Set<Index> indicesToFail = indicesToFailForCloseOrDeletion(currentState, indices);
         if (indicesToFail != null) {
-            throw new IllegalArgumentException("Cannot close indices that are being snapshotted: " + indicesToFail +
+            throw new SnapshotInProgressException("Cannot close indices that are being snapshotted: " + indicesToFail +
                 ". Try again after snapshot finishes or cancel the currently running snapshot.");
         }
     }

diff --git a/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java
@@ -79,6 +79,7 @@
 import org.elasticsearch.snapshots.Snapshot;
 import org.elasticsearch.snapshots.SnapshotException;
 import org.elasticsearch.snapshots.SnapshotId;
+import org.elasticsearch.snapshots.SnapshotInProgressException;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.TestSearchContext;
 import org.elasticsearch.test.VersionUtils;
@@ -809,6 +810,7 @@ public void testIds() {
         ids.put(149, MultiBucketConsumerService.TooManyBucketsException.class);
         ids.put(150, CoordinationStateRejectedException.class);
         ids.put(151, ClusterAlreadyBootstrappedException.class);
+        ids.put(152, SnapshotInProgressException.class);
 
         Map<Class<? extends ElasticsearchException>, Integer> reverse = new HashMap<>();
         for (Map.Entry<Integer, Class<? extends ElasticsearchException>> entry : ids.entrySet()) {

diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataDeleteIndexServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataDeleteIndexServiceTests.java
@@ -32,6 +32,7 @@
 import org.elasticsearch.repositories.IndexId;
 import org.elasticsearch.snapshots.Snapshot;
 import org.elasticsearch.snapshots.SnapshotId;
+import org.elasticsearch.snapshots.SnapshotInProgressException;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.VersionUtils;
 
@@ -63,7 +64,7 @@ SnapshotsInProgress.State.INIT, singletonList(new IndexId(index, "doesn't matter
         ClusterState state = ClusterState.builder(clusterState(index))
                 .putCustom(SnapshotsInProgress.TYPE, snaps)
                 .build();
-        Exception e = expectThrows(IllegalArgumentException.class,
+        Exception e = expectThrows(SnapshotInProgressException.class,
                 () -> service.deleteIndices(state, singleton(state.metaData().getIndices().get(index).getIndex())));
         assertEquals("Cannot delete indices that are being snapshotted: [[" + index + "]]. Try again after snapshot finishes "
                 + "or cancel the currently running snapshot.", e.getMessage());

diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/MetaDataIndexStateServiceTests.java
@@ -46,6 +46,7 @@
 import org.elasticsearch.repositories.IndexId;
 import org.elasticsearch.snapshots.Snapshot;
 import org.elasticsearch.snapshots.SnapshotId;
+import org.elasticsearch.snapshots.SnapshotInProgressException;
 import org.elasticsearch.test.ESTestCase;
 
 import java.util.Arrays;
@@ -157,7 +158,7 @@ public void testAddIndexClosedBlocks() {
             assertFalse(blockedIndices.containsKey(closed));
         }
         {
-            IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> {
+            SnapshotInProgressException exception = expectThrows(SnapshotInProgressException.class, () -> {
                     ClusterState state = addRestoredIndex("restored", randomIntBetween(1, 3), randomIntBetween(0, 3), initialState);
                     if (randomBoolean()) {
                         state = addOpenedIndex("opened", randomIntBetween(1, 3), randomIntBetween(0, 3), state);
@@ -171,7 +172,7 @@ public void testAddIndexClosedBlocks() {
             assertThat(exception.getMessage(), containsString("Cannot close indices that are being restored: [[restored]]"));
         }
         {
-            IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> {
+            SnapshotInProgressException exception = expectThrows(SnapshotInProgressException.class, () -> {
                 ClusterState state = addSnapshotIndex("snapshotted", randomIntBetween(1, 3), randomIntBetween(0, 3), initialState);
                 if (randomBoolean()) {
                     state = addOpenedIndex("opened", randomIntBetween(1, 3), randomIntBetween(0, 3), state);

diff --git a/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java b/server/src/test/java/org/elasticsearch/snapshots/SharedClusterSnapshotRestoreIT.java
@@ -2492,15 +2492,15 @@ public void testCloseOrDeleteIndexDuringSnapshot() throws Exception {
                         logger.info("--> delete index while non-partial snapshot is running");
                         client.admin().indices().prepareDelete("test-idx-1").get();
                         fail("Expected deleting index to fail during snapshot");
-                    } catch (IllegalArgumentException e) {
+                    } catch (SnapshotInProgressException e) {
                         assertThat(e.getMessage(), containsString("Cannot delete indices that are being snapshotted: [[test-idx-1/"));
                     }
                 } else {
                     try {
                         logger.info("--> close index while non-partial snapshot is running");
                         client.admin().indices().prepareClose("test-idx-1").get();
                         fail("Expected closing index to fail during snapshot");
-                    } catch (IllegalArgumentException e) {
+                    } catch (SnapshotInProgressException e) {
                         assertThat(e.getMessage(), containsString("Cannot close indices that are being snapshotted: [[test-idx-1/"));
                     }
                 }