LUCENE-1397: fix thread hazard whereby optimize can throw 'BG merge hit exception' IOException without setting the root cause

git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@698025 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/lucene/index/ConcurrentMergeScheduler.java

@@ -273,11 +273,6 @@ public class ConcurrentMergeScheduler extends MergeScheduler {
 
       } catch (Throwable exc) {
 
-        if (merge != null) {
-          merge.setException(exc);
-          writer.addMergeException(merge);
-        }
-
         // Ignore the exception if it was due to abort:
         if (!(exc instanceof MergePolicy.MergeAbortedException)) {
           synchronized(ConcurrentMergeScheduler.this) {

src/java/org/apache/lucene/index/IndexWriter.java

@@ -3919,6 +3919,33 @@ public class IndexWriter {
     }
   }
 
+  final private void handleMergeException(Throwable t, MergePolicy.OneMerge merge) throws IOException {
+    // Set the exception on the merge, so if
+    // optimize() is waiting on us it sees the root
+    // cause exception:
+    merge.setException(t);
+    addMergeException(merge);
+
+    if (t instanceof MergePolicy.MergeAbortedException) {
+      // We can ignore this exception (it happens when
+      // close(false) or rollback is called), unless the
+      // merge involves segments from external directories,
+      // in which case we must throw it so, for example, the
+      // rollbackTransaction code in addIndexes* is
+      // executed.
+      if (merge.isExternal)
+        throw (MergePolicy.MergeAbortedException) t;
+    } else if (t instanceof IOException)
+      throw (IOException) t;
+    else if (t instanceof RuntimeException)
+      throw (RuntimeException) t;
+    else if (t instanceof Error)
+      throw (Error) t;
+    else
+      // Should not get here
+      throw new RuntimeException(t);
+  }
+
   /**
    * Merges the indicated segments, replacing them in the stack with a
    * single segment.
@@ -3939,17 +3966,8 @@ public class IndexWriter {
 
           mergeMiddle(merge);
           success = true;
-        } catch (MergePolicy.MergeAbortedException e) {
-          merge.setException(e);
-          addMergeException(merge);
-
-          // We can ignore this exception, unless the merge
-          // involves segments from external directories, in
-          // which case we must throw it so, for example, the
-          // rollbackTransaction code in addIndexes* is
-          // executed.
-          if (merge.isExternal)
-            throw e;
+        } catch (Throwable t) {
+          handleMergeException(t, merge);
         }
       } finally {
         synchronized(this) {
@@ -3960,7 +3978,6 @@ public class IndexWriter {
             if (!success) {
               if (infoStream != null)
                 message("hit exception during merge");
-              addMergeException(merge);
               if (merge.info != null && !segmentInfos.contains(merge.info))
                 deleter.refresh(merge.info.name);
             }
@@ -4340,14 +4357,15 @@ public class IndexWriter {
             // remove the partially created CFS:
             success = true;
           } else
-            throw ioe;
+            handleMergeException(ioe, merge);
         }
+      } catch (Throwable t) {
+        handleMergeException(t, merge);
       } finally {
         if (!success) {
           if (infoStream != null)
             message("hit exception creating compound file during merge");
           synchronized(this) {
-            addMergeException(merge);
             deleter.deleteFile(compoundFileName);
           }
         }
@@ -4393,6 +4411,7 @@ public class IndexWriter {
   }
 
   synchronized void addMergeException(MergePolicy.OneMerge merge) {
+    assert merge.getException() != null;
     if (!mergeExceptions.contains(merge) && mergeGen == merge.mergeGen)
       mergeExceptions.add(merge);
   }

src/test/org/apache/lucene/index/TestIndexWriter.java

@@ -4118,4 +4118,29 @@ public class TestIndexWriter extends LuceneTestCase
       _TestUtil.rmDir(indexDir);
     }
   }
+
+  public void testOptimizeExceptions() throws IOException {
+    RAMDirectory startDir = new MockRAMDirectory();
+    IndexWriter w = new IndexWriter(startDir, false, new WhitespaceAnalyzer(), IndexWriter.MaxFieldLength.UNLIMITED);    
+    w.setMaxBufferedDocs(2);
+    w.setMergeFactor(100);
+    for(int i=0;i<27;i++)
+      addDoc(w);
+    w.close();
+
+    for(int i=0;i<200;i++) {
+      MockRAMDirectory dir = new MockRAMDirectory(startDir);
+      w = new IndexWriter(dir, false, new WhitespaceAnalyzer(), IndexWriter.MaxFieldLength.UNLIMITED);
+      ((ConcurrentMergeScheduler) w.getMergeScheduler()).setSuppressExceptions();
+      dir.setRandomIOExceptionRate(0.5, 100);
+      try {
+        w.optimize();
+      } catch (IOException ioe) {
+        if (ioe.getCause() == null)
+          fail("optimize threw IOException without root cause");
+      }
+      w.close();
+      dir.close();
+    }
+  }
 }