[lucene-solr] branch branch_8x updated: Ensure we only rollback IW once (#1764)

[si...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

simonw pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new fae87d2  Ensure we only rollback IW once (#1764)
fae87d2 is described below

commit fae87d2d562df64af0d8b690af8d6386f58c63ad
Author: Simon Willnauer <si...@apache.org>
AuthorDate: Thu Aug 20 08:40:25 2020 +0200

    Ensure we only rollback IW once (#1764)
    
    Today we might rollback IW more than once if we hit an exception during
    the rollback code when we shutdown. This change moves the rollback code outside
    the try block to ensure we always roll back but never roll back twice.
---
 .../java/org/apache/lucene/index/IndexWriter.java  |  2 +-
 .../lucene/index/TestIndexWriterExceptions.java    | 42 ++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
index cb3cd22..d45bed1 100644
--- a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
@@ -1097,7 +1097,6 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
         flush(true, true);
         waitForMerges();
         commitInternal(config.getMergePolicy());
-        rollbackInternal(); // ie close, since we just committed
       } catch (Throwable t) {
         // Be certain to close the index on any exception
         try {
@@ -1107,6 +1106,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable,
         }
         throw t;
       }
+      rollbackInternal(); // if we got that far lets rollback and close
     }
   }
 
diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
index 75874ca..f995466 100644
--- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
+++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
@@ -2016,4 +2016,46 @@ public class TestIndexWriterExceptions extends LuceneTestCase {
 
     dir.close();
   }
+
+
+  public void testOnlyRollbackOnceOnException() throws IOException {
+    AtomicBoolean once = new AtomicBoolean(false);
+    InfoStream stream = new InfoStream() {
+      @Override
+      public void message(String component, String message) {
+        if ("TP".equals(component) && "rollback before checkpoint".equals(message)) {
+          if (once.compareAndSet(false, true)) {
+            throw new RuntimeException("boom");
+          } else {
+            throw new AssertionError("has been rolled back twice");
+          }
+
+        }
+      }
+
+      @Override
+      public boolean isEnabled(String component) {
+        return "TP".equals(component);
+      }
+
+      @Override
+      public void close() {
+      }
+    };
+    try (Directory dir = newDirectory()) {
+      try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig().setInfoStream(stream)){
+        @Override
+        protected boolean isEnableTestPoints() {
+          return true;
+        }
+      }) {
+        writer.rollback();
+        fail();
+      }
+    } catch (RuntimeException e) {
+      assertEquals("boom", e.getMessage());
+      assertEquals("has suppressed exceptions: " + Arrays.toString(e.getSuppressed()), 0, e.getSuppressed().length);
+      assertNull(e.getCause());
+    }
+  }
 }