diff --git a/java/src/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilder.java b/java/src/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilder.java index 1ce2d8825..110078246 100644 --- a/java/src/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilder.java +++ b/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 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(); - try { - // check to see if the index file is out of date - 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; + Index idx = null; + if (indexFile.canRead()) + idx = attemptIndexFromDisk(inputFile, codec, indexFile, lock); - logger.warn("Index file " + indexFile + " is out of date (old version), deleting and updating the index file"); - indexFile.delete(); - } - return writeIndexToDisk(inputFile, codec, onDisk, indexFile, obtainedLock); + // if we managed to make an index, return + if (idx != null) return idx; + + // 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 { - lock.unlock(); + return idx; + } + + /** + * 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 onDisk if they asked for disk storage or now - * @param indexFile the index file location - * @param obtainedLock did we obtain the lock on the file? - * @return the index object + * @param codec the codec + * @return a LinearIndex, given the file location * @throws IOException */ - private static LinearIndex writeIndexToDisk(File inputFile, FeatureCodec codec, boolean onDisk, File indexFile, boolean obtainedLock) throws IOException { - LinearIndexCreator create = new LinearIndexCreator(inputFile, codec); - + private static Index createIndexInMemory(File inputFile, FeatureCodec codec) throws IOException { // 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 - - // 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; - } + return new LinearIndex(16000,inputFile.getAbsolutePath()); } } diff --git a/java/src/org/broadinstitute/sting/utils/file/FSLock.java b/java/src/org/broadinstitute/sting/utils/file/FSLock.java deleted file mode 100644 index e5be9624c..000000000 --- a/java/src/org/broadinstitute/sting/utils/file/FSLock.java +++ /dev/null @@ -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); - } - } -} diff --git a/java/test/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilderUnitTest.java b/java/test/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilderUnitTest.java index 158ac9602..3d1cb036c 100644 --- a/java/test/org/broadinstitute/sting/gatk/refdata/tracks/builders/TribbleRMDTrackBuilderUnitTest.java +++ b/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 org.apache.log4j.Logger; -import org.apache.log4j.spi.LoggingEvent; +import net.sf.picard.reference.IndexedFastaSequenceFile; +import org.broad.tribble.index.Index; 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 - * - * Class TribbleRMDTrackBuilderUnitTest - * - * Testing out the builder for tribble Tracks + * @author aaron + *

+ * Class TribbleRMDTrackBuilderUnitTest + *

+ * 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 classes = builder.getAvailableTrackNamesAndTypes(); + Map 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",""); - logger.info("creating temp file " + tmpFile); + File tmpFile = File.createTempFile("TribbleUnitTestFile", ""); + 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());