Browse Source

Suppress ShardNotInPrimaryModeException warning in retention lease sync (#97051)

A `ShardNotInPrimaryModeException` is sometimes expected here if the
primary is relocated concurrently with the sync, so we shouldn't report
it at `WARN` level.

Also with this change we log the exception at `DEBUG` in all cases.
David Turner 2 years ago
parent
commit
87123e35d8

+ 15 - 14
server/src/main/java/org/elasticsearch/index/seqno/RetentionLeaseSyncAction.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.index.seqno;
 
+import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.apache.lucene.store.AlreadyClosedException;
@@ -33,6 +34,7 @@ import org.elasticsearch.index.IndexingPressure;
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.shard.IndexShardClosedException;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.index.shard.ShardNotInPrimaryModeException;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.indices.SystemIndices;
 import org.elasticsearch.tasks.Task;
@@ -60,10 +62,6 @@ public class RetentionLeaseSyncAction extends TransportWriteAction<
     public static final String ACTION_NAME = "indices:admin/seq_no/retention_lease_sync";
     private static final Logger LOGGER = LogManager.getLogger(RetentionLeaseSyncAction.class);
 
-    protected static Logger getLogger() {
-        return LOGGER;
-    }
-
     @Inject
     public RetentionLeaseSyncAction(
         final Settings settings,
@@ -134,14 +132,7 @@ public class RetentionLeaseSyncAction extends TransportWriteAction<
 
                         @Override
                         public void handleException(TransportException e) {
-                            if (ExceptionsHelper.unwrap(
-                                e,
-                                IndexNotFoundException.class,
-                                AlreadyClosedException.class,
-                                IndexShardClosedException.class
-                            ) == null) {
-                                getLogger().warn(() -> format("%s retention lease sync failed", shardId), e);
-                            }
+                            LOGGER.log(getExceptionLogLevel(e), () -> format("%s retention lease sync failed", shardId), e);
                             task.setPhase("finished");
                             taskManager.unregister(task);
                             listener.onFailure(e);
@@ -152,6 +143,16 @@ public class RetentionLeaseSyncAction extends TransportWriteAction<
         }
     }
 
+    static Level getExceptionLogLevel(Exception e) {
+        return ExceptionsHelper.unwrap(
+            e,
+            IndexNotFoundException.class,
+            AlreadyClosedException.class,
+            IndexShardClosedException.class,
+            ShardNotInPrimaryModeException.class
+        ) == null ? Level.WARN : Level.DEBUG;
+    }
+
     @Override
     protected void dispatchedShardOperationOnPrimary(
         Request request,
@@ -163,7 +164,7 @@ public class RetentionLeaseSyncAction extends TransportWriteAction<
             Objects.requireNonNull(request);
             Objects.requireNonNull(primary);
             primary.persistRetentionLeases();
-            return new WritePrimaryResult<>(request, new Response(), null, primary, getLogger(), postWriteRefresh);
+            return new WritePrimaryResult<>(request, new Response(), null, primary, LOGGER, postWriteRefresh);
         });
     }
 
@@ -174,7 +175,7 @@ public class RetentionLeaseSyncAction extends TransportWriteAction<
             Objects.requireNonNull(replica);
             replica.updateRetentionLeasesOnReplica(request.getRetentionLeases());
             replica.persistRetentionLeases();
-            return new WriteReplicaResult<>(request, null, null, replica, getLogger());
+            return new WriteReplicaResult<>(request, null, null, replica, LOGGER);
         });
     }
 

+ 28 - 0
server/src/test/java/org/elasticsearch/index/seqno/RetentionLeaseSyncActionTests.java

@@ -8,6 +8,8 @@
 
 package org.elasticsearch.index.seqno;
 
+import org.apache.logging.log4j.Level;
+import org.apache.lucene.store.AlreadyClosedException;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.ActionTestUtils;
 import org.elasticsearch.action.support.PlainActionFuture;
@@ -18,10 +20,14 @@ import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.core.IOUtils;
 import org.elasticsearch.gateway.WriteStateException;
 import org.elasticsearch.index.Index;
+import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.IndexingPressure;
 import org.elasticsearch.index.shard.IndexShard;
+import org.elasticsearch.index.shard.IndexShardClosedException;
+import org.elasticsearch.index.shard.IndexShardState;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.index.shard.ShardNotInPrimaryModeException;
 import org.elasticsearch.indices.EmptySystemIndices;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.test.ESTestCase;
@@ -33,6 +39,7 @@ import org.elasticsearch.transport.TransportService;
 import java.util.Collections;
 import java.util.concurrent.atomic.AtomicBoolean;
 
+import static org.elasticsearch.index.seqno.RetentionLeaseSyncAction.getExceptionLogLevel;
 import static org.elasticsearch.test.ClusterServiceUtils.createClusterService;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.mockito.Mockito.mock;
@@ -181,4 +188,25 @@ public class RetentionLeaseSyncActionTests extends ESTestCase {
         assertNull(action.indexBlockLevel());
     }
 
+    public void testExceptionLogLevel() {
+        assertEquals(Level.WARN, getExceptionLogLevel(new RuntimeException("simulated")));
+        assertEquals(Level.WARN, getExceptionLogLevel(new RuntimeException("simulated", new RuntimeException("simulated"))));
+
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new IndexNotFoundException("index")));
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new RuntimeException("simulated", new IndexNotFoundException("index"))));
+
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new AlreadyClosedException("index")));
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new RuntimeException("simulated", new AlreadyClosedException("index"))));
+
+        final var shardId = new ShardId("test", "_na_", 0);
+
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new IndexShardClosedException(shardId)));
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new RuntimeException("simulated", new IndexShardClosedException(shardId))));
+
+        assertEquals(Level.DEBUG, getExceptionLogLevel(new ShardNotInPrimaryModeException(shardId, IndexShardState.CLOSED)));
+        assertEquals(
+            Level.DEBUG,
+            getExceptionLogLevel(new RuntimeException("simulated", new ShardNotInPrimaryModeException(shardId, IndexShardState.CLOSED)))
+        );
+    }
 }