From b58e470eb7061ba8ac4ac649dadc66bb2b64a29f Mon Sep 17 00:00:00 2001 From: Michael McCandless Date: Thu, 30 Apr 2009 19:50:34 +0000 Subject: [PATCH] LUCENE-1611: fix case where OutOfMemoryException in IndexWriter could cause infinite merging to happen git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@770414 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 4 + .../org/apache/lucene/index/IndexWriter.java | 104 ++++++++++++------ 2 files changed, 75 insertions(+), 33 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index c01a61baf1f..4b0c7224a3c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -150,6 +150,10 @@ Bug fixes 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 diff --git a/src/java/org/apache/lucene/index/IndexWriter.java b/src/java/org/apache/lucene/index/IndexWriter.java index ce88afd56a2..3296626dc50 100644 --- a/src/java/org/apache/lucene/index/IndexWriter.java +++ b/src/java/org/apache/lucene/index/IndexWriter.java @@ -2043,7 +2043,9 @@ public class IndexWriter { // 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 @@ public class IndexWriter { 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 @@ public class IndexWriter { closed = true; } } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "closeInternal"); } finally { synchronized(this) { closing = false; @@ -2356,8 +2359,7 @@ public class IndexWriter { if (doFlush) flush(true, false, false); } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "addDocument"); } } @@ -2379,8 +2381,7 @@ public class IndexWriter { if (doFlush) flush(true, false, false); } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "deleteDocuments(Term)"); } } @@ -2404,8 +2405,7 @@ public class IndexWriter { if (doFlush) flush(true, false, false); } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "deleteDocuments(Term[])"); } } @@ -2514,8 +2514,7 @@ public class IndexWriter { if (doFlush) flush(true, false, false); } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "updateDocument"); } } @@ -2708,6 +2707,11 @@ public class IndexWriter { 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 @@ public class IndexWriter { 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 @@ public class IndexWriter { 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 @@ public class IndexWriter { success = true; } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "rollbackInternal"); } finally { synchronized(this) { if (!success) { @@ -3375,8 +3387,7 @@ public class IndexWriter { } } } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "addIndexes(Directory[])"); } finally { if (docWriter != null) { docWriter.resumeAllThreads(); @@ -3518,8 +3529,7 @@ public class IndexWriter { } } } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "addIndexesNoOptimize"); } finally { if (docWriter != null) { docWriter.resumeAllThreads(); @@ -3761,8 +3771,7 @@ public class IndexWriter { } } } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "addIndexes(IndexReader[])"); } finally { if (docWriter != null) { docWriter.resumeAllThreads(); @@ -3794,8 +3803,9 @@ public class IndexWriter { * @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 @@ public class IndexWriter { 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 @@ public class IndexWriter { // 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 @@ public class IndexWriter { 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 @@ public class IndexWriter { 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 @@ public class IndexWriter { } } } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "merge"); } } @@ -4477,6 +4503,10 @@ public class IndexWriter { 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 @@ public class IndexWriter { assert testPoint("startStartCommit"); - if (hitOOM) - return; + if (hitOOM) { + throw new IllegalStateException("this writer hit an OutOfMemoryError; cannot commit"); + } try { @@ -5273,8 +5304,7 @@ public class IndexWriter { } } } catch (OutOfMemoryError oom) { - hitOOM = true; - throw oom; + handleOOM(oom, "startCommit"); } assert testPoint("finishStartCommit"); } @@ -5397,6 +5427,14 @@ public class IndexWriter { 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