updating the Tribble track loading code to use the new shared locks, updated lots of new tests, add infrastructure for the TreeInterval, and removed the old locking class.

git-svn-id: file:///humgen/gsa-scr1/gsa-engineering/svn_contents/trunk@3837 348d0f76-0448-11de-a6fe-93d51630548a

java/src/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilder.java

@@ -31,16 +31,15 @@ import org.broad.tribble.*;
 import org.broad.tribble.index.Index;
 import org.broad.tribble.index.IndexFactory;
 import org.broad.tribble.index.linear.LinearIndex;
-import org.broad.tribble.index.linear.LinearIndexCreator;
 import org.broad.tribble.source.BasicFeatureSource;
 import org.broad.tribble.vcf.NameAwareCodec;
 import org.broadinstitute.sting.gatk.refdata.tracks.TribbleTrack;
 import org.broadinstitute.sting.gatk.refdata.tracks.RMDTrack;
 import org.broadinstitute.sting.gatk.refdata.tracks.RMDTrackCreationException;
 import org.broadinstitute.sting.utils.collections.Pair;
-import org.broadinstitute.sting.utils.file.FSLock;
 import org.broadinstitute.sting.utils.classloader.PluginManager;
 import org.broadinstitute.sting.utils.StingException;
+import org.broadinstitute.sting.utils.file.FSLockWithShared;
 
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -178,65 +177,108 @@ public class TribbleRMDTrackBuilder extends PluginManager<FeatureCodec> implemen
 
         // create the index file name, locking on the index file name
         File indexFile = new File(inputFile.getAbsoluteFile() + linearIndexExtension);
-        FSLock lock = new FSLock(indexFile);
+        FSLockWithShared lock = new FSLockWithShared(indexFile);
 
         // acquire a lock on the file
