You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by squito <gi...@git.apache.org> on 2018/10/12 02:18:31 UTC
[GitHub] spark pull request #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than...
GitHub user squito opened a pull request:
https://github.com/apache/spark/pull/22705
[SPARK-25704][CORE][WIP] Allocate a bit less than Int.MaxValue
JVMs don't you allocate arrays of length exactly Int.MaxValue, so leave
a little extra room. This is necessary when reading blocks >2GB off
the network (for remote reads or for cache replication).
WIP because I'm still running tests on a real cluster
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/squito/spark SPARK-25704
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22705.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #22705
----
commit cb07badcd853da0e4083b7e02bdfdf86c9d295f1
Author: Imran Rashid <ir...@...>
Date: 2018-10-12T01:54:34Z
[SPARK-25704][CORE] Allocate a bit less than Int.MaxValue
JVMs don't you allocate arrays of length exactly Int.MaxValue, so leave
a little extra room. This is necessary when reading blocks >2GB off
the network (for remote reads or for cache replication).
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4041/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/22705
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97465/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97465 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97465/testReport)** for PR 22705 at commit [`c811476`](https://github.com/apache/spark/commit/c811476410b4b6e5c29265ce60ec1c2a327209f4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97399/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/22705#discussion_r224977082
--- Diff: core/src/main/scala/org/apache/spark/util/io/ChunkedByteBuffer.scala ---
@@ -195,7 +196,11 @@ object ChunkedByteBuffer {
val is = new FileInputStream(file)
ByteStreams.skipFully(is, offset)
val in = new LimitedInputStream(is, length)
- val chunkSize = math.min(maxChunkSize, length).toInt
+ // Though in theory you should be able to index into an array of size Int.MaxValue, in practice
+ // jvms don't let you go up to limit. It seems you may only need - 2, but we leave a little
+ // extra room.
+ val maxArraySize = Int.MaxValue - 512
--- End diff --
We already had seen the similar problem in other places. Can we use [this value](https://github.com/apache/spark/blob/master/common/unsafe/src/main/java/org/apache/spark/unsafe/array/ByteArrayMethods.java#L52) here, too?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97399 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97399/testReport)** for PR 22705 at commit [`c811476`](https://github.com/apache/spark/commit/c811476410b4b6e5c29265ce60ec1c2a327209f4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/4013/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3909/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22705: [SPARK-25704][CORE] Allocate a bit less than Int....
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22705
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97423 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97423/testReport)** for PR 22705 at commit [`c811476`](https://github.com/apache/spark/commit/c811476410b4b6e5c29265ce60ec1c2a327209f4).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97290 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97290/testReport)** for PR 22705 at commit [`cb07bad`](https://github.com/apache/spark/commit/cb07badcd853da0e4083b7e02bdfdf86c9d295f1).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on a diff in the pull request:
https://github.com/apache/spark/pull/22705#discussion_r225208317
--- Diff: core/src/main/scala/org/apache/spark/util/io/ChunkedByteBuffer.scala ---
@@ -195,7 +196,11 @@ object ChunkedByteBuffer {
val is = new FileInputStream(file)
ByteStreams.skipFully(is, offset)
val in = new LimitedInputStream(is, length)
- val chunkSize = math.min(maxChunkSize, length).toInt
+ // Though in theory you should be able to index into an array of size Int.MaxValue, in practice
+ // jvms don't let you go up to limit. It seems you may only need - 2, but we leave a little
+ // extra room.
+ val maxArraySize = Int.MaxValue - 512
--- End diff --
great suggestion, thanks, just updated
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/22705
Jenkins, retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97423 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97423/testReport)** for PR 22705 at commit [`c811476`](https://github.com/apache/spark/commit/c811476410b4b6e5c29265ce60ec1c2a327209f4).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by kiszk <gi...@git.apache.org>.
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/22705
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97423/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97290/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/3991/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97290 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97290/testReport)** for PR 22705 at commit [`cb07bad`](https://github.com/apache/spark/commit/cb07badcd853da0e4083b7e02bdfdf86c9d295f1).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22705#discussion_r224965160
--- Diff: core/src/main/scala/org/apache/spark/util/io/ChunkedByteBuffer.scala ---
@@ -195,7 +196,11 @@ object ChunkedByteBuffer {
val is = new FileInputStream(file)
ByteStreams.skipFully(is, offset)
val in = new LimitedInputStream(is, length)
- val chunkSize = math.min(maxChunkSize, length).toInt
+ // Though in theory you should be able to index into an array of size Int.MaxValue, in practice
+ // jvms don't let you go up to limit. It seems you may only need - 2, but we leave a little
+ // extra room.
+ val maxArraySize = Int.MaxValue - 512
--- End diff --
Looks like the max is safely more like `Int.MaxValue - 5` from things like https://www.quora.com/What-is-the-maximum-size-of-the-array-in-Java You could probably push it further, but whatever.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97399 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97399/testReport)** for PR 22705 at commit [`c811476`](https://github.com/apache/spark/commit/c811476410b4b6e5c29265ce60ec1c2a327209f4).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22705
**[Test build #97465 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97465/testReport)** for PR 22705 at commit [`c811476`](https://github.com/apache/spark/commit/c811476410b4b6e5c29265ce60ec1c2a327209f4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE][WIP] Allocate a bit less than Int.Ma...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22705
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22705: [SPARK-25704][CORE] Allocate a bit less than Int.MaxValu...
Posted by squito <gi...@git.apache.org>.
Github user squito commented on the issue:
https://github.com/apache/spark/pull/22705
merged to master / 2.4.
Something went wrong w/ the merge script when putting it on 2.4 (I think I accidentally had a dirty working dir at the time), but it was actually a clean cherry pick, so I pushed it manually here:
https://github.com/apache/spark/commit/1001d2314275c902da519725da266a23b537e33a
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org