lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rm...@apache.org
Subject svn commit: r1625853 - in /lucene/dev/trunk/lucene: ./ core/src/java/org/apache/lucene/index/ core/src/java/org/apache/lucene/store/ core/src/java/org/apache/lucene/util/ core/src/test/org/apache/lucene/index/
Date Wed, 17 Sep 2014 23:39:43 GMT
Author: rmuir
Date: Wed Sep 17 23:39:42 2014
New Revision: 1625853

URL: http://svn.apache.org/r1625853
Log:
LUCENE-5958: OOM or exceptions during checkpoint make IndexWriter have a bad day

Modified:
    lucene/dev/trunk/lucene/CHANGES.txt
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/AlreadyClosedException.java
    lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/IOUtils.java
    lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java
    lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOutOfMemory.java

Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Wed Sep 17 23:39:42 2014
@@ -169,6 +169,11 @@ Bug Fixes
 * LUCENE-5934: Fix backwards compatibility for 4.0 indexes.
   (Ian Lea, Uwe Schindler, Robert Muir, Ryan Ernst)
 
+* LUCENE-5958: Don't let exceptions during checkpoint corrupt the index. 
+  Refactor existing OOM handling too, so you don't need to handle OOM special
+  for every IndexWriter method: instead such disasters will cause IW to close itself
+  defensively. (Robert Muir, Mike McCandless)
+
 Tests
 
 * LUCENE-5936: Add backcompat checks to verify what is tested matches known versions

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java Wed
Sep 17 23:39:42 2014
@@ -27,6 +27,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.regex.Matcher;
 
 import org.apache.lucene.store.AlreadyClosedException;
@@ -109,7 +110,6 @@ final class IndexFileDeleter implements 
    *  infoStream is enabled */
   public static boolean VERBOSE_REF_COUNTS = false;
 
-  // Used only for assert
   private final IndexWriter writer;
 
   // called only from assert
@@ -126,6 +126,7 @@ final class IndexFileDeleter implements 
    */
   public IndexFileDeleter(Directory directory, IndexDeletionPolicy policy, SegmentInfos segmentInfos,
                           InfoStream infoStream, IndexWriter writer, boolean initialIndexExists)
throws IOException {
+    Objects.requireNonNull(writer);
     this.infoStream = infoStream;
     this.writer = writer;
 
@@ -343,10 +344,10 @@ final class IndexFileDeleter implements 
   }
 
   private void ensureOpen() throws AlreadyClosedException {
-    if (writer == null) {
-      throw new AlreadyClosedException("this IndexWriter is closed");
-    } else {
-      writer.ensureOpen(false);
+    writer.ensureOpen(false);
+    // since we allow 'closing' state, we must still check this, we could be closing because
we hit e.g. OOM
+    if (writer.tragedy != null) {
+      throw new AlreadyClosedException("refusing to delete any files: this IndexWriter hit
an unrecoverable exception", writer.tragedy);
     }
   }
 

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java Wed Sep
17 23:39:42 2014
@@ -147,16 +147,11 @@ import org.apache.lucene.util.Version;
   {@link ConcurrentMergeScheduler}. </p>
 
   <a name="OOME"></a><p><b>NOTE</b>: if you hit an
-  OutOfMemoryError then IndexWriter will quietly record this
-  fact and block all future segment commits.  This is a
+  OutOfMemoryError, or disaster strikes during a checkpoint
+  then IndexWriter will close itself.  This is a
   defensive measure in case any internal state (buffered
-  documents and deletions) were corrupted.  Any subsequent
-  calls to {@link #commit()} will throw an
-  IllegalStateException.  The only course of action is to
-  call {@link #close()}, which internally will call {@link
-  #rollback()}, to undo any changes to the index since the
-  last commit.  You can also just call {@link #rollback()}
-  directly.</p>
+  documents, deletions, reference counts) were corrupted.  
+  Any subsequent calls will throw an AlreadyClosedException.</p>
 
   <a name="thread-safety"></a><p><b>NOTE</b>: {@link
   IndexWriter} instances are completely thread
@@ -246,7 +241,8 @@ public class IndexWriter implements Clos
    * IndexWriterConfig#setInfoStream(InfoStream)}).
    */
   public final static int MAX_TERM_LENGTH = DocumentsWriterPerThread.MAX_TERM_LENGTH_UTF8;
-  volatile private boolean hitOOM;
+  // when unrecoverable disaster strikes, we populate this with the reason that we had to
close IndexWriter
+  volatile Throwable tragedy;
 
   private final Directory directory;  // where this index resides
   private final Analyzer analyzer;    // how to analyze text
@@ -428,7 +424,7 @@ public class IndexWriter implements Clos
             }
           }
         } catch (OutOfMemoryError oom) {
-          handleOOM(oom, "getReader");
+          tragicEvent(oom, "getReader");
           // never reached but javac disagrees:
           return null;
         } finally {
@@ -437,10 +433,12 @@ public class IndexWriter implements Clos
               infoStream.message("IW", "hit exception during NRT reader");
             }
           }
