You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2014/09/03 17:31:59 UTC
svn commit: r1622281 - 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: mikemccand
Date: Wed Sep 3 15:31:59 2014
New Revision: 1622281
URL: http://svn.apache.org/r1622281
Log:
LUCENE-5919: fix IndexWriter exception handling when calling IndexFileDeleter.decRef
Modified:
lucene/dev/branches/branch_4x/ (props changed)
lucene/dev/branches/branch_4x/lucene/ (props changed)
lucene/dev/branches/branch_4x/lucene/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_4x/lucene/core/ (props changed)
lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.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/TestIndexFileDeleter.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=1622281&r1=1622280&r2=1622281&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/CHANGES.txt (original)
+++ lucene/dev/branches/branch_4x/lucene/CHANGES.txt Wed Sep 3 15:31:59 2014
@@ -49,6 +49,11 @@ Bug Fixes
tests (and test iterations). Fix for FaultyIndexInput in particular.
(Dawid Weiss)
+* LUCENE-5919: Fix exception handling inside IndexWriter when
+ deleteFile throws an exception, to not over-decRef index files,
+ possibly deleting a file that's still in use in the index, leading
+ to corruption. (Mike McCandless)
+
Build
* LUCENE-5909: Smoke tester now has better command line parsing and
Modified: lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java?rev=1622281&r1=1622280&r2=1622281&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java Wed Sep 3 15:31:59 2014
@@ -33,6 +33,7 @@ import org.apache.lucene.store.AlreadyCl
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.NoSuchDirectoryException;
import org.apache.lucene.util.CollectionUtil;
+import org.apache.lucene.util.IOUtils;
import org.apache.lucene.util.InfoStream;
/*
@@ -367,17 +368,25 @@ final class IndexFileDeleter implements
// First decref all files that had been referred to by
// the now-deleted commits:
+ Throwable firstThrowable = null;
for(int i=0;i<size;i++) {
CommitPoint commit = commitsToDelete.get(i);
if (infoStream.isEnabled("IFD")) {
infoStream.message("IFD", "deleteCommits: now decRef commit \"" + commit.getSegmentsFileName() + "\"");
}
- for (final String file : commit.files) {
- decRef(file);
+ try {
+ decRef(commit.files);
+ } catch (Throwable t) {
+ if (firstThrowable == null) {
+ firstThrowable = t;
+ }
}
}
commitsToDelete.clear();
+ // NOTE: does nothing if firstThrowable is null
+ IOUtils.reThrow(firstThrowable);
+
// Now compact commits to remove deleted ones (preserving the sort):
size = commits.size();
int readFrom = 0;
@@ -456,8 +465,11 @@ final class IndexFileDeleter implements
assert locked();
if (!lastFiles.isEmpty()) {
- decRef(lastFiles);
- lastFiles.clear();
+ try {
+ decRef(lastFiles);
+ } finally {
+ lastFiles.clear();
+ }
}
deletePendingFiles();
@@ -554,8 +566,11 @@ final class IndexFileDeleter implements
deleteCommits();
} else {
// DecRef old files from the last checkpoint, if any:
- decRef(lastFiles);
- lastFiles.clear();
+ try {
+ decRef(lastFiles);
+ } finally {
+ lastFiles.clear();
+ }
// Save files so we can decr on next checkpoint/commit:
lastFiles.addAll(segmentInfos.files(directory, false));
@@ -593,10 +608,34 @@ final class IndexFileDeleter implements
rc.IncRef();
}
+ /** Decrefs all provided files, even on exception; throws first exception hit, if any. */
void decRef(Collection<String> files) throws IOException {
assert locked();
+ Throwable firstThrowable = null;
+ for(final String file : files) {
+ try {
+ decRef(file);
+ } catch (Throwable t) {
+ if (firstThrowable == null) {
+ // Save first exception and throw it in the end, but be sure to finish decRef all files
+ firstThrowable = t;
+ }
+ }
+ }
+
+ // NOTE: does nothing if firstThrowable is null
+ IOUtils.reThrow(firstThrowable);
+ }
+
+ /** Decrefs all provided files, ignoring any exceptions hit; call this if
+ * you are already handling an exception. */
+ void decRefWhileHandlingException(Collection<String> files) throws IOException {
+ assert locked();
for(final String file : files) {
- decRef(file);
+ try {
+ decRef(file);
+ } catch (Throwable t) {
+ }
}
}
@@ -611,16 +650,17 @@ final class IndexFileDeleter implements
if (0 == rc.DecRef()) {
// This file is no longer referenced by any past
// commit points nor by the in-memory SegmentInfos:
- deleteFile(fileName);
- refCounts.remove(fileName);
+ try {
+ deleteFile(fileName);
+ } finally {
+ refCounts.remove(fileName);
+ }
}
}
void decRef(SegmentInfos segmentInfos) throws IOException {
assert locked();
- for (final String file : segmentInfos.files(directory, false)) {
- decRef(file);
- }
+ decRef(segmentInfos.files(directory, false));
}
public boolean exists(String fileName) {
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=1622281&r1=1622280&r2=1622281&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 Wed Sep 3 15:31:59 2014
@@ -2171,9 +2171,12 @@ public class IndexWriter implements Clos
if (pendingCommit != null) {
pendingCommit.rollbackCommit(directory);
- deleter.decRef(pendingCommit);
- pendingCommit = null;
- notifyAll();
+ try {
+ deleter.decRef(pendingCommit);
+ } finally {
+ pendingCommit = null;
+ notifyAll();
+ }
}
// Don't bother saving any changes in our segmentInfos
@@ -2226,7 +2229,9 @@ public class IndexWriter implements Clos
try {
pendingCommit.rollbackCommit(directory);
deleter.decRef(pendingCommit);
- } catch (Throwable t) {}
+ } catch (Throwable t) {
+ }
+ pendingCommit = null;
}
// close all the closeables we can (but important is readerPool and writeLock to prevent leaks)
@@ -3066,7 +3071,7 @@ public class IndexWriter implements Clos
if (!success) {
synchronized (this) {
if (filesToCommit != null) {
- deleter.decRef(filesToCommit);
+ deleter.decRefWhileHandlingException(filesToCommit);
filesToCommit = null;
}
}
@@ -3197,10 +3202,13 @@ public class IndexWriter implements Clos
deleter.checkpoint(pendingCommit, true);
} finally {
// Matches the incRef done in prepareCommit:
- deleter.decRef(filesToCommit);
- filesToCommit = null;
- pendingCommit = null;
- notifyAll();
+ try {
+ deleter.decRef(filesToCommit);
+ } finally {
+ filesToCommit = null;
+ pendingCommit = null;
+ notifyAll();
+ }
}
if (infoStream.isEnabled("IW")) {
@@ -4518,8 +4526,11 @@ public class IndexWriter implements Clos
if (infoStream.isEnabled("IW")) {
infoStream.message("IW", " skip startCommit(): no changes pending");
}
- deleter.decRef(filesToCommit);
- filesToCommit = null;
+ try {
+ deleter.decRef(filesToCommit);
+ } finally {
+ filesToCommit = null;
+ }
return;
}
@@ -4590,7 +4601,7 @@ public class IndexWriter implements Clos
}
// Hit exception
- deleter.decRef(filesToCommit);
+ deleter.decRefWhileHandlingException(filesToCommit);
filesToCommit = null;
}
}
Modified: lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java?rev=1622281&r1=1622280&r2=1622281&view=diff
==============================================================================
--- lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java (original)
+++ lucene/dev/branches/branch_4x/lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java Wed Sep 3 15:31:59 2014
@@ -399,4 +399,95 @@ public class TestIndexFileDeleter extend
dir.close();
}
+
+ // LUCENE-5919
+ public void testExcInDecRef() throws Exception {
+ MockDirectoryWrapper dir = newMockDirectory();
+
+ // disable slow things: we don't rely upon sleeps here.
+ dir.setThrottling(MockDirectoryWrapper.Throttling.NEVER);
+ dir.setUseSlowOpenClosers(false);
+
+ final AtomicBoolean doFailExc = new AtomicBoolean();
+
+ dir.failOn(new MockDirectoryWrapper.Failure() {
+ @Override
+ public void eval(MockDirectoryWrapper dir) throws IOException {
+ if (doFailExc.get() && random().nextInt(4) == 1) {
+ Exception e = new Exception();
+ StackTraceElement stack[] = e.getStackTrace();
+ for (int i = 0; i < stack.length; i++) {
+ if (stack[i].getClassName().equals(IndexFileDeleter.class.getName()) && stack[i].getMethodName().equals("decRef")) {
+ throw new RuntimeException("fake fail");
+ }
+ }
+ }
+ }
+ });
+
+ IndexWriterConfig iwc = newIndexWriterConfig(new MockAnalyzer(random()));
+ //iwc.setMergeScheduler(new SerialMergeScheduler());
+ MergeScheduler ms = iwc.getMergeScheduler();
+ if (ms instanceof ConcurrentMergeScheduler) {
+ final ConcurrentMergeScheduler suppressFakeFail = new ConcurrentMergeScheduler() {
+ @Override
+ protected void handleMergeException(Throwable exc) {
+ // suppress only FakeIOException:
+ if (exc instanceof RuntimeException && exc.getMessage().equals("fake fail")) {
+ // ok to ignore
+ } else {
+ super.handleMergeException(exc);
+ }
+ }
+ };
+ final ConcurrentMergeScheduler cms = (ConcurrentMergeScheduler) ms;
+ suppressFakeFail.setMaxMergesAndThreads(cms.getMaxMergeCount(), cms.getMaxThreadCount());
+ suppressFakeFail.setMergeThreadPriority(cms.getMergeThreadPriority());
+ iwc.setMergeScheduler(suppressFakeFail);
+ }
+
+ RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc);
+
+ // Since we hit exc during merging, a partial
+ // forceMerge can easily return when there are still
+ // too many segments in the index:
+ w.setDoRandomForceMergeAssert(false);
+
+ doFailExc.set(true);
+ int ITERS = atLeast(1000);
+ for(int iter=0;iter<ITERS;iter++) {
+ try {
+ if (random().nextInt(10) == 5) {
+ w.commit();
+ } else if (random().nextInt(10) == 7) {
+ w.getReader().close();
+ } else {
+ Document doc = new Document();
+ doc.add(newTextField("field", "some text", Field.Store.NO));
+ w.addDocument(doc);
+ }
+ } catch (IOException ioe) {
+ if (ioe.getMessage().contains("background merge hit exception")) {
+ Throwable cause = ioe.getCause();
+ if (cause != null && cause instanceof RuntimeException && ((RuntimeException) cause).getMessage().equals("fake fail")) {
+ // ok
+ } else {
+ throw ioe;
+ }
+ } else {
+ throw ioe;
+ }
+ } catch (RuntimeException re) {
+ if (re.getMessage().equals("fake fail")) {
+ // ok
+ } else {
+ throw re;
+ }
+ }
+ }
+
+ doFailExc.set(false);
+ w.close();
+ dir.close();
+ }
}