You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2014/03/21 17:51:30 UTC
svn commit: r1579978 - in /lucene/dev/branches/branch_4x: ./ lucene/
lucene/core/ lucene/core/src/java/org/apache/lucene/index/
lucene/core/src/test/org/apache/lucene/index/
Author: rmuir
Date: Fri Mar 21 16:51:29 2014
New Revision: 1579978
URL: http://svn.apache.org/r1579978
Log:
LUCENE-5544: Exceptions during IW.rollback can leak files and locks
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt
lucene/dev/branches/branch_4x/lucene/core/ (props changed)
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
Modified: lucene/dev/branches/branch_4x/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/CHANGES.txt?rev=1579978&r1=1579977&r2=1579978&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Fri Mar 21 16:51:29 2014
@@ -187,6 +187,9 @@ Bug fixes
* LUCENE-5538: Fix FastVectorHighlighter bug with index-time synonyms when the
query is more complex than a single phrase. (Robert Muir)
+* LUCENE-5544: Exceptions during IndexWriter.rollback could leak file handles
+ and the write lock. (Robert Muir)
+
Test Framework
* LUCENE-5449: Rename _TestUtil and _TestHelper to remove the leading _.
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java?rev=1579978&r1=1579977&r2=1579978&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java Fri Mar 21 16:51:29 2014
@@ -17,6 +17,7 @@ package org.apache.lucene.index;
* limitations under the License.
*/
+import java.io.Closeable;
import java.io.IOException;
import java.util.Collection;
import java.util.HashSet;
@@ -102,7 +103,7 @@ import org.apache.lucene.util.InfoStream
* or none") added to the index.
*/
-final class DocumentsWriter {
+final class DocumentsWriter implements Closeable {
private final Directory directory;
private volatile boolean closed;
@@ -345,7 +346,8 @@ final class DocumentsWriter {
return deleteQueue.anyChanges();
}
- void close() {
+ @Override
+ public void close() {
closed = true;
flushControl.setClosed();
}
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java?rev=1579978&r1=1579977&r2=1579978&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java Fri Mar 21 16:51:29 2014
@@ -422,7 +422,7 @@ public class IndexWriter implements Clos
* places if it is in "near real-time mode" (getReader()
* has been called on this instance). */
- class ReaderPool {
+ class ReaderPool implements Closeable {
private final Map<SegmentCommitInfo,ReadersAndUpdates> readerMap = new HashMap<>();
@@ -489,6 +489,11 @@ public class IndexWriter implements Clos
readerMap.remove(rld.info);
}
}
+
+ @Override
+ public void close() throws IOException {
+ dropAll(false);
+ }
/** Remove all our references to readers, and commits
* any pending changes. */
@@ -2069,8 +2074,8 @@ public class IndexWriter implements Clos
*/
@Override
public void rollback() throws IOException {
- ensureOpen();
-
+ // don't call ensureOpen here: this acts like "close()" in closeable.
+
// Ensure that only one thread actually gets to do the
// closing, and make sure no commit is also in progress:
synchronized(commitLock) {
@@ -2140,6 +2145,15 @@ public class IndexWriter implements Clos
deleter.refresh();
lastCommitChangeCount = changeCount;
+
+ processEvents(false, true);
+ deleter.refresh();
+ deleter.close();
+
+ IOUtils.close(writeLock); // release write lock
+ writeLock = null;
+
+ assert docWriter.perThreadPool.numDeactivatedThreadStates() == docWriter.perThreadPool.getMaxThreadStates() : "" + docWriter.perThreadPool.numDeactivatedThreadStates() + " " + docWriter.perThreadPool.getMaxThreadStates();
}
success = true;
@@ -2148,16 +2162,29 @@ public class IndexWriter implements Clos
} finally {
synchronized(this) {
if (!success) {
- closing = false;
- notifyAll();
- if (infoStream.isEnabled("IW")) {
- infoStream.message("IW", "hit exception during rollback");
+ // we tried to be nice about it: do the minimum
+
+ // don't leak a segments_N file if there is a pending commit
+ if (pendingCommit != null) {
+ try {
+ pendingCommit.rollbackCommit(directory);
+ deleter.decRef(pendingCommit);
+ } catch (Throwable t) {}
}
+
+ // close all the closeables we can (but important is readerPool and writeLock to prevent leaks)
+ IOUtils.closeWhileHandlingException(mergePolicy, mergeScheduler, readerPool, deleter, writeLock);
+ writeLock = null;
+ }
+ closed = true;
+ closing = false;
+ try {
+ processEvents(false, true);
+ } finally {
+ notifyAll();
}
}
}
-
- closeInternal(false, false);
}
/**
Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java?rev=1579978&r1=1579977&r2=1579978&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java Fri Mar 21 16:51:29 2014
@@ -1944,5 +1944,135 @@ public class TestIndexWriterExceptions e
dir.close();
}
+
+ public void testExceptionDuringRollback() throws Exception {
+ // currently: fail in two different places
+ final String messageToFailOn = random().nextBoolean() ?
+ "rollback: done finish merges" : "rollback before checkpoint";
+
+ // infostream that throws exception during rollback
+ InfoStream evilInfoStream = new InfoStream() {
+ @Override
+ public void message(String component, String message) {
+ if (messageToFailOn.equals(message)) {
+ throw new RuntimeException("BOOM!");
+ }
+ }
+ @Override
+ public boolean isEnabled(String component) {
+ return true;
+ }
+
+ @Override
+ public void close() throws IOException {}
+ };
+
+ Directory dir = newMockDirectory(); // we want to ensure we don't leak any locks or file handles
+ IndexWriterConfig iwc = new IndexWriterConfig(TEST_VERSION_CURRENT, null);
+ iwc.setInfoStream(evilInfoStream);
+ IndexWriter iw = new IndexWriter(dir, iwc);
+ Document doc = new Document();
+ for (int i = 0; i < 10; i++) {
+ iw.addDocument(doc);
+ }
+ iw.commit();
+
+ iw.addDocument(doc);
+
+ // pool readers
+ DirectoryReader r = DirectoryReader.open(iw, false);
+
+ // sometimes sneak in a pending commit: we don't want to leak a file handle to that segments_N
+ if (random().nextBoolean()) {
+ iw.prepareCommit();
+ }
+
+ try {
+ iw.rollback();
+ fail();
+ } catch (RuntimeException expected) {
+ assertEquals("BOOM!", expected.getMessage());
+ }
+
+ r.close();
+
+ // even though we hit exception: we are closed, no locks or files held, index in good state
+ assertTrue(iw.isClosed());
+ assertFalse(IndexWriter.isLocked(dir));
+
+ r = DirectoryReader.open(dir);
+ assertEquals(10, r.maxDoc());
+ r.close();
+
+ // no leaks
+ dir.close();
+ }
+
+ public void testRandomExceptionDuringRollback() throws Exception {
+ // fail in random places on i/o
+ final int numIters = RANDOM_MULTIPLIER * 75;
+ for (int iter = 0; iter < numIters; iter++) {
+ MockDirectoryWrapper dir = newMockDirectory();
+ dir.failOn(new MockDirectoryWrapper.Failure() {
+
+ @Override
+ public void eval(MockDirectoryWrapper dir) throws IOException {
+ boolean maybeFail = false;
+ StackTraceElement[] trace = new Exception().getStackTrace();
+
+ for (int i = 0; i < trace.length; i++) {
+ if ("rollbackInternal".equals(trace[i].getMethodName())) {
+ maybeFail = true;
+ break;
+ }
+ }
+
+ if (maybeFail && random().nextInt(10) == 0) {
+ if (VERBOSE) {
+ System.out.println("TEST: now fail; thread=" + Thread.currentThread().getName() + " exc:");
+ new Throwable().printStackTrace(System.out);
+ }
+ throw new FakeIOException();
+ }
+ }
+ });
+
+ IndexWriterConfig iwc = new IndexWriterConfig(TEST_VERSION_CURRENT, null);
+ IndexWriter iw = new IndexWriter(dir, iwc);
+ Document doc = new Document();
+ for (int i = 0; i < 10; i++) {
+ iw.addDocument(doc);
+ }
+ iw.commit();
+
+ iw.addDocument(doc);
+
+ // pool readers
+ DirectoryReader r = DirectoryReader.open(iw, false);
+
+ // sometimes sneak in a pending commit: we don't want to leak a file handle to that segments_N
+ if (random().nextBoolean()) {
+ iw.prepareCommit();
+ }
+
+ try {
+ iw.rollback();
+ } catch (FakeIOException expected) {
+ }
+
+ r.close();
+
+ // even though we hit exception: we are closed, no locks or files held, index in good state
+ assertTrue(iw.isClosed());
+ assertFalse(IndexWriter.isLocked(dir));
+
+ r = DirectoryReader.open(dir);
+ assertEquals(10, r.maxDoc());
+ r.close();
+
+ // no leaks
+ dir.close();
+ }
+ }
}