LUCENE-1184: allow SnapshotDeletionPolicy to be re-used across close/open of IndexWriter

git-svn-id: https://svn.apache.org/repos/asf/lucene/java/trunk@631223 13f79535-47bb-0310-9956-ffa450edef68

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

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);

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();
       }
     }

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 {
-
+      copyFiles(dir, dp, dp.snapshot());
-      // 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);
-      }
-
     } 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 {