-          // Done: finish the full flush!
-          docWriter.finishFullFlush(success);
-          processEvents(false, true);
-          doAfterFlush();
+          if (tragedy == null) {
+            // Done: finish the full flush! (unless we hit OOM or something)
+            docWriter.finishFullFlush(success);
+            processEvents(false, true);
+            doAfterFlush();
+          }
         }
       }
       if (anySegmentFlushed) {
@@ -702,7 +700,7 @@ public class IndexWriter implements Clos
    */
   protected final void ensureOpen(boolean failIfClosing) throws AlreadyClosedException {
     if (closed || (failIfClosing && closing)) {
-      throw new AlreadyClosedException("this IndexWriter is closed");
+      throw new AlreadyClosedException("this IndexWriter is closed", tragedy);
     }
   }
 
@@ -1081,10 +1079,6 @@ public class IndexWriter implements Clos
    * replaced with the Unicode replacement character
    * U+FFFD.</p>
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @throws CorruptIndexException if the index is corrupt
    * @throws IOException if there is a low-level IO error
    */
@@ -1100,10 +1094,6 @@ public class IndexWriter implements Clos
    * index and IndexWriter state after an Exception, and
    * flushing/merging temporary free space requirements.</p>
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @throws CorruptIndexException if the index is corrupt
    * @throws IOException if there is a low-level IO error
    */
@@ -1143,10 +1133,6 @@ public class IndexWriter implements Clos
    * and will likely break them up.  Use such tools at your
    * own risk!
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @throws CorruptIndexException if the index is corrupt
    * @throws IOException if there is a low-level IO error
    *
@@ -1219,7 +1205,7 @@ public class IndexWriter implements Clos
         }
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "updateDocuments");
+      tragicEvent(oom, "updateDocuments");
     }
   }
 
@@ -1303,10 +1289,6 @@ public class IndexWriter implements Clos
    * terms. All given deletes are applied and flushed atomically
    * at the same time.
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @param terms array of terms to identify the documents
    * to be deleted
    * @throws CorruptIndexException if the index is corrupt
@@ -1319,7 +1301,7 @@ public class IndexWriter implements Clos
         processEvents(true, false);
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "deleteDocuments(Term..)");
+      tragicEvent(oom, "deleteDocuments(Term..)");
     }
   }
 
@@ -1327,10 +1309,6 @@ public class IndexWriter implements Clos
    * Deletes the document(s) matching any of the provided queries.
    * All given deletes are applied and flushed atomically at the same time.
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @param queries array of queries to identify the documents
    * to be deleted
    * @throws CorruptIndexException if the index is corrupt
@@ -1343,7 +1321,7 @@ public class IndexWriter implements Clos
         processEvents(true, false);
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "deleteDocuments(Query..)");
+      tragicEvent(oom, "deleteDocuments(Query..)");
     }
   }
 
@@ -1354,10 +1332,6 @@ public class IndexWriter implements Clos
    * by a reader on the same index (flush may happen only after
    * the add).
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @param term the term to identify the document(s) to be
    * deleted
    * @param doc the document to be added
@@ -1376,10 +1350,6 @@ public class IndexWriter implements Clos
    * by a reader on the same index (flush may happen only after
    * the add).
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @param term the term to identify the document(s) to be
    * deleted
    * @param doc the document to be added
@@ -1405,7 +1375,7 @@ public class IndexWriter implements Clos
         }
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "updateDocument");
+      tragicEvent(oom, "updateDocument");
     }
   }
 
@@ -1414,11 +1384,6 @@ public class IndexWriter implements Clos
    * given <code>value</code>. You can only update fields that already exist
in
    * the index, not add new fields through this method.
    * 
