You are viewing a plain text version of this content. The canonical link for it is here.
Posted to java-commits@lucene.apache.org by mi...@apache.org on 2009/04/30 21:50:34 UTC

svn commit: r770414 - in /lucene/java/trunk: CHANGES.txt src/java/org/apache/lucene/index/IndexWriter.java

Author: mikemccand
Date: Thu Apr 30 19:50:34 2009
New Revision: 770414

URL: http://svn.apache.org/viewvc?rev=770414&view=rev
Log:
LUCENE-1611: fix case where OutOfMemoryException in IndexWriter could cause infinite merging to happen

Modified:
    lucene/java/trunk/CHANGES.txt
    lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java

Modified: lucene/java/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/java/trunk/CHANGES.txt?rev=770414&r1=770413&r2=770414&view=diff
==============================================================================
--- lucene/java/trunk/CHANGES.txt (original)
+++ lucene/java/trunk/CHANGES.txt Thu Apr 30 19:50:34 2009
@@ -150,6 +150,10 @@
    when loading documents from the index.  (P Eger via Mike
    McCandless)
 
+7. LUCENE-1611: Fix case where OutOfMemoryException in IndexWriter
+   could cause "infinite merging" to happen.  (Christiaan Fluit via
+   Mike McCandless)
+
 New features
 
  1. LUCENE-1411: Added expert API to open an IndexWriter on a prior

Modified: lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java?rev=770414&r1=770413&r2=770414&view=diff
==============================================================================
--- lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/java/trunk/src/java/org/apache/lucene/index/IndexWriter.java Thu Apr 30 19:50:34 2009
@@ -2043,7 +2043,9 @@
 
       // Only allow a new merge to be triggered if we are
       // going to wait for merges:
-      flush(waitForMerges, true, true);
+      if (!hitOOM) {
+        flush(waitForMerges, true, true);
+      }
 
       if (waitForMerges)
         // Give merge scheduler last chance to run, in case
@@ -2059,7 +2061,9 @@
       if (infoStream != null)
         message("now call final commit()");
       
-      commit(0);
+      if (!hitOOM) {
+        commit(0);
+      }
 
       if (infoStream != null)
         message("at close: " + segString());
@@ -2081,8 +2085,7 @@
         closed = true;
       }
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "closeInternal");
     } finally {
       synchronized(this) {
         closing = false;
@@ -2356,8 +2359,7 @@
       if (doFlush)
         flush(true, false, false);
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "addDocument");
     }
   }
 
@@ -2379,8 +2381,7 @@
       if (doFlush)
         flush(true, false, false);
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "deleteDocuments(Term)");
     }
   }
 
@@ -2404,8 +2405,7 @@
       if (doFlush)
         flush(true, false, false);
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "deleteDocuments(Term[])");
     }
   }
 
@@ -2514,8 +2514,7 @@
       if (doFlush)
         flush(true, false, false);
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "updateDocument");
     }
   }
 
