Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 957545) +++ lucene/CHANGES.txt (working copy) @@ -393,6 +393,9 @@ throws an exception when term count exceeds doc count. (Mike McCandless, Uwe Schindler) +* LUCENE-2513: when opening writable IndexReader on a not-current + commit, do not overwrite "future" commits. (Mike McCandless) + New features * LUCENE-2128: Parallelized fetching document frequencies during weight Index: lucene/src/test/org/apache/lucene/index/TestIndexWriter.java =================================================================== --- lucene/src/test/org/apache/lucene/index/TestIndexWriter.java (revision 957545) +++ lucene/src/test/org/apache/lucene/index/TestIndexWriter.java (working copy) @@ -743,6 +743,7 @@ try { writer = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer()).setOpenMode(OpenMode.CREATE)); } catch (Exception e) { + e.printStackTrace(System.out); fail("writer failed to open on a crashed index"); } @@ -4978,4 +4979,80 @@ _TestUtil.rmDir(tempDir); } } + + public void testFutureCommit() throws Exception { + Directory dir = new MockRAMDirectory(); + + IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer()).setIndexDeletionPolicy(NoDeletionPolicy.INSTANCE)); + Document doc = new Document(); + w.addDocument(doc); + + // commit to "first" + Map commitData = new HashMap(); + commitData.put("tag", "first"); + w.commit(commitData); + + // commit to "second" + w.addDocument(doc); + commitData.put("tag", "second"); + w.commit(commitData); + w.close(); + + // open "first" with IndexWriter + IndexCommit commit = null; + for(IndexCommit c : IndexReader.listCommits(dir)) { + if (c.getUserData().get("tag").equals("first")) { + commit = c; + break; + } + } + + assertNotNull(commit); + + w = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer()).setIndexDeletionPolicy(NoDeletionPolicy.INSTANCE).setIndexCommit(commit)); + + assertEquals(1, w.numDocs()); + + // commit IndexWriter to "third" + w.addDocument(doc); + commitData.put("tag", "third"); + w.commit(commitData); + w.close(); + + // make sure "second" commit is still there + commit = null; + for(IndexCommit c : IndexReader.listCommits(dir)) { + if (c.getUserData().get("tag").equals("second")) { + commit = c; + break; + } + } + + assertNotNull(commit); + + IndexReader r = IndexReader.open(commit, true); + assertEquals(2, r.numDocs()); + r.close(); + + // open "second", w/ writeable IndexReader & commit + r = IndexReader.open(commit, NoDeletionPolicy.INSTANCE, false); + assertEquals(2, r.numDocs()); + r.deleteDocument(0); + r.deleteDocument(1); + commitData.put("tag", "fourth"); + r.commit(commitData); + r.close(); + + // make sure "third" commit is still there + commit = null; + for(IndexCommit c : IndexReader.listCommits(dir)) { + if (c.getUserData().get("tag").equals("third")) { + commit = c; + break; + } + } + assertNotNull(commit); + + dir.close(); + } } Index: lucene/src/java/org/apache/lucene/index/DirectoryReader.java =================================================================== --- lucene/src/java/org/apache/lucene/index/DirectoryReader.java (revision 957545) +++ lucene/src/java/org/apache/lucene/index/DirectoryReader.java (working copy) @@ -71,6 +71,11 @@ private int numDocs = -1; private boolean hasDeletions = false; + // Max version in index as of when we opened; this can be + // > our current segmentInfos version in case we were + // opened on a past IndexCommit: + private long maxIndexVersion; + // static IndexReader open(final Directory directory, final IndexDeletionPolicy deletionPolicy, final IndexCommit commit, final boolean readOnly, // final int termInfosIndexDivisor) throws CorruptIndexException, IOException { // return open(directory, deletionPolicy, commit, readOnly, termInfosIndexDivisor, null); @@ -359,6 +364,10 @@ } } starts[subReaders.length] = maxDoc; + + if (!readOnly) { + maxIndexVersion = SegmentInfos.readCurrentVersion(directory, codecs); + } } @Override @@ -743,7 +752,7 @@ // we have to check whether index has changed since this reader was opened. // if so, this reader is no longer valid for deletion - if (SegmentInfos.readCurrentVersion(directory, codecs) > segmentInfos.getVersion()) { + if (SegmentInfos.readCurrentVersion(directory, codecs) > maxIndexVersion) { stale = true; this.writeLock.release(); this.writeLock = null; @@ -775,6 +784,7 @@ IndexFileDeleter deleter = new IndexFileDeleter(directory, deletionPolicy == null ? new KeepOnlyLastCommitDeletionPolicy() : deletionPolicy, segmentInfos, null, null, codecs); + segmentInfos.updateGeneration(deleter.getLastSegmentInfos()); // Checkpoint the state we are about to change, in // case we have to roll back: @@ -813,6 +823,8 @@ deleter.checkpoint(segmentInfos, true); deleter.close(); + maxIndexVersion = segmentInfos.getVersion(); + if (writeLock != null) { writeLock.release(); // release write lock writeLock = null; Index: lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java =================================================================== --- lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java (revision 957545) +++ lucene/src/java/org/apache/lucene/index/IndexFileDeleter.java (working copy) @@ -102,6 +102,7 @@ private DocumentsWriter docWriter; final boolean startingCommitDeleted; + private SegmentInfos lastSegmentInfos; /** Change to true to see details of reference counts when * infoStream != null */ @@ -168,34 +169,44 @@ // This is a commit (segments or segments_N), and // it's valid (<= the max gen). Load it, then // incref all files it refers to: - if (SegmentInfos.generationFromSegmentsFileName(fileName) <= currentGen) { + if (infoStream != null) { + message("init: load commit \"" + fileName + "\""); + } + SegmentInfos sis = new SegmentInfos(); + try { + sis.read(directory, fileName, codecs); + } catch (FileNotFoundException e) { + // LUCENE-948: on NFS (and maybe others), if + // you have writers switching back and forth + // between machines, it's very likely that the + // dir listing will be stale and will claim a + // file segments_X exists when in fact it + // doesn't. So, we catch this and handle it + // as if the file does not exist if (infoStream != null) { - message("init: load commit \"" + fileName + "\""); + message("init: hit FileNotFoundException when loading commit \"" + fileName + "\"; skipping this commit point"); } - SegmentInfos sis = new SegmentInfos(); - try { - sis.read(directory, fileName, codecs); - } catch (FileNotFoundException e) { - // LUCENE-948: on NFS (and maybe others), if - // you have writers switching back and forth - // between machines, it's very likely that the - // dir listing will be stale and will claim a - // file segments_X exists when in fact it - // doesn't. So, we catch this and handle it - // as if the file does not exist - if (infoStream != null) { - message("init: hit FileNotFoundException when loading commit \"" + fileName + "\"; skipping this commit point"); - } - sis = null; + sis = null; + } catch (IOException e) { + if (SegmentInfos.generationFromSegmentsFileName(fileName) <= currentGen) { + throw e; + } else { + // Most likely we are opening an index that + // has an aborted "future" commit, so suppress + // exc in this case } - if (sis != null) { - CommitPoint commitPoint = new CommitPoint(commitsToDelete, directory, sis); - if (sis.getGeneration() == segmentInfos.getGeneration()) { - currentCommitPoint = commitPoint; - } - commits.add(commitPoint); - incRef(sis, true); + } + if (sis != null) { + CommitPoint commitPoint = new CommitPoint(commitsToDelete, directory, sis); + if (sis.getGeneration() == segmentInfos.getGeneration()) { + currentCommitPoint = commitPoint; } + commits.add(commitPoint); + incRef(sis, true); + + if (lastSegmentInfos == null || sis.getGeneration() > lastSegmentInfos.getGeneration()) { + lastSegmentInfos = sis; + } } } } @@ -254,6 +265,10 @@ deleteCommits(); } + public SegmentInfos getLastSegmentInfos() { + return lastSegmentInfos; + } + /** * Remove the CommitPoints in the commitsToDelete List by * DecRef'ing all files from each SegmentInfos.