LUCENE-6204: Remove CompoundFileFormat.files()

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

lucene/CHANGES.txt

@@ -46,6 +46,10 @@ Optimizations
   instead of a separate merge sort for each segment.  In delete-heavy
   use cases this can be a sizable speedup. (Mike McCandless)
 
+API Changes
+
+* LUCENE-6204: Remove CompoundFileFormat.files(). (Robert Muir)
+
 Other
 
 * LUCENE-6193: Collapse identical catch branches in try-catch statements.

lucene/codecs/src/java/org/apache/lucene/codecs/simpletext/SimpleTextCompoundFormat.java

@@ -210,11 +210,6 @@ public class SimpleTextCompoundFormat extends CompoundFormat {
     }
   }
   
-  @Override
-  public String[] files(SegmentInfo si) {
-    return new String[] { IndexFileNames.segmentFileName(si.name, "", DATA_EXTENSION) };
-  }
-  
   // helper method to strip strip away 'prefix' from 'scratch' and return as String
   private String stripPrefix(BytesRefBuilder scratch, BytesRef prefix) throws IOException {
     return new String(scratch.bytes(), prefix.length, scratch.length() - prefix.length, StandardCharsets.UTF_8);

lucene/core/src/java/org/apache/lucene/codecs/CompoundFormat.java

@@ -47,11 +47,4 @@ public abstract class CompoundFormat {
    * Packs the provided files into a compound format.
    */
   public abstract void write(Directory dir, SegmentInfo si, Collection<String> files, IOContext context) throws IOException;
-
-  /**
-   * Returns the compound file names used by this segment.
-   */
-  // TODO: get this out of here, and use trackingdirwrapper. but this is really scary in IW right now...
-  // NOTE: generally si.useCompoundFile is not even yet 'set' when this is called.
-  public abstract String[] files(SegmentInfo si);
 }

lucene/core/src/java/org/apache/lucene/codecs/lucene50/Lucene50CompoundFormat.java

@@ -105,14 +105,6 @@ public final class Lucene50CompoundFormat extends CompoundFormat {
     }
   }
 
-  @Override
-  public String[] files(SegmentInfo si) {
-    return new String[] {
-      IndexFileNames.segmentFileName(si.name, "", DATA_EXTENSION),
-      IndexFileNames.segmentFileName(si.name, "", ENTRIES_EXTENSION)
-    };
-  }
-
   /** Extension of compound file */
   static final String DATA_EXTENSION = "cfs";
   /** Extension of compound file entries */

lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java

