From f7d9e7bdc48e04e66f6d71de0929ab79c56efa2e Mon Sep 17 00:00:00 2001 From: Armin Braun Date: Mon, 18 Nov 2019 14:12:55 +0100 Subject: [PATCH] Better Exceptions on Concurrent Snapshot Operations (#49220) (#49237) * Better Exceptions on Concurrent Snapshot Operations It is somewhat tricky to debug test failures from concurrent operations without having the exact knowledge of what ran concurrently so I added it to these exceptions in all spots. --- .../cleanup/TransportCleanupRepositoryAction.java | 9 ++++++--- .../org/elasticsearch/cluster/RestoreInProgress.java | 5 ++++- .../org/elasticsearch/snapshots/SnapshotsService.java | 11 ++++++----- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java index 15e58e91650..e054ad266a5 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/repositories/cleanup/TransportCleanupRepositoryAction.java @@ -176,15 +176,18 @@ public final class TransportCleanupRepositoryAction extends TransportMasterNodeA final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { throw new IllegalStateException( - "Cannot cleanup [" + repositoryName + "] - a repository cleanup is already in-progress"); + "Cannot cleanup [" + repositoryName + "] - a repository cleanup is already in-progress in [" + + repositoryCleanupInProgress + "]"); } SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); if (deletionsInProgress != null && deletionsInProgress.hasDeletionsInProgress()) { - throw new IllegalStateException("Cannot cleanup [" + repositoryName + "] - a snapshot is currently being deleted"); + throw new IllegalStateException("Cannot cleanup [" + repositoryName + + "] - a snapshot is currently being deleted in [" + deletionsInProgress + "]"); } SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); if (snapshots != null && !snapshots.entries().isEmpty()) { - throw new IllegalStateException("Cannot cleanup [" + repositoryName + "] - a snapshot is currently running"); + throw new IllegalStateException( + "Cannot cleanup [" + repositoryName + "] - a snapshot is currently running in [" + snapshots + "]"); } return ClusterState.builder(currentState).putCustom(RepositoryCleanupInProgress.TYPE, new RepositoryCleanupInProgress( diff --git a/server/src/main/java/org/elasticsearch/cluster/RestoreInProgress.java b/server/src/main/java/org/elasticsearch/cluster/RestoreInProgress.java index 63c2ea52597..0bea73ff1c9 100644 --- a/server/src/main/java/org/elasticsearch/cluster/RestoreInProgress.java +++ b/server/src/main/java/org/elasticsearch/cluster/RestoreInProgress.java @@ -82,7 +82,10 @@ public class RestoreInProgress extends AbstractNamedDiffable implements @Override public String toString() { - return new StringBuilder("RestoreInProgress[").append(entries).append("]").toString(); + StringBuilder builder = new StringBuilder("RestoreInProgress["); + entries.forEach(entry -> builder.append("{").append(entry.key).append("}{").append(entry.value.snapshot).append("},")); + builder.setCharAt(builder.length() - 1, ']'); + return builder.toString(); } public Entry get(String restoreUUID) { diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java index bfc3319ada1..01e061f4a89 100644 --- a/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java +++ b/server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java @@ -278,12 +278,12 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); if (deletionsInProgress != null && deletionsInProgress.hasDeletionsInProgress()) { throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, - "cannot snapshot while a snapshot deletion is in-progress"); + "cannot snapshot while a snapshot deletion is in-progress in [" + deletionsInProgress + "]"); } final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { throw new ConcurrentSnapshotExecutionException(repositoryName, snapshotName, - "cannot snapshot while a repository cleanup is in-progress"); + "cannot snapshot while a repository cleanup is in-progress in [" + repositoryCleanupInProgress + "]"); } SnapshotsInProgress snapshots = currentState.custom(SnapshotsInProgress.TYPE); if (snapshots == null || snapshots.entries().isEmpty()) { @@ -1195,12 +1195,12 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus SnapshotDeletionsInProgress deletionsInProgress = currentState.custom(SnapshotDeletionsInProgress.TYPE); if (deletionsInProgress != null && deletionsInProgress.hasDeletionsInProgress()) { throw new ConcurrentSnapshotExecutionException(snapshot, - "cannot delete - another snapshot is currently being deleted"); + "cannot delete - another snapshot is currently being deleted in [" + deletionsInProgress + "]"); } final RepositoryCleanupInProgress repositoryCleanupInProgress = currentState.custom(RepositoryCleanupInProgress.TYPE); if (repositoryCleanupInProgress != null && repositoryCleanupInProgress.cleanupInProgress() == false) { throw new ConcurrentSnapshotExecutionException(snapshot.getRepository(), snapshot.getSnapshotId().getName(), - "cannot delete snapshot while a repository cleanup is in-progress"); + "cannot delete snapshot while a repository cleanup is in-progress in [" + repositoryCleanupInProgress + "]"); } RestoreInProgress restoreInProgress = currentState.custom(RestoreInProgress.TYPE); if (restoreInProgress != null) { @@ -1208,7 +1208,8 @@ public class SnapshotsService extends AbstractLifecycleComponent implements Clus // otherwise we could end up deleting a snapshot that is being restored // and the files the restore depends on would all be gone if (restoreInProgress.isEmpty() == false) { - throw new ConcurrentSnapshotExecutionException(snapshot, "cannot delete snapshot during a restore"); + throw new ConcurrentSnapshotExecutionException(snapshot, + "cannot delete snapshot during a restore in progress in [" + restoreInProgress + "]"); } } ClusterState.Builder clusterStateBuilder = ClusterState.builder(currentState);