-        boolean obtainedLock = lock.lock();
+        Index idx = null;
-        try {
+        if (indexFile.canRead())
-            // check to see if the index file is out of date
+            idx = attemptIndexFromDisk(inputFile, codec, indexFile, lock);
-            if (indexFile.exists() && indexFile.canRead() && obtainedLock && indexFile.lastModified() < inputFile.lastModified()) {
-                logger.warn("Tribble index file " + indexFile + " is older than the track file " + inputFile + ", deleting and regenerating");
-                indexFile.delete();
-            }
-            // if the file exists, and we can read it, load the index from disk (i.e. wasn't deleted in the last step).
-            if (indexFile.exists() && indexFile.canRead() && obtainedLock) {
-                logger.info("Loading Tribble index from disk for file " + inputFile);
-                Index index = IndexFactory.loadIndex(indexFile.getAbsolutePath());
-                if (index.isCurrentVersion())
-                    return index;
 
-                logger.warn("Index file " + indexFile + " is out of date (old version), deleting and updating the index file");
+        // if we managed to make an index, return
-                indexFile.delete();
+        if (idx != null) return idx;
-            }
+
-            return writeIndexToDisk(inputFile, codec, onDisk, indexFile, obtainedLock);
+        // we couldn't read the file, or we fell out of the conditions above, continue on to making a new index
+        return createNewIndex(inputFile, codec, onDisk, indexFile, lock);
+    }
+
+    /**
+     * attempt to read the index from disk
+     * @param inputFile the input file
+     * @param codec the codec to read from
+     * @param indexFile the index file itself
+     * @param lock the lock file
+     * @return an index, or null if we couldn't load one
+     * @throws IOException if we fail for FS issues
+     */
+    protected static Index attemptIndexFromDisk(File inputFile, FeatureCodec codec, File indexFile, FSLockWithShared lock) throws IOException {
+        boolean locked = lock.sharedLock();
+        Index idx;
+        try {
+            if (!locked) // can't lock file
+                idx = createIndexInMemory(inputFile, codec);
+            else
+                idx = loadFromDisk(inputFile, indexFile);
+        } finally {
+            if (locked) lock.unlock();
         }
-        finally {
+        return idx;
-            lock.unlock();
+    }
+
+    /**
+     * load the index from disk, checking for out of date indexes and old versions (both of which are deleted)
+     * @param inputFile the input file
+     * @param indexFile the input file, plus the index extension
+     * @return an Index, or null if we're unable to load
+     */
+    public static Index loadFromDisk(File inputFile, File indexFile) {
+        logger.info("Loading Tribble index from disk for file " + inputFile);
+        Index index = IndexFactory.loadIndex(indexFile.getAbsolutePath());
+
+        // check if the file is up-to date (filestamp and version check)
+        if (index.isCurrentVersion() && indexFile.lastModified() > inputFile.lastModified())
+            return index;
+        else if (indexFile.lastModified() < inputFile.lastModified())
+            logger.warn("Index file " + indexFile + " is out of date (index older than input file), deleting and updating the index file");
+        else // we've loaded an old version of the index, we want to remove it
+            logger.warn("Index file " + indexFile + " is out of date (old version), deleting and updating the index file");
+
+        // however we got here, remove the index and return null
+        boolean deleted = indexFile.delete();
+
+        if (!deleted) logger.warn("Index file " + indexFile + " is out of date, but could not be removed; it will not be trusted (we'll try to rebuild an in-memory copy)");
+        return null;
+    }
+
+
+    /**
+     * attempt to create the index, and write it to disk
+     * @param inputFile the input file
+     * @param codec the codec to use
+     * @param onDisk if they asked for disk storage or now
+     * @param indexFile the index file location
+     * @param lock the locking object
+     * @return the index object
+     * @throws IOException
+     */
+    private static Index createNewIndex(File inputFile, FeatureCodec codec, boolean onDisk, File indexFile, FSLockWithShared lock) throws IOException {
+        Index index = createIndexInMemory(inputFile, codec);
+
+        boolean locked = false; // could we exclusive lock the file?
+        try {
+            locked = lock.exclusiveLock();
+            if (locked) {
+                logger.info("Writing Tribble index to disk for file " + inputFile);
+                index.write(indexFile);
+            }
+            else // we can't write it to disk, just store it in memory, tell them this
+                if (onDisk) logger.info("Unable to write to " + indexFile + " for the index file, creating index in memory only");
+            return index;
+        } finally {
+            if (locked) lock.unlock();
         }
 
     }
 
     /**
-     * attempt to create the index, and to disk
+     * create the index in memory, given the input file and feature codec
      * @param inputFile the input file
-     * @param codec the codec to use
+     * @param codec the codec
-     * @param onDisk if they asked for disk storage or now
+     * @return a LinearIndex, given the file location
-     * @param indexFile the index file location
-     * @param obtainedLock did we obtain the lock on the file?
-     * @return the index object
      * @throws IOException
      */
-    private static LinearIndex writeIndexToDisk(File inputFile, FeatureCodec codec, boolean onDisk, File indexFile, boolean obtainedLock) throws IOException {
+    private static Index createIndexInMemory(File inputFile, FeatureCodec codec) throws IOException {
-        LinearIndexCreator create = new LinearIndexCreator(inputFile, codec);
-
         // this can take a while, let them know what we're doing
         logger.info("Creating Tribble index in memory for file " + inputFile);
-        LinearIndex index = (LinearIndex)create.createIndex(); // we don't want to write initially, so we pass in null
+        return new LinearIndex(16000,inputFile.getAbsolutePath());
-
-        // if the index doesn't exist, and we can write to the directory, and we got a lock: write to the disk
-        if (indexFile.getParentFile().canWrite() &&
-                (!indexFile.exists() || indexFile.canWrite()) &&
-                onDisk &&
-                obtainedLock) {
-            logger.info("Writing Tribble index to disk for file " + inputFile);
-            index.write(indexFile);
-            return index;
-        }
-        // we can't write it to disk, just store it in memory
-        else {
-            // if they wanted to write, let them know we couldn't
-            if (onDisk) logger.info("Unable to write to " + indexFile + " for the index file, creating index in memory only");
-            return index;
-        }
     }
 }

java/src/org/broadinstitute/sting/utils/file/FSLock.java

@@ -1,82 +0,0 @@
-package org.broadinstitute.sting.utils.file;
-
-import org.apache.log4j.Logger;
-import org.broadinstitute.sting.utils.StingException;
-
-import java.io.File;
-import java.io.FileNotFoundException;
-import java.io.IOException;
-import java.io.RandomAccessFile;
-import java.nio.channels.FileChannel;
-import java.nio.channels.FileLock;
-
-/**
- * a quick implementation of a file based lock, using the Java NIO classes
- */
-public class FSLock {
-    // connect to the logger
-    private static Logger logger = Logger.getLogger(FSLock.class);
-
-    // the lock string
-    private static final String lockString = ".lock";
-
-    // the file we're attempting to lock
-    private final File lockFile;
-
-    // the file lock
-    private FileLock lock = null;
-
-    // the file channel we open
-    FileChannel fc = null;
-
-    /**
-     * create a file system, given a base file to which a lock string gets appended.
-     * @param baseLocation the base file location
-     */
-    public FSLock(File baseLocation) {
-        lockFile = new File(baseLocation.getAbsoluteFile() + lockString);
-        if (lockFile != null) lockFile.deleteOnExit();
-    }
-
-    /**
-     * lock the file
-     *
-     * @return boolean true if we obtained a lock
-     */
-    public synchronized boolean lock() {
-        if (lock != null) throw new IllegalStateException("Unable to lock on file " + lockFile + " there is already a lock active");
-
-        // if the file already exists, or we can't write to the parent directory, return false
-        if (lockFile.exists() || !lockFile.getParentFile().canWrite())
-            return false;
-        try {
-            fc = new RandomAccessFile(lockFile,"rw").getChannel();
-            lock = fc.tryLock();
-            return lock != null && lock.isValid();
-        } catch (FileNotFoundException e) {
-            logger.info("Unable to create lock file (due to no file found exception), file = " + lockFile,e);
-            return false;
-        } catch (IOException e) {
-            logger.info("Unable to create lock file (due to IO exception), file = " + lockFile,e);
-            return false;
-        }
-    }
-
-    /**
-     * unlock the file
-     *
-     * note: this allows unlocking a file that failed to lock (no required user checks on null locks).
-     */
-    public void unlock() {
-         try {
-            if (lock != null)
-                lock.release();
-            if (fc != null)
-                fc.close();
-            if (lockFile.exists())
-                lockFile.delete();
-        } catch (IOException e) {
-            throw new StingException("Unable to create lock file named " + lockFile,e);
-        }
-    }
-}

java/test/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilderUnitTest.java

@@ -24,11 +24,12 @@
 package org.broadinstitute.sting.gatk.refdata.tracks.builders;
 
 
-import org.apache.log4j.AppenderSkeleton;
+import net.sf.picard.reference.IndexedFastaSequenceFile;
-import org.apache.log4j.Logger;
+import org.broad.tribble.index.Index;
-import org.apache.log4j.spi.LoggingEvent;
 import org.broad.tribble.vcf.VCFCodec;
 import org.broadinstitute.sting.BaseTest;
+import org.broadinstitute.sting.utils.GenomeLocParser;
+import org.broadinstitute.sting.utils.file.FSLockWithShared;
 import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
@@ -39,12 +40,11 @@ import java.util.Map;
 
 
 /**
- * 
+ * @author aaron
- * @author aaron 
+ *         <p/>
- * 
+ *         Class TribbleRMDTrackBuilderUnitTest
- * Class TribbleRMDTrackBuilderUnitTest
+ *         <p/>
- *
+ *         Testing out the builder for tribble Tracks
- * Testing out the builder for tribble Tracks
  */
 public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
     private TribbleRMDTrackBuilder builder;
@@ -52,37 +52,99 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
     @Before
     public void setup() {
         builder = new TribbleRMDTrackBuilder();
+        IndexedFastaSequenceFile seq = new IndexedFastaSequenceFile(new File(oneKGLocation + "reference/human_b36_both.fasta"));
+        GenomeLocParser.setupRefContigOrdering(seq);
     }
 
     @Test
     public void testBuilder() {
-        Map<String,Class> classes = builder.getAvailableTrackNamesAndTypes();
+        Map<String, Class> classes = builder.getAvailableTrackNamesAndTypes();
         Assert.assertTrue(classes.size() > 0);
     }
 
     //@Test
+    // in this test, the index exists, but is out of date.
     public void testBuilderIndexUnwriteable() {
-        File vcfFile = new File(validationDataLocation + "/ROD_validation/relic.vcf");
+        File vcfFile = new File(validationDataLocation + "/ROD_validation/read_only/relic.vcf");
         try {
-            builder.loadIndex(vcfFile,new VCFCodec(), true);
+            builder.loadIndex(vcfFile, new VCFCodec(), true);
         } catch (IOException e) {
             e.printStackTrace();
             Assert.fail("IO exception unexpected" + e.getMessage());
         }
         // make sure we didn't write the file (check that it's timestamp is within bounds)
         //System.err.println(new File(vcfFile + TribbleRMDTrackBuilder.linearIndexExtension).lastModified());
-        Assert.assertTrue(Math.abs(1275597793000l - new File(vcfFile + TribbleRMDTrackBuilder.linearIndexExtension).lastModified()) < 100);
+        Assert.assertTrue(Math.abs(1279591752000l - new File(vcfFile + TribbleRMDTrackBuilder.linearIndexExtension).lastModified()) < 100);
 
     }
 
+    // we have a good index file, in a read-only dir. This would cause the previous version to remake the index; make
+    // sure we don't do this
+    @Test
+    public void testDirIsLockedIndexFromDisk() {
+        File vcfFile = new File(validationDataLocation + "/ROD_validation/read_only/good_index.vcf");
+        File vcfFileIndex = new File(validationDataLocation + "/ROD_validation/read_only/good_index.vcf.idx");
+        Index ind = null;
+        try {
+            ind = builder.attemptIndexFromDisk(vcfFile,new VCFCodec(),vcfFileIndex,new FSLockWithShared(vcfFile));
+        } catch (IOException e) {
+            Assert.fail("We weren't expecting an exception -> " + e.getMessage());
+        }
+        Assert.assertTrue(ind != null);
+    }
+
+
+
+    @Test
+    public void testBuilderIndexDirectoryUnwritable() {
+        File vcfFile = new File(validationDataLocation + "/ROD_validation/read_only/no_index.vcf.vcf");
+        File vcfFileIndex = new File(validationDataLocation + "/ROD_validation/read_only/no_index.vcf.idx");
+
+        Index ind = null;
+        try {
+            ind = builder.loadIndex(vcfFile, new VCFCodec(), true);
+        } catch (IOException e) {
+            e.printStackTrace();
+            Assert.fail("IO exception unexpected" + e.getMessage());
+        }
+        // make sure we didn't write the file (check that it's timestamp is within bounds)
+        Assert.assertTrue(!vcfFileIndex.exists());
+        Assert.assertTrue(ind != null);
+
+    }
+
+
+    @Test
+    public void testGenerateIndexForUnindexedFile() {
+        File vcfFile = new File(validationDataLocation + "/ROD_validation/always_reindex.vcf");
+        File vcfFileIndex = new File(validationDataLocation + "/ROD_validation/always_reindex.vcf.idx");
+
+        // if we can't write to the directory, don't fault the tester, just pass
+        if (!vcfFileIndex.getParentFile().canWrite()) {
+            logger.warn("Unable to run test testGenerateIndexForUnindexedFile: unable to write to dir " + vcfFileIndex.getParentFile());
+            return;
+        }
+        // clean-up our test, and previous tests that may have written the file
+        vcfFileIndex.deleteOnExit();
+        if (vcfFileIndex.exists()) vcfFileIndex.delete();
+
+        try {
+            builder.loadIndex(vcfFile, new VCFCodec(), true);
+        } catch (IOException e) {
+            e.printStackTrace();
+            Assert.fail("IO exception unexpected" + e.getMessage());
+        }
+        // make sure we wrote the file
+        Assert.assertTrue(vcfFileIndex.exists());
+    }
+
 
     // test to make sure we delete the index and regenerate if it's out of date
     //@Test
     public void testBuilderIndexOutOfDate() {
-        Logger logger = Logger.getLogger(TribbleRMDTrackBuilder.class);
         File vcfFile = createOutofDateIndexFile(new File(validationDataLocation + "/ROD_validation/newerTribbleTrack.vcf"));
         try {
-            builder.loadIndex(vcfFile,new VCFCodec(), true);
+            builder.loadIndex(vcfFile, new VCFCodec(), true);
         } catch (IOException e) {
             e.printStackTrace();
             Assert.fail("IO exception unexpected" + e.getMessage());
@@ -95,13 +157,12 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
     }
 
     // test to make sure we delete the index and regenerate if it's out of date
-    //@Test
+    // @Test
     public void testBuilderIndexGoodDate() {
-        Logger logger = Logger.getLogger(TribbleRMDTrackBuilder.class);
         File vcfFile = createCorrectDateIndexFile(new File(validationDataLocation + "/ROD_validation/newerTribbleTrack.vcf"));
         Long indexTimeStamp = new File(vcfFile.getAbsolutePath() + ".idx").lastModified();
         try {
-            builder.loadIndex(vcfFile,new VCFCodec(), true);
+            builder.loadIndex(vcfFile, new VCFCodec(), true);
         } catch (IOException e) {
             e.printStackTrace();
             Assert.fail("IO exception unexpected" + e.getMessage());
@@ -115,14 +176,17 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
 
     /**
      * create a temporary file and an associated out of date index file
+     *
      * @param tribbleFile the tribble file
      * @return a file pointing to the new tmp location, with out of date index
      */
     private File createOutofDateIndexFile(File tribbleFile) {
         try {
             // first copy the tribble file to a temperary file
-            File tmpFile = File.createTempFile("TribbleUnitTestFile","");
+            File tmpFile = File.createTempFile("TribbleUnitTestFile", "");
-            logger.info("creating temp file " + tmpFile); 
+            tmpFile.deleteOnExit();
+
+            logger.info("creating temp file " + tmpFile);
             // create a fake index, before we copy so it's out of date
             File tmpIndex = new File(tmpFile.getAbsolutePath() + ".idx");
             tmpIndex.deleteOnExit();
@@ -131,7 +195,7 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
             Thread.sleep(2000);
 
             // copy the vcf (tribble) file to the tmp file location
-            copyFile(tribbleFile,tmpFile);
+            copyFile(tribbleFile, tmpFile);
 
             // sleep again, to make sure the timestamps are different (vcf vrs updated index file)
             Thread.sleep(2000);
@@ -148,17 +212,19 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
 
     /**
      * create a temporary file and an associated out of date index file
+     *
      * @param tribbleFile the tribble file
      * @return a file pointing to the new tmp location, with out of date index
      */
     private File createCorrectDateIndexFile(File tribbleFile) {
         try {
             // first copy the tribble file to a temperary file
-            File tmpFile = File.createTempFile("TribbleUnitTestFile","");
+            File tmpFile = File.createTempFile("TribbleUnitTestFile", "");
+            tmpFile.deleteOnExit();
             logger.info("creating temp file " + tmpFile);
 
             // copy the vcf (tribble) file to the tmp file location
-            copyFile(tribbleFile,tmpFile);
+            copyFile(tribbleFile, tmpFile);
 
             // sleep again, to make sure the timestamps are different (vcf vrs updated index file)
             Thread.sleep(2000);
@@ -168,7 +234,7 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
             tmpIndex.deleteOnExit();
 
             // copy the vcf (tribble) file to the tmp file location
-            copyFile(new File(tribbleFile + ".idx"),tmpIndex);
+            copyFile(new File(tribbleFile + ".idx"), tmpIndex);
 
             return tmpFile;
 
@@ -182,6 +248,7 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
 
     /**
      * copy a file, from http://www.exampledepot.com/egs/java.nio/File2File.html
+     *
      * @param srFile the source file
      * @param dtFile the destination file
      */
@@ -191,7 +258,7 @@ public class TribbleRMDTrackBuilderUnitTest extends BaseTest {
             FileChannel srcChannel = new FileInputStream(srFile).getChannel();
 
             // Create channel on the destination
-            FileChannel dstChannel = new FileOutputStream(dtFile).getChannel(); 
+            FileChannel dstChannel = new FileOutputStream(dtFile).getChannel();
 
             // Copy file contents from source to destination
             dstChannel.transferFrom(srcChannel, 0, srcChannel.size());