-   * <p>
-   * <b>NOTE</b>: if this method hits an OutOfMemoryError you should immediately
-   * close the writer. See <a href="#OOME">above</a> for details.
-   * </p>
-   * 
    * @param term
    *          the term to identify the document(s) to be updated
    * @param field
@@ -1440,7 +1405,7 @@ public class IndexWriter implements Clos
         processEvents(true, false);
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "updateNumericDocValue");
+      tragicEvent(oom, "updateNumericDocValue");
     }
   }
 
@@ -1453,11 +1418,6 @@ public class IndexWriter implements Clos
    * <b>NOTE:</b> this method currently replaces the existing value of all
    * affected documents with the new value.
    * 
-   * <p>
-   * <b>NOTE:</b> if this method hits an OutOfMemoryError you should immediately
-   * close the writer. See <a href="#OOME">above</a> for details.
-   * </p>
-   * 
    * @param term
    *          the term to identify the document(s) to be updated
    * @param field
@@ -1482,7 +1442,7 @@ public class IndexWriter implements Clos
         processEvents(true, false);
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "updateBinaryDocValue");
+      tragicEvent(oom, "updateBinaryDocValue");
     }
   }
   
@@ -1492,11 +1452,6 @@ public class IndexWriter implements Clos
    * {@link Term} to the same value. All updates are atomically applied and
    * flushed together.
    * 
-   * <p>
-   * <b>NOTE</b>: if this method hits an OutOfMemoryError you should immediately
-   * close the writer. See <a href="#OOME">above</a> for details.
-   * </p>
-   * 
    * @param updates
    *          the updates to apply
    * @throws CorruptIndexException
@@ -1532,7 +1487,7 @@ public class IndexWriter implements Clos
         processEvents(true, false);
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "updateDocValues");
+      tragicEvent(oom, "updateDocValues");
     }
   }
   
