From ad457d188e50005f0eb0b2f9257e341da590a9bf Mon Sep 17 00:00:00 2001
From: Simon Willnauer <simonw@apache.org>
Date: Fri, 15 Mar 2019 10:59:52 +0100
Subject: [PATCH] Improve RIW exception handling and opt out of concurrent
 flushing if exception is expected

---
 .../TestIDVersionPostingsFormat.java          |  2 +-
 .../document/TestContextSuggestField.java     |  4 +-
 .../lucene/index/RandomIndexWriter.java       | 52 +++++++++++++------
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/lucene/sandbox/src/test/org/apache/lucene/codecs/idversion/TestIDVersionPostingsFormat.java b/lucene/sandbox/src/test/org/apache/lucene/codecs/idversion/TestIDVersionPostingsFormat.java
index 0318109a133..d51bf72329d 100644
--- a/lucene/sandbox/src/test/org/apache/lucene/codecs/idversion/TestIDVersionPostingsFormat.java
+++ b/lucene/sandbox/src/test/org/apache/lucene/codecs/idversion/TestIDVersionPostingsFormat.java
@@ -481,7 +481,7 @@ public class TestIDVersionPostingsFormat extends LuceneTestCase {
     doc.add(newStringField("id", "id", Field.Store.NO));
     expectThrows(IllegalArgumentException.class, () -> {
       w.addDocument(doc);
-      w.commit();
+      w.commit(false);
     });
 
              
diff --git a/lucene/suggest/src/test/org/apache/lucene/search/suggest/document/TestContextSuggestField.java b/lucene/suggest/src/test/org/apache/lucene/search/suggest/document/TestContextSuggestField.java
index 8beea129622..1814b5dfb4e 100644
--- a/lucene/suggest/src/test/org/apache/lucene/search/suggest/document/TestContextSuggestField.java
+++ b/lucene/suggest/src/test/org/apache/lucene/search/suggest/document/TestContextSuggestField.java
@@ -87,7 +87,7 @@ public class TestContextSuggestField extends LuceneTestCase {
       IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, () -> {
         document.add(new ContextSuggestField("name", charsRefBuilder.toString(), 1, "sugg"));
         iw.addDocument(document);
-        iw.commit();
+        iw.commit(false);
       });
       assertTrue(expected.getMessage().contains("[0x1d]"));
     }
@@ -139,7 +139,7 @@ public class TestContextSuggestField extends LuceneTestCase {
       // mixing suggest field types for same field name should error out
       IllegalArgumentException expected = expectThrows(IllegalArgumentException.class, () -> {
         iw.addDocument(document);
-        iw.commit();
+        iw.commit(false);
       });
       assertTrue(expected.getMessage().contains("mixed types"));
     }
diff --git a/lucene/test-framework/src/java/org/apache/lucene/index/RandomIndexWriter.java b/lucene/test-framework/src/java/org/apache/lucene/index/RandomIndexWriter.java
index de6defb924f..00350339cca 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/index/RandomIndexWriter.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/index/RandomIndexWriter.java
@@ -20,8 +20,9 @@ import java.io.Closeable;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Iterator;
+import java.util.List;
 import java.util.Random;
-import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.MockAnalyzer;
@@ -314,29 +315,50 @@ public class RandomIndexWriter implements Closeable {
     LuceneTestCase.maybeChangeLiveIndexWriterConfig(r, w.getConfig());
     return w.deleteDocuments(q);
   }
-  
+
   public long commit() throws IOException {
+    return commit(r.nextInt(10) == 0);
+  }
+  
+  public long commit(boolean flushConcurrently) throws IOException {
     LuceneTestCase.maybeChangeLiveIndexWriterConfig(r, w.getConfig());
-    if (r.nextInt(10) == 0) {
-      AtomicReference<Throwable> exception = new AtomicReference<>();
-      Thread t = new Thread(() -> {
+    if (flushConcurrently) {
+      List<Throwable> throwableList = new CopyOnWriteArrayList<>();
+      Thread thread = new Thread(() -> {
         try {
           flushAllBuffersSequentially();
-        } catch (IOException e) {
-          exception.set(e);
+        } catch (Throwable e) {
+          throwableList.add(e);
         }
       });
-      t.start();
-      long seqId = w.commit();
+      thread.start();
       try {
-        t.join();
-      } catch (InterruptedException e) {
-        throw new AssertionError(e);
+        return w.commit();
+      } catch (Throwable t) {
+        throwableList.add(t);
+      } finally {
+        try {
+          // make sure we wait for the thread to join otherwise it might still be processing events
+          // and the IW won't be fully closed in the case of a fatal exception
+          thread.join();
+        } catch (InterruptedException e) {
+          throwableList.add(e);
+        }
       }
-      if (exception.get() != null) {
-        throw new AssertionError(exception.get());
+      if (throwableList.size() != 0) {
+        Throwable primary = throwableList.get(0);
+        for (int i = 1; i < throwableList.size(); i++) {
+          primary.addSuppressed(throwableList.get(i));
+        }
+        if (primary instanceof IOException) {
+          throw (IOException)primary;
+        } else if (primary instanceof RuntimeException) {
+          throw (RuntimeException)primary;
+        } else {
+          throw new AssertionError(primary);
+        }
       }
-      return seqId;
+
     }
     return w.commit();
   }