samtools / htsjdk

A Java API for high-throughput sequencing data (HTS) formats.
http://samtools.github.io/htsjdk/
283 stars 242 forks source link

Make AbstractFastaSequenceFile serializable by Kryo for Spark. #1408

Closed tomwhite closed 5 years ago

tomwhite commented 5 years ago

See explanation at https://github.com/EsotericSoftware/kryo/issues/469

This change is a fix for https://github.com/broadinstitute/gatk/issues/6091, which is a regression in GATK running on Spark. I've tested the fix manually with GATK.

/cc @lbergelson

codecov-io commented 5 years ago

Codecov Report

Merging #1408 into master will decrease coverage by 0.015%. The diff coverage is 100%.

@@               Coverage Diff               @@
##              master     #1408       +/-   ##
===============================================
- Coverage     68.101%   68.086%   -0.015%     
+ Complexity      8373      8371        -2     
===============================================
  Files            573       573               
  Lines          33963     33963               
  Branches        5668      5668               
===============================================
- Hits           23129     23124        -5     
- Misses          8645      8648        +3     
- Partials        2189      2191        +2
Impacted Files Coverage Δ Complexity Δ
.../samtools/reference/AbstractFastaSequenceFile.java 57.5% <100%> (ø) 12 <2> (ø) :arrow_down:
...htsjdk/samtools/util/nio/DeleteOnExitPathHook.java 78.947% <0%> (-10.526%) 3% <0%> (-1%)
...samtools/util/AsyncBlockCompressedInputStream.java 72% <0%> (-4%) 12% <0%> (-1%)
lbergelson commented 5 years ago

@tomwhite Is this happening now because of the kryo update or because of htsjdk changes?

tomwhite commented 5 years ago

@lbergelson htsjdk certainly has a change in the last release that caused this. Whether going from Kryo 3 to 4 also made a difference, I'm not sure. (Thanks for merging.)