@@ -2708,6 +2707,11 @@
     if (doWait) {
       synchronized(this) {
         while(true) {
+
+          if (hitOOM) {
+            throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot complete optimize");
+          }
+
           if (mergeExceptions.size() > 0) {
             // Forward any exceptions in background merge
             // threads to the current thread:
@@ -2795,6 +2799,10 @@
         boolean running = true;
         while(running) {
 
+          if (hitOOM) {
+            throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot complete expungeDeletes");
+          }
+
           // Check each merge that MergePolicy asked us to
           // do, to see if any of them are still running and
           // if any of them have hit an exception.
@@ -2883,6 +2891,11 @@
     if (stopMerges)
       return;
 
+    // Do not start new merges if we've hit OOME
+    if (hitOOM) {
+      return;
+    }
+
     final MergePolicy.MergeSpecification spec;
     if (optimize) {
       spec = mergePolicy.findMergesForOptimize(segmentInfos, this, maxNumSegmentsOptimize, segmentsToOptimize);
@@ -3196,8 +3209,7 @@
 
       success = true;
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "rollbackInternal");
     } finally {
       synchronized(this) {
         if (!success) {
@@ -3375,8 +3387,7 @@
         }
       }
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "addIndexes(Directory[])");
     } finally {
       if (docWriter != null) {
         docWriter.resumeAllThreads();
@@ -3518,8 +3529,7 @@
         }
       }
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "addIndexesNoOptimize");
     } finally {
       if (docWriter != null) {
         docWriter.resumeAllThreads();
@@ -3761,8 +3771,7 @@
         }
       }
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "addIndexes(IndexReader[])");
     } finally {
       if (docWriter != null) {
         docWriter.resumeAllThreads();
@@ -3794,8 +3803,9 @@
    * @throws IOException if there is a low-level IO error
    */
   public final void flush() throws CorruptIndexException, IOException {  
-    if (hitOOM)
+    if (hitOOM) {
       throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot flush");
+    }
 
     flush(true, false, true);
   }
@@ -3850,8 +3860,9 @@
 
   private final void prepareCommit(String commitUserData, boolean internal) throws CorruptIndexException, IOException {
 
-    if (hitOOM)
+    if (hitOOM) {
       throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot commit");
+    }
 
     if (autoCommit && !internal)
       throw new IllegalStateException("this method can only be used when autoCommit is false");
@@ -3980,6 +3991,21 @@
   // synchronized, ie, merges should be allowed to commit
   // even while a flush is happening
   private synchronized final boolean doFlush(boolean flushDocStores, boolean flushDeletes) throws CorruptIndexException, IOException {
+    try {
+      return doFlushInternal(flushDocStores, flushDeletes);
+    } finally {
+      docWriter.clearFlushPending();
+    }
+  }
+
+  // TODO: this method should not have to be entirely
+  // synchronized, ie, merges should be allowed to commit
+  // even while a flush is happening
+  private synchronized final boolean doFlushInternal(boolean flushDocStores, boolean flushDeletes) throws CorruptIndexException, IOException {
+
+    if (hitOOM) {
+      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot flush");
+    }
 
     ensureOpen(false);
 
@@ -4133,10 +4159,10 @@
       return flushDocs;
 
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "doFlush");
+      // never hit
+      return false;
     } finally {
-      docWriter.clearFlushPending();
       docWriter.resumeAllThreads();
     }
   }
@@ -4260,8 +4286,9 @@
 
     assert testPoint("startCommitMerge");
 
-    if (hitOOM)
-      return false;
+    if (hitOOM) {
+      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot complete merge");
+    }
 
     if (infoStream != null)
       message("commitMerge: " + merge.segString(directory) + " index=" + segString());
@@ -4401,8 +4428,7 @@
         }
       }
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "merge");
     }
   }
 
@@ -4477,6 +4503,10 @@
     assert merge.registerDone;
     assert !merge.optimize || merge.maxNumSegmentsOptimize > 0;
 
+    if (hitOOM) {
+      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot merge");
+    }
+
     if (merge.info != null)
       // mergeInit already done
       return;
@@ -5095,8 +5125,9 @@
 
     assert testPoint("startStartCommit");
 
-    if (hitOOM)
-      return;
+    if (hitOOM) {
+      throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot commit");
+    }
 
     try {
 
@@ -5273,8 +5304,7 @@
         }
       }
     } catch (OutOfMemoryError oom) {
-      hitOOM = true;
-      throw oom;
+      handleOOM(oom, "startCommit");
     }
     assert testPoint("finishStartCommit");
   }
@@ -5397,6 +5427,14 @@
     return mergedSegmentWarmer;
   }
 
+  private void handleOOM(OutOfMemoryError oom, String location) {
+    if (infoStream != null) {
+      message("hit OutOfMemoryError inside " + location);
+    }
+    hitOOM = true;
+    throw oom;
+  }
+
   // Used only by assert for testing.  Current points:
   //   startDoFlush
   //   startCommitMerge