Fix shard failure due to translog tragic close on upload failure (ope…

…nsearch-project#10363) Signed-off-by: Ashish Singh <[email protected]>
andrross · Oct 4, 2023 · a0cb344 · a0cb344
1 parent e5024a8
commit a0cb344
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 18 deletions.
diff --git a/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java b/server/src/main/java/org/opensearch/index/translog/RemoteFsTranslog.java
@@ -242,15 +242,10 @@ public static TranslogTransferManager buildTranslogTransferManager(
 
     @Override
     public boolean ensureSynced(Location location) throws IOException {
-        try {
-            assert location.generation <= current.getGeneration();
-            if (location.generation == current.getGeneration()) {
-                ensureOpen();
-                return prepareAndUpload(primaryTermSupplier.getAsLong(), location.generation);
-            }
-        } catch (final Exception ex) {
-            closeOnTragicEvent(ex);
-            throw ex;
+        assert location.generation <= current.getGeneration();
+        if (location.generation == current.getGeneration()) {
+            ensureOpen();
+            return prepareAndUpload(primaryTermSupplier.getAsLong(), location.generation);
         }
         return false;
     }
@@ -355,14 +350,8 @@ private boolean syncToDisk() throws IOException {
 
     @Override
     public void sync() throws IOException {
-        try {
-            if (syncToDisk() || syncNeeded()) {
-                prepareAndUpload(primaryTermSupplier.getAsLong(), null);
-            }
-        } catch (final Exception e) {
-            tragedy.setTragicException(e);
-            closeOnTragicEvent(e);
-            throw e;
+        if (syncToDisk() || syncNeeded()) {
+            prepareAndUpload(primaryTermSupplier.getAsLong(), null);
         }
     }
 

diff --git a/server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java b/server/src/test/java/org/opensearch/index/translog/RemoteFsTranslogTests.java
@@ -67,6 +67,7 @@
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
@@ -1049,7 +1050,7 @@ public void testSyncUpTo() throws IOException {
         }
     }
 
-    public void testSyncUpFailure() throws IOException {
+    public void testSyncUpLocationFailure() throws IOException {
         int translogOperations = randomIntBetween(1, 20);
         int count = 0;
         fail.failAlways();
@@ -1101,6 +1102,26 @@ public void testSyncUpFailure() throws IOException {
         assertDownloadStatsNoDownloads(statsTracker);
     }
 
+    public void testSyncUpAlwaysFailure() throws IOException {
+        int translogOperations = randomIntBetween(1, 20);
+        int count = 0;
+        fail.failAlways();
+        for (int op = 0; op < translogOperations; op++) {
+            translog.add(
+                new Translog.Index(String.valueOf(op), count, primaryTerm.get(), Integer.toString(count).getBytes(StandardCharsets.UTF_8))
+            );
+            try {
+                translog.sync();
+                fail("io exception expected");
+            } catch (IOException e) {
+                assertTrue("at least one operation pending", translog.syncNeeded());
+            }
+        }
+        assertTrue(translog.isOpen());
+        fail.failNever();
+        translog.sync();
+    }
+
     public void testSyncUpToStream() throws IOException {
         int iters = randomIntBetween(5, 10);
         for (int i = 0; i < iters; i++) {