[#1246] feat(tez): Support remote spill for unordered input. - #1250
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1250 +/- ##
============================================
+ Coverage 54.17% 55.17% +0.99%
- Complexity 2659 2669 +10
============================================
Files 396 377 -19
Lines 23013 20752 -2261
Branches 1971 1977 +6
============================================
- Hits 12467 11449 -1018
+ Misses 9777 8600 -1177
+ Partials 769 703 -66
... and 28 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| } | ||
|
|
||
| @Override | ||
| public void free() { |
There was a problem hiding this comment.
Do we need consider the issues of thread safe?
There was a problem hiding this comment.
We don’t have to think about thread safety, it's impossible to run in parallel.
When write operation is done, then call commit, then send to completed queue, then read or free.
Other implement for FetchInput also does not consider thread safety.
| } else { | ||
| LOG.info("Allocate DiskFetchedInput, length:{}", actualSize); | ||
| return new DiskFetchedInput( | ||
| actualSize + 8, |
There was a problem hiding this comment.
Could use a constant variable instead of 8?
There was a problem hiding this comment.
OK, I will do it.
| if (remoteSpillEnable) { | ||
| LOG.info("AllocateType RemoteFetchedInput, compressedSize:{}", compressedSize); | ||
| return new RemoteFetchedInput( | ||
| actualSize + 8, |
roryqi
left a comment
roryqi
left a comment
There was a problem hiding this comment.
LGTM, thanks @zhengchenyu , merged to master.
3 participants
What changes were proposed in this pull request?
When unordered input fetch data from shuffle server, if the fetched data is too large, will spill data to local filesystem.
This PR provide another way: spill data to remote filesystem.
Why are the changes needed?
Fix: #1246
Does this PR introduce any user-facing change?
No.
How was this patch tested?
unit test and tez example test on cluster.