Incorporate PR review feedback

Signed-off-by: Ashish Singh <[email protected]>
opensearch-project · Oct 11, 2023 · 408ffa1 · 408ffa1
1 parent 64b5610
commit 408ffa1
Show file tree

Hide file tree

Showing 10 changed files with 20 additions and 41 deletions.
diff --git a/...alClusterTest/java/org/opensearch/remotestore/RemoteStoreBackpressureAndResiliencyIT.java b/...alClusterTest/java/org/opensearch/remotestore/RemoteStoreBackpressureAndResiliencyIT.java
@@ -15,6 +15,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.util.concurrent.AbstractAsyncTask;
+import org.opensearch.common.util.concurrent.UncategorizedExecutionException;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.core.common.unit.ByteSizeUnit;
@@ -23,7 +24,6 @@
 import org.opensearch.index.IndexService;
 import org.opensearch.index.remote.RemoteSegmentTransferTracker;
 import org.opensearch.index.shard.IndexShard;
-import org.opensearch.index.translog.transfer.TranslogUploadFailedException;
 import org.opensearch.indices.IndicesService;
 import org.opensearch.repositories.RepositoriesService;
 import org.opensearch.snapshots.mockstore.MockRepository;
@@ -185,8 +185,8 @@ public void testAsyncTrimTaskSucceeds() {
         translogRepo.setRandomControlIOExceptionRate(1d);
 
         for (int i = 0; i < randomIntBetween(5, 10); i++) {
-            TranslogUploadFailedException exception = assertThrows(TranslogUploadFailedException.class, this::indexSingleDoc);
-            assertTrue(exception.getMessage().contains("Failed to upload") && exception.getMessage().contains("files during transfer"));
+            UncategorizedExecutionException exception = assertThrows(UncategorizedExecutionException.class, this::indexSingleDoc);
+            assertEquals("Failed execution", exception.getMessage());
         }
 
         translogRepo.setRandomControlIOExceptionRate(0d);
@@ -244,8 +244,8 @@ public void testFlushDuringRemoteUploadFailures() {
         logger.info("--> Failing all remote store interaction");
         translogRepo.setRandomControlIOExceptionRate(1d);
 
-        Exception ex = assertThrows(TranslogUploadFailedException.class, () -> indexSingleDoc());
-        assertEquals("Failed to upload 2 files during transfer", ex.getMessage());
+        Exception ex = assertThrows(UncategorizedExecutionException.class, () -> indexSingleDoc());
+        assertEquals("Failed execution", ex.getMessage());
 
         FlushResponse flushResponse = client().admin().indices().prepareFlush(INDEX_NAME).setForce(true).execute().actionGet();
         assertEquals(1, flushResponse.getFailedShards());

diff --git a/server/src/main/java/org/opensearch/OpenSearchServerException.java b/server/src/main/java/org/opensearch/OpenSearchServerException.java
@@ -15,7 +15,6 @@
 import static org.opensearch.OpenSearchException.OpenSearchExceptionHandleRegistry.registerExceptionHandle;
 import static org.opensearch.OpenSearchException.UNKNOWN_VERSION_ADDED;
 import static org.opensearch.Version.V_2_10_0;
-import static org.opensearch.Version.V_2_11_0;
 import static org.opensearch.Version.V_2_1_0;
 import static org.opensearch.Version.V_2_4_0;
 import static org.opensearch.Version.V_2_5_0;
@@ -1176,14 +1175,6 @@ public static void registerExceptions() {
             )
         );
         registerExceptionHandle(new OpenSearchExceptionHandle(CryptoRegistryException.class, CryptoRegistryException::new, 171, V_2_10_0));
-        registerExceptionHandle(
-            new OpenSearchExceptionHandle(
-                org.opensearch.index.translog.transfer.TranslogUploadFailedException.class,
-                org.opensearch.index.translog.transfer.TranslogUploadFailedException::new,
-                172,
-                V_2_11_0
-            )
-        );
         registerExceptionHandle(
             new OpenSearchExceptionHandle(
                 org.opensearch.cluster.block.IndexCreateBlockException.class,

diff --git a/server/src/main/java/org/opensearch/index/engine/InternalEngine.java b/server/src/main/java/org/opensearch/index/engine/InternalEngine.java
@@ -112,7 +112,6 @@
 import org.opensearch.index.translog.TranslogManager;
 import org.opensearch.index.translog.listener.CompositeTranslogEventListener;
 import org.opensearch.index.translog.listener.TranslogEventListener;
-import org.opensearch.index.translog.transfer.TranslogUploadFailedException;
 import org.opensearch.search.suggest.completion.CompletionStats;
 import org.opensearch.threadpool.ThreadPool;
 
@@ -1889,9 +1888,6 @@ public void flush(boolean force, boolean waitIfOngoing) throws EngineException {
                         }
 
                         translogManager.trimUnreferencedReaders();
-                    } catch (TranslogUploadFailedException e) {
-                        // Do not fail engine as this is due to translog upload failure
-                        throw new FlushFailedEngineException(shardId, e);
                     } catch (AlreadyClosedException e) {
                         failOnTragicEvent(e);
                         throw e;

diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java
@@ -1476,7 +1476,7 @@ public void trimTranslog() {
     /**
      * Rolls the tranlog generation and cleans unneeded.
      */
-    public void rollTranslogGeneration() {
+    public void rollTranslogGeneration() throws IOException {
         final Engine engine = getEngine();
         engine.translogManager().rollTranslogGeneration();
     }

diff --git a/server/src/main/java/org/opensearch/index/translog/InternalTranslogManager.java b/server/src/main/java/org/opensearch/index/translog/InternalTranslogManager.java
@@ -84,7 +84,7 @@ public InternalTranslogManager(
      * Rolls the translog generation and cleans unneeded.
      */
     @Override
-    public void rollTranslogGeneration() throws TranslogException {
+    public void rollTranslogGeneration() throws TranslogException, IOException {
         try (ReleasableLock ignored = readLock.acquire()) {
             engineLifeCycleAware.ensureOpen();
             translog.rollGeneration();

diff --git a/server/src/main/java/org/opensearch/index/translog/TranslogManager.java b/server/src/main/java/org/opensearch/index/translog/TranslogManager.java
@@ -21,7 +21,7 @@ public interface TranslogManager {
     /**
      * Rolls the translog generation and cleans unneeded.
      */
-    void rollTranslogGeneration() throws TranslogException;
+    void rollTranslogGeneration() throws TranslogException, IOException;
 
     /**
      * Performs recovery from the transaction log up to {@code recoverUpToSeqNo} (inclusive).

diff --git a/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java b/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java
@@ -176,7 +176,7 @@ public boolean transferSnapshot(TransferSnapshot transferSnapshot, TranslogTrans
                     remoteTranslogTransferTracker.addUploadTimeInMillis((System.nanoTime() - metadataUploadStartTime) / 1_000_000L);
                     remoteTranslogTransferTracker.addUploadBytesFailed(metadataBytesToUpload);
                     // outer catch handles capturing stats on upload failure
-                    throw new TranslogUploadFailedException(shardId, "Failed to upload " + tlogMetadata.getName(), exception);
+                    throw new TranslogUploadFailedException("Failed to upload " + tlogMetadata.getName(), exception);
                 }
 
                 remoteTranslogTransferTracker.addUploadTimeInMillis((System.nanoTime() - metadataUploadStartTime) / 1_000_000L);
@@ -185,10 +185,7 @@ public boolean transferSnapshot(TransferSnapshot transferSnapshot, TranslogTrans
                 translogTransferListener.onUploadComplete(transferSnapshot);
                 return true;
             } else {
-                Exception ex = new TranslogUploadFailedException(
-                    shardId,
-                    "Failed to upload " + exceptionList.size() + " files during transfer"
-                );
+                Exception ex = new TranslogUploadFailedException("Failed to upload " + exceptionList.size() + " files during transfer");
                 exceptionList.forEach(ex::addSuppressed);
                 throw ex;
             }

diff --git a/...r/src/main/java/org/opensearch/index/translog/transfer/TranslogUploadFailedException.java b/...r/src/main/java/org/opensearch/index/translog/transfer/TranslogUploadFailedException.java
@@ -8,27 +8,20 @@
 
 package org.opensearch.index.translog.transfer;
 
-import org.opensearch.core.common.io.stream.StreamInput;
-import org.opensearch.core.index.shard.ShardId;
-import org.opensearch.index.translog.TranslogException;
-
 import java.io.IOException;
 
 /**
  * Exception is thrown if there are any exceptions while uploading translog to remote store.
  * @opensearch.internal
  */
-public class TranslogUploadFailedException extends TranslogException {
+public class TranslogUploadFailedException extends IOException {
 
-    public TranslogUploadFailedException(ShardId shardId, String message) {
-        super(shardId, message);
+    public TranslogUploadFailedException(String message) {
+        super(message);
     }
 
-    public TranslogUploadFailedException(ShardId shardId, String message, Throwable cause) {
-        super(shardId, message, cause);
+    public TranslogUploadFailedException(String message, Throwable cause) {
+        super(message, cause);
     }
 
-    public TranslogUploadFailedException(StreamInput in) throws IOException {
-        super(in);
-    }
 }
diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java
@@ -96,7 +96,6 @@
 import org.opensearch.index.shard.IndexShardState;
 import org.opensearch.index.shard.PrimaryShardClosedException;
 import org.opensearch.index.shard.ShardNotInPrimaryModeException;
-import org.opensearch.index.translog.transfer.TranslogUploadFailedException;
 import org.opensearch.indices.IndexTemplateMissingException;
 import org.opensearch.indices.InvalidIndexTemplateException;
 import org.opensearch.indices.recovery.PeerRecoveryNotFound;
@@ -893,7 +892,6 @@ public void testIds() {
         ids.put(169, NodeWeighedAwayException.class);
         ids.put(170, SearchPipelineProcessingException.class);
         ids.put(171, CryptoRegistryException.class);
-        ids.put(172, TranslogUploadFailedException.class);
         ids.put(10001, IndexCreateBlockException.class);
 
         Map<Class<? extends OpenSearchException>, Integer> reverse = new HashMap<>();

diff --git a/server/src/test/java/org/opensearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/opensearch/index/engine/InternalEngineTests.java
@@ -7256,7 +7256,11 @@ public void testMaxSeqNoInCommitUserData() throws Exception {
             engine.ensureOpen();
             while (running.get()
                 && assertAndGetInternalTranslogManager(engine.translogManager()).getTranslog().currentFileGeneration() < 500) {
-                engine.translogManager().rollTranslogGeneration(); // make adding operations to translog slower
+                try {
+                    engine.translogManager().rollTranslogGeneration(); // make adding operations to translog slower
+                } catch (IOException e) {
+                    fail("io exception not expected");
+                }
             }
         });
         rollTranslog.start();