@@ -1637,10 +1592,6 @@ public class IndexWriter implements Clos
    * newly created segments will not be merged unless you
    * call forceMerge again.</p>
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * <p><b>NOTE</b>: if you call {@link #abortMerges}, which
    * aborts all running merges, then any thread still
    * running this method might hit a {@link
@@ -1663,10 +1614,6 @@ public class IndexWriter implements Clos
    *  all merging completes.  This is only meaningful with a
    *  {@link MergeScheduler} that is able to run merges in
    *  background threads.
-   *
-   *  <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   *  you should immediately close the writer.  See <a
-   *  href="#OOME">above</a> for details.</p>
    */
   public void forceMerge(int maxNumSegments, boolean doWait) throws IOException {
     ensureOpen();
@@ -1708,8 +1655,8 @@ public class IndexWriter implements Clos
       synchronized(this) {
         while(true) {
 
-          if (hitOOM) {
-            throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot
complete forceMerge");
+          if (tragedy != null) {
+            throw new IllegalStateException("this writer hit an unrecoverable error; cannot
complete forceMerge", tragedy);
           }
 
           if (mergeExceptions.size() > 0) {
@@ -1764,10 +1711,6 @@ public class IndexWriter implements Clos
    *  {@link MergeScheduler} that is able to run merges in
    *  background threads.
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * <p><b>NOTE</b>: if you call {@link #abortMerges}, which
    * aborts all running merges, then any thread still
    * running this method might hit a {@link
@@ -1803,8 +1746,8 @@ public class IndexWriter implements Clos
         boolean running = true;
         while(running) {
 
-          if (hitOOM) {
-            throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot
complete forceMergeDeletes");
+          if (tragedy != null) {
+            throw new IllegalStateException("this writer hit an unrecoverable error; cannot
complete forceMergeDeletes", tragedy);
           }
 
           // Check each merge that MergePolicy asked us to
@@ -1853,10 +1796,6 @@ public class IndexWriter implements Clos
    *  <p><b>NOTE</b>: this method first flushes a new
    *  segment (if there are indexed documents), and applies
    *  all buffered deletes.
-   *
-   *  <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   *  you should immediately close the writer.  See <a
-   *  href="#OOME">above</a> for details.</p>
    */
   public void forceMergeDeletes() throws IOException {
     forceMergeDeletes(true);
@@ -1874,10 +1813,6 @@ public class IndexWriter implements Clos
    * 
    * This method will call the {@link MergePolicy} with
    * {@link MergeTrigger#EXPLICIT}.
-   *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
    */
   public final void maybeMerge() throws IOException {
     maybeMerge(config.getMergePolicy(), MergeTrigger.EXPLICIT, UNBOUNDED_MAX_MERGE_SEGMENTS);
@@ -1902,8 +1837,8 @@ public class IndexWriter implements Clos
       return false;
     }
 
-    // Do not start new merges if we've hit OOME
-    if (hitOOM) {
+    // Do not start new merges if disaster struck
+    if (tragedy != null) {
       return false;
     }
     boolean newMergesFound = false;
@@ -2064,7 +1999,7 @@ public class IndexWriter implements Clos
 
       success = true;
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "rollbackInternal");
+      tragicEvent(oom, "rollbackInternal");
     } finally {
       if (!success) {
         // Must not hold IW's lock while closing
@@ -2165,7 +2100,7 @@ public class IndexWriter implements Clos
             globalFieldNumberMap.clear();
             success = true;
           } catch (OutOfMemoryError oom) {
-            handleOOM(oom, "deleteAll");
+            tragicEvent(oom, "deleteAll");
           } finally {
             if (!success) {
               if (infoStream.isEnabled("IW")) {
@@ -2399,11 +2334,6 @@ public class IndexWriter implements Clos
    *
    * <p>This requires this index not be among those to be added.
    *
-   * <p>
-   * <b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer. See <a
-   * href="#OOME">above</a> for details.
-   *
    * @throws CorruptIndexException if the index is corrupt
    * @throws IOException if there is a low-level IO error
    * @throws LockObtainFailedException if we were unable to
@@ -2495,7 +2425,7 @@ public class IndexWriter implements Clos
       successTop = true;
 
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "addIndexes(Directory...)");
+      tragicEvent(oom, "addIndexes(Directory...)");
     } finally {
       if (successTop) {
         IOUtils.close(locks);
@@ -2517,10 +2447,6 @@ public class IndexWriter implements Clos
    * free space required in the Directory, and non-CFS segments on an Exception.
    * 
    * <p>
-   * <b>NOTE</b>: if this method hits an OutOfMemoryError you should immediately
-   * close the writer. See <a href="#OOME">above</a> for details.
-   * 
-   * <p>
    * <b>NOTE:</b> empty segments are dropped by this method and not added to
this
    * index.
    * 
@@ -2657,7 +2583,7 @@ public class IndexWriter implements Clos
         checkpoint();
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "addIndexes(IndexReader...)");
+      tragicEvent(oom, "addIndexes(IndexReader...)");
     }
     maybeMerge();
   }
@@ -2756,10 +2682,6 @@ public class IndexWriter implements Clos
    * <p>You can also just call {@link #commit()} directly
    *  without prepareCommit first in which case that method
    *  will internally call prepareCommit.
-   *
-   *  <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   *  you should immediately close the writer.  See <a
-   *  href="#OOME">above</a> for details.</p>
    */
   @Override
   public final void prepareCommit() throws IOException {
@@ -2776,8 +2698,8 @@ public class IndexWriter implements Clos
         infoStream.message("IW", "  index before flush " + segString());
       }
 
-      if (hitOOM) {
-        throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot commit");
+      if (tragedy != null) {
+        throw new IllegalStateException("this writer hit an unrecoverable error; cannot commit",
tragedy);
       }
 
       if (pendingCommit != null) {
@@ -2843,7 +2765,7 @@ public class IndexWriter implements Clos
           }
         }
       } catch (OutOfMemoryError oom) {
-        handleOOM(oom, "prepareCommit");
+        tragicEvent(oom, "prepareCommit");
       }
      
       boolean success = false;
@@ -2916,10 +2838,6 @@ public class IndexWriter implements Clos
    * loss it may still lose data.  Lucene cannot guarantee
    * consistency on such devices.  </p>
    *
-   * <p><b>NOTE</b>: if this method hits an OutOfMemoryError
-   * you should immediately close the writer.  See <a
-   * href="#OOME">above</a> for details.</p>
-   *
    * @see #prepareCommit
    */
   @Override
@@ -2971,25 +2889,43 @@ public class IndexWriter implements Clos
 
   private synchronized final void finishCommit() throws IOException {
 
+    boolean success = false;
+    
     if (pendingCommit != null) {
       try {
         if (infoStream.isEnabled("IW")) {
           infoStream.message("IW", "commit: pendingCommit != null");
         }
         pendingCommit.finishCommit(directory);
-        if (infoStream.isEnabled("IW")) {
-          infoStream.message("IW", "commit: wrote segments file \"" + pendingCommit.getSegmentsFileName()
+ "\"");
+        success = true;
+        // we committed, if anything goes wrong after this: we are screwed
+        try {
+          if (infoStream.isEnabled("IW")) {
+            infoStream.message("IW", "commit: wrote segments file \"" + pendingCommit.getSegmentsFileName()
+ "\"");
+          }
+          segmentInfos.updateGeneration(pendingCommit);
+          lastCommitChangeCount = pendingCommitChangeCount;
+          rollbackSegments = pendingCommit.createBackupSegmentInfos();
+          // NOTE: don't use this.checkpoint() here, because
+          // we do not want to increment changeCount:
+          deleter.checkpoint(pendingCommit, true);
+        } catch (Throwable tragedy) {
+          tragicEvent(tragedy, "finishCommit");
         }
-        segmentInfos.updateGeneration(pendingCommit);
-        lastCommitChangeCount = pendingCommitChangeCount;
-        rollbackSegments = pendingCommit.createBackupSegmentInfos();
-        // NOTE: don't use this.checkpoint() here, because
-        // we do not want to increment changeCount:
-        deleter.checkpoint(pendingCommit, true);
       } finally {
         // Matches the incRef done in prepareCommit:
         try {
-          deleter.decRef(filesToCommit);
+          if (success == false || tragedy == null) {
+            try {
+              deleter.decRef(filesToCommit);
+            } catch (Throwable t) {
+              // if the commit succeeded, we are in screwed state
+              // otherwise, throw our original exception
+              if (success) {
+                tragicEvent(tragedy, "finishCommit");
+              }
+            } 
+          }
         } finally {
           filesToCommit = null;
           pendingCommit = null;
@@ -3045,8 +2981,8 @@ public class IndexWriter implements Clos
   }
 
   private boolean doFlush(boolean applyAllDeletes) throws IOException {
-    if (hitOOM) {
-      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot flush");
+    if (tragedy != null) {
+      throw new IllegalStateException("this writer hit an unrecoverable error; cannot flush",
tragedy);
     }
 
     doBeforeFlush();
@@ -3081,7 +3017,7 @@ public class IndexWriter implements Clos
         return anySegmentFlushed;
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "doFlush");
+      tragicEvent(oom, "doFlush");
       // never hit
       return false;
     } finally {
@@ -3407,8 +3343,8 @@ public class IndexWriter implements Clos
 
     assert testPoint("startCommitMerge");
 
-    if (hitOOM) {
-      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot complete
merge");
+    if (tragedy != null) {
+      throw new IllegalStateException("this writer hit an unrecoverable error; cannot complete
merge", tragedy);
     }
 
     if (infoStream.isEnabled("IW")) {
@@ -3625,7 +3561,7 @@ public class IndexWriter implements Clos
         }
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "merge");
+      tragicEvent(oom, "merge");
     }
     if (merge.info != null && !merge.isAborted()) {
       if (infoStream.isEnabled("IW")) {
@@ -3754,8 +3690,8 @@ public class IndexWriter implements Clos
     assert merge.registerDone;
     assert merge.maxNumSegments == -1 || merge.maxNumSegments > 0;
 
-    if (hitOOM) {
-      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot merge");
+    if (tragedy != null) {
+      throw new IllegalStateException("this writer hit an unrecoverable error; cannot merge",
tragedy);
     }
 
     if (merge.info != null) {
@@ -4285,8 +4221,8 @@ public class IndexWriter implements Clos
     assert testPoint("startStartCommit");
     assert pendingCommit == null;
 
-    if (hitOOM) {
-      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot commit");
+    if (tragedy != null) {
+      throw new IllegalStateException("this writer hit an unrecoverable error; cannot commit",
tragedy);
     }
 
     try {
@@ -4384,7 +4320,7 @@ public class IndexWriter implements Clos
         }
       }
     } catch (OutOfMemoryError oom) {
-      handleOOM(oom, "startCommit");
+      tragicEvent(oom, "startCommit");
     }
     assert testPoint("finishStartCommit");
   }
@@ -4436,12 +4372,26 @@ public class IndexWriter implements Clos
     public abstract void warm(AtomicReader reader) throws IOException;
   }
 
-  private void handleOOM(OutOfMemoryError oom, String location) {
+  private void tragicEvent(Throwable tragedy, String location) {
     if (infoStream.isEnabled("IW")) {
-      infoStream.message("IW", "hit OutOfMemoryError inside " + location);
+      infoStream.message("IW", "hit " + tragedy.getClass().getSimpleName() + " inside " +
location);
+    }
+    // its possible you could have a really bad day
+    if (this.tragedy == null) {
+      this.tragedy = tragedy;
+    }
+    // if we are already closed (e.g. called by rollback), this will be a no-op.
+    synchronized(commitLock) {
+      if (closing == false) {
+        try {
+          rollback();
+        } catch (Throwable ignored) {
+          // it would be confusing to addSuppressed here, its unrelated to the disaster,
+          // and its possible our internal state is amiss anyway.
+        }
+      }
     }
-    hitOOM = true;
-    throw oom;
+    IOUtils.reThrowUnchecked(tragedy);
   }
 
   // Used only by assert for testing.  Current points:

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/AlreadyClosedException.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/AlreadyClosedException.java?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/AlreadyClosedException.java
(original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/store/AlreadyClosedException.java
Wed Sep 17 23:39:42 2014
@@ -25,4 +25,8 @@ public class AlreadyClosedException exte
   public AlreadyClosedException(String message) {
     super(message);
   }
+  
+  public AlreadyClosedException(String message, Throwable cause) {
+    super(message, cause);
+  }
 }

Modified: lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/IOUtils.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/IOUtils.java?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/IOUtils.java (original)
+++ lucene/dev/trunk/lucene/core/src/java/org/apache/lucene/util/IOUtils.java Wed Sep 17 23:39:42
2014
@@ -341,7 +341,7 @@ public final class IOUtils {
   }
 
   /**
-   * Simple utilty method that takes a previously caught
+   * Simple utility method that takes a previously caught
    * {@code Throwable} and rethrows either {@code
    * IOException} or an unchecked exception.  If the
    * argument is null then this method does nothing.
@@ -356,7 +356,7 @@ public final class IOUtils {
   }
 
   /**
-   * Simple utilty method that takes a previously caught
+   * Simple utility method that takes a previously caught
    * {@code Throwable} and rethrows it as an unchecked exception.
    * If the argument is null then this method does nothing.
    */

Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java
(original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java
Wed Sep 17 23:39:42 2014
@@ -26,6 +26,7 @@ import org.apache.lucene.codecs.Codec;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexWriterConfig.OpenMode;
+import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.IndexInput;
@@ -479,6 +480,8 @@ public class TestIndexFileDeleter extend
       } catch (RuntimeException re) {
         if (re.getMessage().equals("fake fail")) {
           // ok
+        } else if (re instanceof AlreadyClosedException && re.getCause() != null
&& "fake fail".equals(re.getCause().getMessage())) {
+          break; // our test got unlucky, triggered our strange exception after successful
finishCommit, caused a disaster!
         } else {
           throw re;
         }

Modified: lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOutOfMemory.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOutOfMemory.java?rev=1625853&r1=1625852&r2=1625853&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOutOfMemory.java
(original)
+++ lucene/dev/trunk/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterOutOfMemory.java
Wed Sep 17 23:39:42 2014
@@ -37,6 +37,7 @@ import org.apache.lucene.document.Sorted
 import org.apache.lucene.document.SortedSetDocValuesField;
 import org.apache.lucene.document.StoredField;
 import org.apache.lucene.document.TextField;
+import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.MockDirectoryWrapper;
 import org.apache.lucene.store.MockDirectoryWrapper.Failure;
 import org.apache.lucene.util.BytesRef;
@@ -136,17 +137,9 @@ public class TestIndexWriterOutOfMemory 
               } else if (thingToDo == 2) {
                 iw.updateBinaryDocValue(new Term("id", Integer.toString(i)), "dv2", new BytesRef(Integer.toString(i+1)));
               }
-            } catch (OutOfMemoryError e) {
-              if (e.getMessage() != null && e.getMessage().startsWith("Fake OutOfMemoryError"))
{
-                exceptionStream.println("\nTEST: got expected fake exc:" + e.getMessage());
-                e.printStackTrace(exceptionStream);
-                try {
-                  iw.rollback();
-                } catch (Throwable t) {}
-                continue STARTOVER;
-              } else {
-                Rethrow.rethrow(e);
-              }
+            } catch (OutOfMemoryError | AlreadyClosedException disaster) {
+              getOOM(disaster, iw, exceptionStream);
+              continue STARTOVER;
             }
           } else {
             // block docs
@@ -163,16 +156,8 @@ public class TestIndexWriterOutOfMemory 
               if (random().nextBoolean()) {
                 iw.deleteDocuments(new Term("id", Integer.toString(i)), new Term("id", Integer.toString(-i)));
               }
-            } catch (OutOfMemoryError e) {
-              if (e.getMessage() != null && e.getMessage().startsWith("Fake OutOfMemoryError"))
{
-                exceptionStream.println("\nTEST: got expected fake exc:" + e.getMessage());
-                e.printStackTrace(exceptionStream);
-              } else {
-                Rethrow.rethrow(e);
-              }
-              try {
-                iw.rollback();
-              } catch (Throwable t) {}
+            } catch (OutOfMemoryError | AlreadyClosedException disaster) {
+              getOOM(disaster, iw, exceptionStream);
               continue STARTOVER;
             }
           }
@@ -194,16 +179,8 @@ public class TestIndexWriterOutOfMemory 
               if (DirectoryReader.indexExists(dir)) {
                 TestUtil.checkIndex(dir);
               }
-            } catch (OutOfMemoryError e) {
-              if (e.getMessage() != null && e.getMessage().startsWith("Fake OutOfMemoryError"))
{
-                exceptionStream.println("\nTEST: got expected fake exc:" + e.getMessage());
-                e.printStackTrace(exceptionStream);
-              } else {
-                Rethrow.rethrow(e);
-              }
-              try {
-                iw.rollback();
-              } catch (Throwable t) {}
+            } catch (OutOfMemoryError | AlreadyClosedException disaster) {
+              getOOM(disaster, iw, exceptionStream);
               continue STARTOVER;
             }
           }
@@ -211,17 +188,9 @@ public class TestIndexWriterOutOfMemory 
         
         try {
           iw.close();
-        } catch (OutOfMemoryError e) {
-          if (e.getMessage() != null && e.getMessage().startsWith("Fake OutOfMemoryError"))
{
-            exceptionStream.println("\nTEST: got expected fake exc:" + e.getMessage());
-            e.printStackTrace(exceptionStream);
-            try {
-              iw.rollback();
-            } catch (Throwable t) {}
-            continue STARTOVER;
-          } else {
-            Rethrow.rethrow(e);
-          }
+        } catch (OutOfMemoryError | AlreadyClosedException disaster) {
+          getOOM(disaster, iw, exceptionStream);
+          continue STARTOVER;
         }
       } catch (Throwable t) {
         System.out.println("Unexpected exception: dumping fake-exception-log:...");
@@ -238,6 +207,27 @@ public class TestIndexWriterOutOfMemory 
     }
   }
   
+  private OutOfMemoryError getOOM(Throwable disaster, IndexWriter writer, PrintStream log)
{
+    Throwable e = disaster;
+    if (e instanceof AlreadyClosedException) {
+      e = e.getCause();
+    }
+    
+    if (e instanceof OutOfMemoryError && e.getMessage() != null && e.getMessage().startsWith("Fake
OutOfMemoryError")) {
+      log.println("\nTEST: got expected fake exc:" + e.getMessage());
+      e.printStackTrace(log);
+      // TODO: remove rollback here, and add this assert to ensure "full OOM protection"
anywhere IW does writes
+      // assertTrue("hit OOM but writer is still open, WTF: ", writer.isClosed());
+      try {
+        writer.rollback();
+      } catch (Throwable t) {}
+      return (OutOfMemoryError) e;
+    } else {
+      Rethrow.rethrow(disaster);
+      return null; // dead
+    }
+  }
+  
   public void testBasics() throws Exception {
     final Random r = new Random(random().nextLong());
     doTest(new Failure() {
@@ -258,7 +248,6 @@ public class TestIndexWriterOutOfMemory 
     });
   }
   
-  @Ignore("LUCENE-5958: not yet")
   public void testCheckpoint() throws Exception {
     final Random r = new Random(random().nextLong());
     doTest(new Failure() {



Mime
View raw message