[SPARK-24297][CORE] Fetch-to-disk by default for > 2gb #21474

squito · 2018-05-31T21:37:13Z

Fetch-to-mem is guaranteed to fail if the message is bigger than 2 GB,
so we might as well use fetch-to-disk in that case. The message includes
some metadata in addition to the block data itself (in particular
UploadBlock has a lot of metadata), so we leave a little room.

Fetch-to-mem is guaranteed to fail if the message is bigger than 2 GB, so we might as well use fetch-to-disk in that case. The message includes some metadata in addition to the block data itself (in particular UploadBlock has a lot of metadata), so we leave a little room.

SparkQA · 2018-06-01T02:06:48Z

Test build #91365 has finished for PR 21474 at commit 6b7c4f7.

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

felixcheung · 2018-06-01T04:22:00Z

core/src/main/scala/org/apache/spark/internal/config/package.scala

+      // as well use fetch-to-disk in that case.  The message includes some metadata in addition
+      // to the block data itself (in particular UploadBlock has a lot of metadata), so we leave
+      // extra room.
+      .createWithDefault(Int.MaxValue - 500)


is the fixed buffer of 500 bytes enough for metadata?

no guarantee its big enough. Seemed OK in the test I tried. But UploadBlock has some variable length strings so can't say for sure.

I'm fine making this much bigger, eg. 1 MB -- you'd only be bigger than that with a pathological case. then there would be some cases where we'd be taking an old message which was fine with fetch-to-mem and we'd switch to fetch-to-disk. But such a tiny case, and not an unreasonable change even for that ... so should be OK.

right, it's the default and the user can change the value to leave more margin.

Actually I prefer 512 to 500 :)

sounds good, updated

felixcheung · 2018-06-02T06:23:09Z

core/src/main/scala/org/apache/spark/internal/config/package.scala

+      // as well use fetch-to-disk in that case.  The message includes some metadata in addition
+      // to the block data itself (in particular UploadBlock has a lot of metadata), so we leave
+      // extra room.
+      .createWithDefault(Int.MaxValue - 500)


right, it's the default and the user can change the value to leave more margin.

SparkQA · 2018-06-28T20:08:47Z

Test build #92428 has finished for PR 21474 at commit 61e1991.

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

HyukjinKwon · 2018-07-16T02:46:12Z

retest this please

HyukjinKwon

LGTM

SparkQA · 2018-07-16T07:05:02Z

Test build #93066 has finished for PR 21474 at commit 61e1991.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2018-07-16T07:16:06Z

retest this please

jiangxb1987 · 2018-07-16T08:47:20Z

LGTM

SparkQA · 2018-07-16T11:21:25Z

Test build #93093 has finished for PR 21474 at commit 61e1991.

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

HyukjinKwon · 2018-07-20T03:29:24Z

@jerryshao, busy? I think you better take a look before merging this in if you are available.

jerryshao · 2018-07-20T03:31:48Z

I will take a look at this sometime day, but don't block on me if it is urgent.

jerryshao · 2018-07-20T12:02:25Z

core/src/main/scala/org/apache/spark/internal/config/package.scala

        "external shuffle service, this feature can only be worked when external shuffle" +
        "service is newer than Spark 2.2.")
      .bytesConf(ByteUnit.BYTE)
-      .createWithDefault(Long.MaxValue)


I think the original purpose to set to Long.MaxValue is to avoid using this configuration by default, user should set to a proper size to enable this feature. But anyway I think the current change is also fine.

HyukjinKwon · 2018-07-23T02:04:35Z

Still LGTM but let me leave it to you @squito and @jerryshao .

HyukjinKwon · 2018-07-23T02:04:43Z

retest this please

SparkQA · 2018-07-23T06:00:46Z

Test build #93423 has finished for PR 21474 at commit 61e1991.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2018-07-23T06:13:04Z

retest this please

jerryshao · 2018-07-24T01:36:14Z

Hi @squito , would you please also update the changes in the doc, thanks!