@@ -485,7 +485,10 @@ class DocumentsWriterPerThread {
     try {
       
       if (indexWriterConfig.getUseCompoundFile()) {
-        filesToDelete.addAll(IndexWriter.createCompoundFile(infoStream, directory, newSegment.info, context));
+        Set<String> originalFiles = newSegment.info.files();
+        // TODO: like addIndexes, we are relying on createCompoundFile to successfully cleanup...
+        IndexWriter.createCompoundFile(infoStream, new TrackingDirectoryWrapper(directory), newSegment.info, context);
+        filesToDelete.addAll(originalFiles);
         newSegment.info.setUseCompoundFile(true);
       }
 

lucene/core/src/java/org/apache/lucene/index/IndexWriter.java

@@ -2580,8 +2580,11 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
       // Now create the compound file if needed
       if (useCompoundFile) {
         Collection<String> filesToDelete = infoPerCommit.files();
+        TrackingDirectoryWrapper trackingCFSDir = new TrackingDirectoryWrapper(mergeDirectory);
+        // TODO: unlike merge, on exception we arent sniping any trash cfs files here?
+        // createCompoundFile tries to cleanup, but it might not always be able to...
         try {
-          createCompoundFile(infoStream, mergeDirectory, info, context);
+          createCompoundFile(infoStream, trackingCFSDir, info, context);
         } finally {
           // delete new non cfs files directly: they were never
           // registered with IFD
@@ -4030,11 +4033,10 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
       if (useCompoundFile) {
         success = false;
 
-        String cfsFiles[] = merge.info.info.getCodec().compoundFormat().files(merge.info.info);
         Collection<String> filesToRemove = merge.info.files();
-
+        TrackingDirectoryWrapper trackingCFSDir = new TrackingDirectoryWrapper(mergeDirectory);
         try {
-          filesToRemove = createCompoundFile(infoStream, mergeDirectory, merge.info.info, context);
+          createCompoundFile(infoStream, trackingCFSDir, merge.info.info, context);
           success = true;
         } catch (IOException ioe) {
           synchronized(this) {
@@ -4055,6 +4057,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
             }
 
             synchronized(this) {
+              Set<String> cfsFiles = new HashSet<>(trackingCFSDir.getCreatedFiles());
               for (String cfsFile : cfsFiles) {
                 deleter.deleteFile(cfsFile);
               }
@@ -4078,6 +4081,7 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
             if (infoStream.isEnabled("IW")) {
               infoStream.message("IW", "abort merge after building CFS");
             }
+            Set<String> cfsFiles = new HashSet<>(trackingCFSDir.getCreatedFiles());
             for (String cfsFile : cfsFiles) {
               deleter.deleteFile(cfsFile);
             }
@@ -4528,11 +4532,13 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
    * deletion files, this SegmentInfo must not reference such files when this
    * method is called, because they are not allowed within a compound file.
    */
-  static final Collection<String> createCompoundFile(InfoStream infoStream, Directory directory, final SegmentInfo info, IOContext context)
+  static final void createCompoundFile(InfoStream infoStream, TrackingDirectoryWrapper directory, final SegmentInfo info, IOContext context)
           throws IOException {
 
-    // TODO: use trackingdirectorywrapper instead of files() to know which files to delete when things fail:
+    // maybe this check is not needed, but why take the risk?
-    String cfsFiles[] = info.getCodec().compoundFormat().files(info);
+    if (!directory.getCreatedFiles().isEmpty()) {
+      throw new IllegalStateException("pass a clean trackingdir for CFS creation");
+    }
     
     if (infoStream.isEnabled("IW")) {
       infoStream.message("IW", "create compound file");
@@ -4546,18 +4552,16 @@ public class IndexWriter implements Closeable, TwoPhaseCommit, Accountable {
       success = true;
     } finally {
       if (!success) {
-        IOUtils.deleteFilesIgnoringExceptions(directory, cfsFiles);
+        Set<String> cfsFiles = new HashSet<>(directory.getCreatedFiles());
+        for (String file : cfsFiles) {
+          IOUtils.deleteFilesIgnoringExceptions(directory, file);
+        }
       }
     }
 
     // Replace all previous files with the CFS/CFE files:
-    Set<String> siFiles = new HashSet<>();
+    Set<String> siFiles = new HashSet<>(directory.getCreatedFiles());
-    for (String cfsFile : cfsFiles) {
-      siFiles.add(cfsFile);
-    }
     info.setFiles(siFiles);
-
-    return files;
   }
   
   /**

lucene/core/src/test/org/apache/lucene/index/TestDoc.java

@@ -232,7 +232,8 @@ public class TestDoc extends LuceneTestCase {
     si.setFiles(new HashSet<>(trackingDir.getCreatedFiles()));
       
     if (useCompoundFile) {
-      Collection<String> filesToDelete = IndexWriter.createCompoundFile(InfoStream.getDefault(), dir, si, newIOContext(random()));
+      Collection<String> filesToDelete = si.files();
+      IndexWriter.createCompoundFile(InfoStream.getDefault(), new TrackingDirectoryWrapper(dir), si, newIOContext(random()));
       si.setUseCompoundFile(true);
       for (final String fileToDelete : filesToDelete) {
         si1.info.dir.deleteFile(fileToDelete);

lucene/core/src/test/org/apache/lucene/index/TestIndexFileDeleter.java

@@ -22,6 +22,7 @@ import java.util.*;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.apache.lucene.analysis.MockAnalyzer;
+import org.apache.lucene.codecs.simpletext.SimpleTextCodec;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.index.IndexWriterConfig.OpenMode;
@@ -115,7 +116,7 @@ public class TestIndexFileDeleter extends LuceneTestCase {
     // non-existent segment:
     copyFile(dir, "_0_1" + ext, "_188_1" + ext);
 
-    String cfsFiles0[] = si0.getCodec().compoundFormat().files(si0);
+    String cfsFiles0[] = si0.getCodec() instanceof SimpleTextCodec ? new String[] { "_0.scf" } : new String[] { "_0.cfs", "_0.cfe" };
     
     // Create a bogus segment file:
     copyFile(dir, cfsFiles0[0], "_188.cfs");
@@ -128,12 +129,12 @@ public class TestIndexFileDeleter extends LuceneTestCase {
     // TODO: assert is bogus (relies upon codec-specific filenames)
     assertTrue(slowFileExists(dir, "_3.fdt") || slowFileExists(dir, "_3.fld"));
     
-    String cfsFiles3[] = si3.getCodec().compoundFormat().files(si3);
+    String cfsFiles3[] = si3.getCodec() instanceof SimpleTextCodec ? new String[] { "_3.scf" } : new String[] { "_3.cfs", "_3.cfe" };
     for (String f : cfsFiles3) {
       assertTrue(!slowFileExists(dir, f));
     }
     
-    String cfsFiles1[] = si1.getCodec().compoundFormat().files(si1);
+    String cfsFiles1[] = si1.getCodec() instanceof SimpleTextCodec ? new String[] { "_1.scf" } : new String[] { "_1.cfs", "_1.cfe" };
     copyFile(dir, cfsFiles1[0], "_3.cfs");
     
     String[] filesPre = dir.listAll();

lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java

@@ -23,6 +23,7 @@ import java.io.StringReader;
 import java.nio.file.NoSuchFileException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Random;
@@ -1199,7 +1200,7 @@ public class TestIndexWriterExceptions extends LuceneTestCase {
       dir.close();
   }
 
-  // Simulate a corrupt index by removing one of the cfs
+  // Simulate a corrupt index by removing one of the
   // files and make sure we get an IOException trying to
   // open the index:
   public void testSimulatedCorruptIndex2() throws IOException {
@@ -1237,8 +1238,9 @@ public class TestIndexWriterExceptions extends LuceneTestCase {
     SegmentInfos sis = SegmentInfos.readLatestCommit(dir);
     for (SegmentCommitInfo si : sis) {
       assertTrue(si.info.getUseCompoundFile());
-      String cfsFiles[] = si.info.getCodec().compoundFormat().files(si.info);
+      List<String> victims = new ArrayList<String>(si.info.files());
-      dir.deleteFile(cfsFiles[0]);
+      Collections.shuffle(victims, random());
+      dir.deleteFile(victims.get(0));
       corrupted = true;
       break;
     }

lucene/test-framework/src/java/org/apache/lucene/codecs/cranky/CrankyCompoundFormat.java

@@ -47,9 +47,4 @@ class CrankyCompoundFormat extends CompoundFormat {
     }
     delegate.write(dir, si, files, context);
   }
-  
-  @Override
-  public String[] files(SegmentInfo si) {
-    return delegate.files(si);
-  }
 }