diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index be6965be606..7fd40137df9 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -236,6 +236,13 @@ Bug fixes * LUCENE-5568: Benchmark module's "default.codec" option didn't work. (David Smiley) +* LUCENE-5574: Closing a near-real-time reader no longer attempts to + delete unreferenced files if the original writer has been closed; + this could cause index corruption in certain cases where index files + were directly changed (deleted, overwritten, etc.) in the index + directory outside of Lucene. (Simon Willnauer, Shai Erera, Robert + Muir, Mike McCandless) + Test Framework * LUCENE-5567: When a suite fails with zombie threads failure marker and count diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java b/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java index a590a515657..611d4f7a508 100644 --- a/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java +++ b/lucene/core/src/java/org/apache/lucene/index/IndexFileDeleter.java @@ -29,6 +29,7 @@ import java.util.List; import java.util.Map; import java.util.regex.Matcher; +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; import org.apache.lucene.store.NoSuchDirectoryException; import org.apache.lucene.util.CollectionUtil; @@ -262,6 +263,14 @@ final class IndexFileDeleter implements Closeable { deleteCommits(); } + private void ensureOpen() throws AlreadyClosedException { + if (writer == null) { + throw new AlreadyClosedException("this IndexWriter is closed"); + } else { + writer.ensureOpen(false); + } + } + public SegmentInfos getLastSegmentInfos() { return lastSegmentInfos; } @@ -578,6 +587,7 @@ final class IndexFileDeleter implements Closeable { void deleteFile(String fileName) throws IOException { assert locked(); + ensureOpen(); try { if (infoStream.isEnabled("IFD")) { infoStream.message("IFD", "delete \"" + fileName + "\""); diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java index 0c6ecb07413..5702aaf2360 100644 --- a/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java +++ b/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java @@ -4566,8 +4566,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit{ deleter.revisitPolicy(); } - // Called by DirectoryReader.doClose - synchronized void deletePendingFiles() throws IOException { + private synchronized void deletePendingFiles() throws IOException { deleter.deletePendingFiles(); } @@ -4665,10 +4664,12 @@ public class IndexWriter implements Closeable, TwoPhaseCommit{ } synchronized void incRefDeleter(SegmentInfos segmentInfos) throws IOException { + ensureOpen(); deleter.incRef(segmentInfos, false); } synchronized void decRefDeleter(SegmentInfos segmentInfos) throws IOException { + ensureOpen(); deleter.decRef(segmentInfos); } diff --git a/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java index fbe4376ae60..4e4d8cdcf0c 100644 --- a/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/StandardDirectoryReader.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; import org.apache.lucene.util.IOUtils; @@ -365,11 +366,15 @@ final class StandardDirectoryReader extends DirectoryReader { } if (writer != null) { - writer.decRefDeleter(segmentInfos); - - // Since we just closed, writer may now be able to - // delete unused files: - writer.deletePendingFiles(); + try { + writer.decRefDeleter(segmentInfos); + } catch (AlreadyClosedException ex) { + // This is OK, it just means our original writer was + // closed before we were, and this may leave some + // un-referenced files in the index, which is + // harmless. The next time IW is opened on the + // index, it will delete them. + } } // throw the first exception diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java index b866b51d626..3dcbb7962d7 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriter.java @@ -2371,4 +2371,42 @@ public class TestIndexWriter extends LuceneTestCase { r.close(); dir.close(); } + + // LUCENE-5574 + public void testClosingNRTReaderDoesNotCorruptYourIndex() throws IOException { + MockDirectoryWrapper dir = newMockDirectory(); + + // Allow deletion of still open files: + dir.setNoDeleteOpenFile(false); + + // Allow writing to same file more than once: + dir.setPreventDoubleWrite(false); + + IndexWriterConfig iwc = newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random())); + LogMergePolicy lmp = new LogDocMergePolicy(); + lmp.setMergeFactor(2); + iwc.setMergePolicy(lmp); + + RandomIndexWriter w = new RandomIndexWriter(random(), dir, iwc); + Document doc = new Document(); + doc.add(new TextField("a", "foo", Field.Store.NO)); + w.addDocument(doc); + w.commit(); + w.addDocument(doc); + + // Get a new reader, but this also sets off a merge: + IndexReader r = w.getReader(); + w.close(); + + // Blow away index and make a new writer: + for(String fileName : dir.listAll()) { + dir.deleteFile(fileName); + } + + w = new RandomIndexWriter(random(), dir); + w.addDocument(doc); + w.close(); + r.close(); + dir.close(); + } } diff --git a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfUnreferencedFiles.java b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfUnreferencedFiles.java index d8068f0c9cc..0090e6d549d 100644 --- a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfUnreferencedFiles.java +++ b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfUnreferencedFiles.java @@ -22,15 +22,14 @@ import java.util.Collections; import org.apache.lucene.document.Document; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriterConfig; -import org.apache.lucene.store.Directory; import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexOutput; +import org.apache.lucene.store.MockDirectoryWrapper; import org.junit.Assert; import org.junit.Test; import org.junit.runner.JUnitCore; import org.junit.runner.Result; import org.junit.runner.notification.Failure; - import com.carrotsearch.randomizedtesting.RandomizedTest; // LUCENE-4456: Test that we fail if there are unreferenced files @@ -41,7 +40,8 @@ public class TestFailIfUnreferencedFiles extends WithNestedTests { public static class Nested1 extends WithNestedTests.AbstractNestedTest { public void testDummy() throws Exception { - Directory dir = newMockDirectory(); + MockDirectoryWrapper dir = newMockDirectory(); + dir.setAssertNoUnrefencedFilesOnClose(true); IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, null)); iw.addDocument(new Document()); iw.close(); diff --git a/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java b/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java index 142ab38e0d0..25ae6b3d447 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java +++ b/lucene/test-framework/src/java/org/apache/lucene/store/MockDirectoryWrapper.java @@ -622,7 +622,8 @@ public class MockDirectoryWrapper extends BaseDirectoryWrapper { return size; } - private boolean assertNoUnreferencedFilesOnClose = true; + // NOTE: This is off by default; see LUCENE-5574 + private boolean assertNoUnreferencedFilesOnClose; public void setAssertNoUnrefencedFilesOnClose(boolean v) { assertNoUnreferencedFilesOnClose = v;