squito · 2018-07-24T14:52:31Z

good point @jerryshao , I've updated the docs now, please take a look, thanks

SparkQA · 2018-07-24T19:14:15Z

Test build #93501 has finished for PR 21474 at commit 0ead6a1.

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

jerryshao

LGTM, merging to master.

tgravescs · 2018-07-26T14:03:58Z

just to clarify here, so the failure case if you are using a shuffle server < 2.2, does it throw an obvious error to the user?

squito · 2018-07-26T14:24:08Z

oh no, good point Tom. it'll fail with

18/07/26 07:15:02 WARN scheduler.TaskSetManager: Lost task 1.0 in stage 1.3 (TID 15, irashid-large-2.gce.cloudera.com, executor 2): FetchFailed(BlockManagerId(1
, irashid-large-3.gce.cloudera.com, 7337, None), shuffleId=0, mapId=1, reduceId=1, message=
org.apache.spark.shuffle.FetchFailedException: java.lang.UnsupportedOperationException
        at org.apache.spark.network.server.StreamManager.openStream(StreamManager.java:60)
        at org.apache.spark.network.server.TransportRequestHandler.processStreamRequest(TransportRequestHandler.java:136)
...

lemme open a jira to improve this.

squito · 2018-07-26T14:27:08Z

https://issues.apache.org/jira/browse/SPARK-24936

Fetch-to-mem is guaranteed to fail if the message is bigger than 2 GB, so we might as well use fetch-to-disk in that case. The message includes some metadata in addition to the block data itself (in particular UploadBlock has a lot of metadata), so we leave a little room. Author: Imran Rashid <[email protected]> Closes apache#21474 from squito/SPARK-24297.

felixcheung reviewed Jun 1, 2018

View reviewed changes

felixcheung approved these changes Jun 2, 2018

View reviewed changes

review feedback

61e1991

HyukjinKwon approved these changes Jul 16, 2018

View reviewed changes

jerryshao reviewed Jul 20, 2018

View reviewed changes

update docs

0ead6a1

jerryshao approved these changes Jul 25, 2018

View reviewed changes

asfgit closed this in 15fff79 Jul 25, 2018

[SPARK-24297][CORE] Fetch-to-disk by default for > 2gb #21474

[SPARK-24297][CORE] Fetch-to-disk by default for > 2gb #21474

Uh oh!

Conversation

squito commented May 31, 2018

Uh oh!

SparkQA commented Jun 1, 2018

Uh oh!

felixcheung Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

squito Jun 1, 2018

Choose a reason for hiding this comment

Uh oh!

felixcheung Jun 2, 2018

Choose a reason for hiding this comment

Uh oh!

jiangxb1987 Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

squito Jun 28, 2018

Choose a reason for hiding this comment

Uh oh!

felixcheung Jun 2, 2018

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 28, 2018

Uh oh!

HyukjinKwon commented Jul 16, 2018

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 16, 2018

Uh oh!

HyukjinKwon commented Jul 16, 2018

Uh oh!

jiangxb1987 commented Jul 16, 2018

Uh oh!

SparkQA commented Jul 16, 2018

Uh oh!

HyukjinKwon commented Jul 20, 2018

Uh oh!

jerryshao commented Jul 20, 2018

Uh oh!

jerryshao Jul 20, 2018

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon commented Jul 23, 2018

Uh oh!

HyukjinKwon commented Jul 23, 2018

Uh oh!

SparkQA commented Jul 23, 2018

Uh oh!

HyukjinKwon commented Jul 23, 2018

Uh oh!

jerryshao commented Jul 24, 2018

Uh oh!

squito commented Jul 24, 2018

Uh oh!

SparkQA commented Jul 24, 2018

Uh oh!

jerryshao left a comment

Choose a reason for hiding this comment

Uh oh!

tgravescs commented Jul 26, 2018

Uh oh!

squito commented Jul 26, 2018

Uh oh!

squito commented Jul 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants