Simplify TranslogWriter#closeWithTragicEvent (#29412)

This commit simplifies the exception handling in
TranslogWriter#closeWithTragicEvent. When invoking this method, the
inner close method could throw an exception which we always catch and
suppress into the exception that led us to tragically close. This commit
moves that repeated logic into closeWithTragicException and now callers
simply need to catch, invoke closeWithTragicException, and rethrow.

Author: jasontedor

server/src/main/java/org/elasticsearch/index/translog/TranslogWriter.java

@@ -164,16 +164,20 @@ public Exception getTragicException() {
         return tragedy;
     }
 
-    private synchronized void closeWithTragicEvent(Exception exception) throws IOException {
-        assert exception != null;
+    private synchronized void closeWithTragicEvent(final Exception ex) {
+        assert ex != null;
         if (tragedy == null) {
-            tragedy = exception;
-        } else if (tragedy != exception) {
+            tragedy = ex;
+        } else if (tragedy != ex) {
             // it should be safe to call closeWithTragicEvents on multiple layers without
             // worrying about self suppression.
-            tragedy.addSuppressed(exception);
+            tragedy.addSuppressed(ex);
+        }
+        try {
+            close();
+        } catch (final IOException | RuntimeException e) {
+            ex.addSuppressed(e);
         }
-        close();
     }
 
     /**
@@ -194,11 +198,7 @@ public synchronized Translog.Location add(final BytesReference data, final long
         try {
             data.writeTo(outputStream);
         } catch (final Exception ex) {
-            try {
-                closeWithTragicEvent(ex);
-            } catch (final Exception inner) {
-                ex.addSuppressed(inner);
-            }
+            closeWithTragicEvent(ex);
             throw ex;
         }
         totalOffset += data.length();
@@ -290,13 +290,9 @@ public TranslogReader closeIntoReader() throws IOException {
             synchronized (this) {
                 try {
                     sync(); // sync before we close..
-                } catch (IOException e) {
-                    try {
-                        closeWithTragicEvent(e);
-                    } catch (Exception inner) {
-                        e.addSuppressed(inner);
-                    }
-                    throw e;
+                } catch (final Exception ex) {
+                    closeWithTragicEvent(ex);
+                    throw ex;
                 }
                 if (closed.compareAndSet(false, true)) {
                     return new TranslogReader(getLastSyncedCheckpoint(), channel, path, getFirstOperationOffset());
@@ -346,12 +342,8 @@ public boolean syncUpTo(long offset) throws IOException {
                         try {
                             outputStream.flush();
                             checkpointToSync = getCheckpoint();
-                        } catch (Exception ex) {
-                            try {
-                                closeWithTragicEvent(ex);
-                            } catch (Exception inner) {
-                                ex.addSuppressed(inner);
-                            }
+                        } catch (final Exception ex) {
+                            closeWithTragicEvent(ex);
                             throw ex;
                         }
                     }
@@ -360,12 +352,8 @@ public boolean syncUpTo(long offset) throws IOException {
                     try {
                         channel.force(false);
                         writeCheckpoint(channelFactory, path.getParent(), checkpointToSync);
-                    } catch (Exception ex) {
-                        try {
-                            closeWithTragicEvent(ex);
-                        } catch (Exception inner) {
-                            ex.addSuppressed(inner);
-                        }
+                    } catch (final Exception ex) {
+                        closeWithTragicEvent(ex);
                         throw ex;
                     }
                     assert lastSyncedCheckpoint.offset <= checkpointToSync.offset :
@@ -392,13 +380,9 @@ protected void readBytes(ByteBuffer targetBuffer, long position) throws IOExcept
                     }
                 }
             }
-        } catch (final IOException e) {
-            try {
-                closeWithTragicEvent(e);
-            } catch (final IOException inner) {
-                e.addSuppressed(inner);
-            }
-            throw e;
+        } catch (final Exception ex) {
+            closeWithTragicEvent(ex);
+            throw ex;
         }
         // we don't have to have a lock here because we only write ahead to the file, so all writes has been complete
         // for the requested location.
@@ -451,12 +435,8 @@ public synchronized void flush() throws IOException {
                 try {
                     ensureOpen();
                     super.flush();
-                } catch (Exception ex) {
-                    try {
-                        closeWithTragicEvent(ex);
-                    } catch (Exception inner) {
-                        ex.addSuppressed(inner);
-                    }
+                } catch (final Exception ex) {
+                    closeWithTragicEvent(ex);
                     throw ex;
                 }
             }