From bca192a327d580de1c8bcd99d15685ccc5f0439f Mon Sep 17 00:00:00 2001
From: Jason Tedor <jason@tedor.me>
Date: Tue, 10 Apr 2018 10:15:54 -0400
Subject: [PATCH] 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.
---
 .../index/translog/TranslogWriter.java        | 66 +++++++------------
 1 file changed, 23 insertions(+), 43 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/translog/TranslogWriter.java b/server/src/main/java/org/elasticsearch/index/translog/TranslogWriter.java
index 4846fdb4e46..3bf70c06531 100644
--- a/server/src/main/java/org/elasticsearch/index/translog/TranslogWriter.java
+++ b/server/src/main/java/org/elasticsearch/index/translog/TranslogWriter.java
@@ -164,16 +164,20 @@ public class TranslogWriter extends BaseTranslogReader implements Closeable {
         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 class TranslogWriter extends BaseTranslogReader implements Closeable {
         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 class TranslogWriter extends BaseTranslogReader implements Closeable {
             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 class TranslogWriter extends BaseTranslogReader implements Closeable {
                         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 class TranslogWriter extends BaseTranslogReader implements Closeable {
                     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 @@ public class TranslogWriter extends BaseTranslogReader implements Closeable {
                     }
                 }
             }
-        } 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 class TranslogWriter extends BaseTranslogReader implements Closeable {
                 try {
                     ensureOpen();
                     super.flush();
-                } catch (Exception ex) {
-                    try {
-                        closeWithTragicEvent(ex);
-                    } catch (Exception inner) {
-                        ex.addSuppressed(inner);
-                    }
+                } catch (final Exception ex) {
+                    closeWithTragicEvent(ex);
                     throw ex;
                 }
             }