Skip to content

Conversation

@jerryshao
Copy link
Contributor

What changes were proposed in this pull request?

This PR is the second attempt of #18684 , NIO's Files API doesn't override skip method for InputStream, so it will bring in performance issue (mentioned in #20119). But using FileInputStream/FileOutputStream will also bring in memory issue (https://dzone.com/articles/fileinputstream-fileoutputstream-considered-harmful), which is severe for long running external shuffle service. So here in this proposal, only fixing the external shuffle service related code.

How was this patch tested?

Existing tests.

@jerryshao
Copy link
Contributor Author

@zsxwing , would you please take a review, thanks!

@SparkQA
Copy link

SparkQA commented Jan 4, 2018

Test build #85651 has finished for PR 20144 at commit 277b801.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zsxwing
Copy link
Member

zsxwing commented Jan 4, 2018

LGTM. Thanks! Merging to master and 2.3.

asfgit pushed a commit that referenced this pull request Jan 4, 2018
…ternal shuffle service

## What changes were proposed in this pull request?

This PR is the second attempt of #18684 , NIO's Files API doesn't override `skip` method for `InputStream`, so it will bring in performance issue (mentioned in #20119). But using `FileInputStream`/`FileOutputStream` will also bring in memory issue (https://dzone.com/articles/fileinputstream-fileoutputstream-considered-harmful), which is severe for long running external shuffle service. So here in this proposal, only fixing the external shuffle service related code.

## How was this patch tested?

Existing tests.

Author: jerryshao <[email protected]>

Closes #20144 from jerryshao/SPARK-21475-v2.

(cherry picked from commit 93f92c0)
Signed-off-by: Shixiong Zhu <[email protected]>
@asfgit asfgit closed this in 93f92c0 Jan 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants