diff --git a/CHANGES.txt b/CHANGES.txt index e43f898aa76..bff812e363b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -101,6 +101,10 @@ New features huge index might not stop within the specified time. (Sean Timm via Doron Cohen) + 8. LUCENE-1184: Allow SnapshotDeletionPolicy to be re-used across + close/re-open of IndexWriter while still protecting an open + snapshot (Tim Brennan via Mike McCandless) + Optimizations 1. LUCENE-705: When building a compound file, use diff --git a/src/java/org/apache/lucene/index/IndexFileDeleter.java b/src/java/org/apache/lucene/index/IndexFileDeleter.java index 71dd3facd70..494cb2b63b8 100644 --- a/src/java/org/apache/lucene/index/IndexFileDeleter.java +++ b/src/java/org/apache/lucene/index/IndexFileDeleter.java @@ -187,7 +187,7 @@ final class IndexFileDeleter { sis = null; } if (sis != null) { - CommitPoint commitPoint = new CommitPoint(sis); + CommitPoint commitPoint = new CommitPoint(commitsToDelete, directory, sis); if (sis.getGeneration() == segmentInfos.getGeneration()) { currentCommitPoint = commitPoint; } @@ -215,7 +215,7 @@ final class IndexFileDeleter { } if (infoStream != null) message("forced open of current segments file " + segmentInfos.getCurrentSegmentFileName()); - currentCommitPoint = new CommitPoint(sis); + currentCommitPoint = new CommitPoint(commitsToDelete, directory, sis); commits.add(currentCommitPoint); incRef(sis, true); } @@ -392,7 +392,7 @@ final class IndexFileDeleter { if (isCommit) { // Append to our commits list: - commits.add(new CommitPoint(segmentInfos)); + commits.add(new CommitPoint(commitsToDelete, directory, segmentInfos)); // Tell policy so it can remove commits: policy.onCommit(commits); @@ -569,14 +569,18 @@ final class IndexFileDeleter { * equals. */ - final private class CommitPoint implements Comparable, IndexCommitPoint { + final private static class CommitPoint implements Comparable, IndexCommitPoint { long gen; List files; String segmentsFileName; boolean deleted; + Directory directory; + Collection commitsToDelete; - public CommitPoint(SegmentInfos segmentInfos) throws IOException { + public CommitPoint(Collection commitsToDelete, Directory directory, SegmentInfos segmentInfos) throws IOException { + this.directory = directory; + this.commitsToDelete = commitsToDelete; segmentsFileName = segmentInfos.getCurrentSegmentFileName(); int size = segmentInfos.size(); files = new ArrayList(size); diff --git a/src/java/org/apache/lucene/index/SnapshotDeletionPolicy.java b/src/java/org/apache/lucene/index/SnapshotDeletionPolicy.java index b5ac5aeb169..b56da4757e6 100644 --- a/src/java/org/apache/lucene/index/SnapshotDeletionPolicy.java +++ b/src/java/org/apache/lucene/index/SnapshotDeletionPolicy.java @@ -31,13 +31,17 @@ import java.io.IOException; * we wrap another arbitrary {@link IndexDeletionPolicy}, this * gives you the freedom to continue using whatever {@link * IndexDeletionPolicy} you would normally want to use with your - * index. */ + * index. Note that you can re-use a single instance of + * SnapshotDeletionPolicy across multiple writers as long + * as they are against the same index Directory. Any + * snapshot held when a writer is closed will "survive" + * when the next writer is opened. */ public class SnapshotDeletionPolicy implements IndexDeletionPolicy { private IndexCommitPoint lastCommit; private IndexDeletionPolicy primary; - private IndexCommitPoint snapshot; + private String snapshot; public SnapshotDeletionPolicy(IndexDeletionPolicy primary) { this.primary = primary; @@ -64,10 +68,10 @@ public class SnapshotDeletionPolicy implements IndexDeletionPolicy { * you release the snapshot. */ public synchronized IndexCommitPoint snapshot() { if (snapshot == null) - snapshot = lastCommit; + snapshot = lastCommit.getSegmentsFileName(); else throw new IllegalStateException("snapshot is already set; please call release() first"); - return snapshot; + return lastCommit; } /** Release the currently held snapshot. */ @@ -93,7 +97,7 @@ public class SnapshotDeletionPolicy implements IndexDeletionPolicy { synchronized(SnapshotDeletionPolicy.this) { // Suppress the delete request if this commit point is // our current snapshot. - if (snapshot != cp) + if (snapshot == null || !snapshot.equals(getSegmentsFileName())) cp.delete(); } } diff --git a/src/test/org/apache/lucene/TestSnapshotDeletionPolicy.java b/src/test/org/apache/lucene/TestSnapshotDeletionPolicy.java index 48ed437cb1c..44c80127a34 100644 --- a/src/test/org/apache/lucene/TestSnapshotDeletionPolicy.java +++ b/src/test/org/apache/lucene/TestSnapshotDeletionPolicy.java @@ -63,6 +63,44 @@ public class TestSnapshotDeletionPolicy extends LuceneTestCase runTest(dir2); } + public void testReuseAcrossWriters() throws IOException { + Directory dir = new MockRAMDirectory(); + + SnapshotDeletionPolicy dp = new SnapshotDeletionPolicy(new KeepOnlyLastCommitDeletionPolicy()); + IndexWriter writer = new IndexWriter(dir, true, new StandardAnalyzer(), dp, + IndexWriter.MaxFieldLength.LIMITED); + // Force frequent commits + writer.setMaxBufferedDocs(2); + Document doc = new Document(); + doc.add(new Field("content", "aaa", Field.Store.YES, Field.Index.TOKENIZED, Field.TermVector.WITH_POSITIONS_OFFSETS)); + for(int i=0;i<7;i++) + writer.addDocument(doc); + IndexCommitPoint cp = dp.snapshot(); + copyFiles(dir, dp, cp); + writer.close(); + copyFiles(dir, dp, cp); + + writer = new IndexWriter(dir, true, new StandardAnalyzer(), dp, + IndexWriter.MaxFieldLength.LIMITED); + copyFiles(dir, dp, cp); + for(int i=0;i<7;i++) + writer.addDocument(doc); + copyFiles(dir, dp, cp); + writer.close(); + copyFiles(dir, dp, cp); + dp.release(); + writer = new IndexWriter(dir, true, new StandardAnalyzer(), dp, + IndexWriter.MaxFieldLength.LIMITED); + writer.close(); + try { + copyFiles(dir, dp, cp); + fail("did not hit expected IOException"); + } catch (IOException ioe) { + // expected + } + dir.close(); + } + private void runTest(Directory dir) throws IOException { // Run for ~7 seconds final long stopTime = System.currentTimeMillis() + 7000; @@ -82,9 +120,9 @@ public class TestSnapshotDeletionPolicy extends LuceneTestCase for(int i=0;i<27;i++) { try { writer.addDocument(doc); - } catch (IOException cie) { + } catch (Throwable t) { RuntimeException re = new RuntimeException("addDocument failed"); - re.initCause(cie); + re.initCause(t); throw re; } } @@ -137,27 +175,9 @@ public class TestSnapshotDeletionPolicy extends LuceneTestCase * just to test that the files indeed exist and are * readable even while the index is changing. */ public void backupIndex(Directory dir, SnapshotDeletionPolicy dp) throws IOException { - // To backup an index we first take a snapshot: - IndexCommitPoint cp = dp.snapshot(); try { - - // While we hold the snapshot, and nomatter how long - // we take to do the backup, the IndexWriter will - // never delete the files in the snapshot: - Collection files = cp.getFileNames(); - Iterator it = files.iterator(); - while(it.hasNext()) { - final String fileName = (String) it.next(); - // NOTE: in a real backup you would not use - // readFile; you would need to use something else - // that copies the file to a backup location. This - // could even be a spawned shell process (eg "tar", - // "zip") that takes the list of files and builds a - // backup. - readFile(dir, fileName); - } - + copyFiles(dir, dp, dp.snapshot()); } finally { // Make sure to release the snapshot, otherwise these // files will never be deleted during this IndexWriter @@ -166,6 +186,25 @@ public class TestSnapshotDeletionPolicy extends LuceneTestCase } } + private void copyFiles(Directory dir, SnapshotDeletionPolicy dp, IndexCommitPoint cp) throws IOException { + + // While we hold the snapshot, and nomatter how long + // we take to do the backup, the IndexWriter will + // never delete the files in the snapshot: + Collection files = cp.getFileNames(); + Iterator it = files.iterator(); + while(it.hasNext()) { + final String fileName = (String) it.next(); + // NOTE: in a real backup you would not use + // readFile; you would need to use something else + // that copies the file to a backup location. This + // could even be a spawned shell process (eg "tar", + // "zip") that takes the list of files and builds a + // backup. + readFile(dir, fileName); + } + } + byte[] buffer = new byte[4096]; private void readFile(Directory dir, String name) throws IOException {