浏览代码

Suppress stack traces from failed snapshot clones (#109495)

Extends the behaviour introduced in #105622 to cover clone-snapshot
operations as well as create-snapshot ones.
David Turner 1 年之前
父节点
当前提交
78587ab41a

+ 24 - 19
server/src/main/java/org/elasticsearch/snapshots/SnapshotsService.java

@@ -395,7 +395,7 @@ public final class SnapshotsService extends AbstractLifecycleComponent implement
             @Override
             public void onFailure(Exception e) {
                 initializingClones.remove(snapshot);
-                logger.warn(() -> format("[%s][%s] failed to clone snapshot", repositoryName, snapshotName), e);
+                logSnapshotFailure("clone", snapshot, e);
                 listener.onFailure(e);
             }
 
@@ -3845,28 +3845,33 @@ public final class SnapshotsService extends AbstractLifecycleComponent implement
 
         @Override
         public void onFailure(Exception e) {
-            final var logLevel = snapshotFailureLogLevel(e);
-            if (logLevel == Level.INFO && logger.isDebugEnabled() == false) {
-                // suppress stack trace at INFO unless extra verbosity is configured
-                logger.info(
-                    format(
-                        "[%s][%s] failed to create snapshot: %s",
-                        snapshot.getRepository(),
-                        snapshot.getSnapshotId().getName(),
-                        e.getMessage()
-                    )
-                );
-            } else {
-                logger.log(
-                    logLevel,
-                    () -> format("[%s][%s] failed to create snapshot", snapshot.getRepository(), snapshot.getSnapshotId().getName()),
-                    e
-                );
-            }
+            logSnapshotFailure("create", snapshot, e);
             listener.onFailure(e);
         }
     }
 
+    private static void logSnapshotFailure(String operation, Snapshot snapshot, Exception e) {
+        final var logLevel = snapshotFailureLogLevel(e);
+        if (logLevel == Level.INFO && logger.isDebugEnabled() == false) {
+            // suppress stack trace at INFO unless extra verbosity is configured
+            logger.info(
+                format(
+                    "[%s][%s] failed to %s snapshot: %s",
+                    snapshot.getRepository(),
+                    snapshot.getSnapshotId().getName(),
+                    operation,
+                    e.getMessage()
+                )
+            );
+        } else {
+            logger.log(
+                logLevel,
+                () -> format("[%s][%s] failed to %s snapshot", snapshot.getRepository(), snapshot.getSnapshotId().getName(), operation),
+                e
+            );
+        }
+    }
+
     private static Level snapshotFailureLogLevel(Exception e) {
         if (MasterService.isPublishFailureException(e)) {
             // no action needed, the new master will take things from here

+ 27 - 1
server/src/test/java/org/elasticsearch/snapshots/SnapshotResiliencyTests.java

@@ -1495,6 +1495,25 @@ public class SnapshotResiliencyTests extends ESTestCase {
                             fail("snapshot should not have started");
                         }
 
+                        @Override
+                        public void onFailure(Exception e) {
+                            assertThat(ExceptionsHelper.unwrapCause(e), instanceOf(SnapshotNameAlreadyInUseException.class));
+                            l.onResponse(null);
+                        }
+                    })
+            )
+            // attempt to clone snapshot
+            .<AcknowledgedResponse>andThen(
+                (l, ignored) -> client().admin()
+                    .cluster()
+                    .prepareCloneSnapshot(repoName, snapshotName, snapshotName)
+                    .setIndices("*")
+                    .execute(new ActionListener<>() {
+                        @Override
+                        public void onResponse(AcknowledgedResponse acknowledgedResponse) {
+                            fail("snapshot should not have started");
+                        }
+
                         @Override
                         public void onFailure(Exception e) {
                             assertThat(ExceptionsHelper.unwrapCause(e), instanceOf(SnapshotNameAlreadyInUseException.class));
@@ -1503,6 +1522,7 @@ public class SnapshotResiliencyTests extends ESTestCase {
                     })
             );
 
+        final var expectedMessage = Strings.format("Invalid snapshot name [%s], snapshot with the same name already exists", snapshotName);
         MockLog.assertThatLogger(() -> {
             deterministicTaskQueue.runAllRunnableTasks();
             assertTrue("executed all runnable tasks but test steps are still incomplete", testListener.isDone());
@@ -1513,7 +1533,13 @@ public class SnapshotResiliencyTests extends ESTestCase {
                 "INFO log",
                 SnapshotsService.class.getCanonicalName(),
                 Level.INFO,
-                Strings.format("*failed to create snapshot*Invalid snapshot name [%s]*", snapshotName)
+                Strings.format("*failed to create snapshot*%s", expectedMessage)
+            ),
+            new MockLog.SeenEventExpectation(
+                "INFO log",
+                SnapshotsService.class.getCanonicalName(),
+                Level.INFO,
+                Strings.format("*failed to clone snapshot*%s", expectedMessage)
             )
         );
     }