changed to look specifically for part files rather than listing the files and searching through them #505
Conversation
|
Is it faster? What if another compression technique is being used, i.e. 7Zip, then we would have |
Codecov Report
@@ Coverage Diff @@
## master #505 +/- ##
==========================================
- Coverage 86.74% 86.03% -0.71%
==========================================
Files 347 347
Lines 11859 11860 +1
Branches 388 607 +219
==========================================
- Hits 10287 10204 -83
- Misses 1572 1656 +84
Continue to review full report at Codecov.
|
| } | ||
| val finalPath = partPath.getOrElse(path) | ||
|
|
||
| val partFile = new Path(pathString, "part-00000") |
There was a problem hiding this comment.
better configure the part file name on the write call path so we don't have to guess.
There was a problem hiding this comment.
it will always be this but the compression codec will determine the extension
|
|
||
| val partFile = new Path(pathString, "part-00000") | ||
| val partZipped = new Path(pathString, "part-00000.gz") | ||
| val finalPath = if (fs.exists(partZipped)) partZipped else if (fs.exists(partFile)) partFile else path |
There was a problem hiding this comment.
exists add redundant filesystem calls that will be even more expensive with S3
we can just call open directly and catch FileNotFound to try another filename
There was a problem hiding this comment.
@gerashegalov fixed to just try loading
|
@gerashegalov suggested this for faster reading |
@tovbinm it should be faster on object stores since you have fast path lookups but slow entry list performance. |
|
@tovbinm this covers no compression and our default settings as well as passing in the exact full path. Do you think we need to cover more possibilities? |
…rifAI into lm/loadSpecificFiles
| } | ||
| } | ||
| } match { | ||
| case Failure(e) => throw new RuntimeException(s"Failed to load workflow because of $e") |
There was a problem hiding this comment.
Let us keep e as the cause exception as well, i.e. provided as the second parameter to the RTE constructor.
I don't think we need to make it that complex. At most I would make Compression Codec configurable instead of doing any guesses at all. |
Related issues
Refer to issue(s) addressed in this pull request from Issues page.
Describe the proposed solution
A clear and concise description of what the changes are.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context about the changes here.