You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by 10110346 <gi...@git.apache.org> on 2018/10/15 07:08:13 UTC
[GitHub] spark pull request #22723: [SPARK-25729][CORE]It is better to replace `minPa...
GitHub user 10110346 opened a pull request:
https://github.com/apache/spark/pull/22723
[SPARK-25729][CORE]It is better to replace `minPartitions` with `defaultParallelism` , when `minPartitions` is less than `defaultParallelism`
## What changes were proposed in this pull request?
In ‘WholeTextFileRDD’，when `minPartitions` is less than `defaultParallelism`，
it is better to replace `minPartitions` with `defaultParallelism` , because this can make better use of resources and improve concurrency.
## How was this patch tested?
Added a unit test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/10110346/spark minPartNum
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22723.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 #22723
----
commit b80bf66a8109faa7f58d45b92417a981666866a0
Author: liuxian <li...@...>
Date: 2018-10-15T06:39:17Z
fix
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r230579084
--- Diff: core/src/main/scala/org/apache/spark/rdd/WholeTextFileRDD.scala ---
@@ -51,7 +51,7 @@ private[spark] class WholeTextFileRDD(
case _ =>
}
val jobContext = new JobContextImpl(conf, jobId)
- inputFormat.setMinPartitions(jobContext, minPartitions)
+ inputFormat.setMinPartitions(sc, jobContext, minPartitions)
--- End diff --
Yeah, thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22723
cc @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r229720799
--- Diff: core/src/main/scala/org/apache/spark/rdd/WholeTextFileRDD.scala ---
@@ -51,7 +51,7 @@ private[spark] class WholeTextFileRDD(
case _ =>
}
val jobContext = new JobContextImpl(conf, jobId)
- inputFormat.setMinPartitions(jobContext, minPartitions)
+ inputFormat.setMinPartitions(sc, jobContext, minPartitions)
--- End diff --
You don't need to pass the context. You just need to pass the one value you're going to use from it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22723
I don't think that's necessarily true. This forces the default to be the minimum, which is a behavior change and not obviously what the user wants.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98171/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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/4746/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22723
BinaryFileRDD uses the `minPartitions` input from the user. See https://issues.apache.org/jira/browse/SPARK-22357 I think the logic is more complex and already takes some account of defaultParallelism for similar reasons.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r230579427
--- Diff: core/src/main/scala/org/apache/spark/input/WholeTextFileInputFormat.scala ---
@@ -48,11 +50,11 @@ private[spark] class WholeTextFileInputFormat
* Allow minPartitions set by end-user in order to keep compatibility with old Hadoop API,
* which is set through setMaxSplitSize
*/
- def setMinPartitions(context: JobContext, minPartitions: Int) {
+ def setMinPartitions(sc: SparkContext, context: JobContext, minPartitions: Int) {
--- End diff --
Ok,thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #97375 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97375/testReport)** for PR 22723 at commit [`b80bf66`](https://github.com/apache/spark/commit/b80bf66a8109faa7f58d45b92417a981666866a0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98171 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98171/testReport)** for PR 22723 at commit [`9c3cc61`](https://github.com/apache/spark/commit/9c3cc618fc102efe8bbd42e769aa56b1d8d422a9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22723
In fact, `BinaryFileRDD ` uses `max(defaultParallelism, minPartitions)`:
`BinaryFileRDD --->setMinPartitions--->Math.max(sc.defaultParallelism, minPartitions)`.
In addition, for this unit test:
https://github.com/apache/spark/blob/b80bf66a8109faa7f58d45b92417a981666866a0/core/src/test/scala/org/apache/spark/FileSuite.scala#L304
if we set `spark.default.parallelism=3`, the result will not be what we expected.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98438/
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 #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r230579423
--- Diff: core/src/main/scala/org/apache/spark/input/WholeTextFileInputFormat.scala ---
@@ -48,11 +50,11 @@ private[spark] class WholeTextFileInputFormat
* Allow minPartitions set by end-user in order to keep compatibility with old Hadoop API,
* which is set through setMaxSplitSize
*/
- def setMinPartitions(context: JobContext, minPartitions: Int) {
+ def setMinPartitions(sc: SparkContext, context: JobContext, minPartitions: Int) {
val files = listStatus(context).asScala
val totalLen = files.map(file => if (file.isDirectory) 0L else file.getLen).sum
- val maxSplitSize = Math.ceil(totalLen * 1.0 /
- (if (minPartitions == 0) 1 else minPartitions)).toLong
+ val minPartNum = Math.max(sc.defaultParallelism, minPartitions)
+ val maxSplitSize = Math.ceil(totalLen * 1.0 / minPartNum).toLong
--- End diff --
Thanks, I have change the description.
I think the number of partitions belong to input format, and it can also be used by other RDD.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98437 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98437/testReport)** for PR 22723 at commit [`ccac0a2`](https://github.com/apache/spark/commit/ccac0a2b321467adfb78159a2f5ae1377dc67073).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98438 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98438/testReport)** for PR 22723 at commit [`63f8d3d`](https://github.com/apache/spark/commit/63f8d3d2b98d79ae214593d06047f93255ab66ef).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r229721018
--- Diff: core/src/main/scala/org/apache/spark/input/WholeTextFileInputFormat.scala ---
@@ -48,11 +50,11 @@ private[spark] class WholeTextFileInputFormat
* Allow minPartitions set by end-user in order to keep compatibility with old Hadoop API,
* which is set through setMaxSplitSize
*/
- def setMinPartitions(context: JobContext, minPartitions: Int) {
+ def setMinPartitions(sc: SparkContext, context: JobContext, minPartitions: Int) {
val files = listStatus(context).asScala
val totalLen = files.map(file => if (file.isDirectory) 0L else file.getLen).sum
- val maxSplitSize = Math.ceil(totalLen * 1.0 /
- (if (minPartitions == 0) 1 else minPartitions)).toLong
+ val minPartNum = Math.max(sc.defaultParallelism, minPartitions)
+ val maxSplitSize = Math.ceil(totalLen * 1.0 / minPartNum).toLong
--- End diff --
Yes, this no longer matches the title or JIRA. I am also not clear on the argument why this is better?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97375/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/97888/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #97888 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97888/testReport)** for PR 22723 at commit [`b80bf66`](https://github.com/apache/spark/commit/b80bf66a8109faa7f58d45b92417a981666866a0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r229717747
--- Diff: core/src/main/scala/org/apache/spark/input/WholeTextFileInputFormat.scala ---
@@ -48,11 +50,11 @@ private[spark] class WholeTextFileInputFormat
* Allow minPartitions set by end-user in order to keep compatibility with old Hadoop API,
* which is set through setMaxSplitSize
*/
- def setMinPartitions(context: JobContext, minPartitions: Int) {
+ def setMinPartitions(sc: SparkContext, context: JobContext, minPartitions: Int) {
--- End diff --
Please update the above comment to explain the new behavior.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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/4745/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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/4382/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #97888 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97888/testReport)** for PR 22723 at commit [`b80bf66`](https://github.com/apache/spark/commit/b80bf66a8109faa7f58d45b92417a981666866a0).
* 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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98166/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98166 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98166/testReport)** for PR 22723 at commit [`9c3cc61`](https://github.com/apache/spark/commit/9c3cc618fc102efe8bbd42e769aa56b1d8d422a9).
* 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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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/4574/
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 #22723: [SPARK-25729][CORE]It is better to replace `minPa...
Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/22723#discussion_r229717581
--- Diff: core/src/main/scala/org/apache/spark/input/WholeTextFileInputFormat.scala ---
@@ -48,11 +50,11 @@ private[spark] class WholeTextFileInputFormat
* Allow minPartitions set by end-user in order to keep compatibility with old Hadoop API,
* which is set through setMaxSplitSize
*/
- def setMinPartitions(context: JobContext, minPartitions: Int) {
+ def setMinPartitions(sc: SparkContext, context: JobContext, minPartitions: Int) {
val files = listStatus(context).asScala
val totalLen = files.map(file => if (file.isDirectory) 0L else file.getLen).sum
- val maxSplitSize = Math.ceil(totalLen * 1.0 /
- (if (minPartitions == 0) 1 else minPartitions)).toLong
+ val minPartNum = Math.max(sc.defaultParallelism, minPartitions)
--- End diff --
This is potentially a behavior change. cc @cloud-fan
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98438 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98438/testReport)** for PR 22723 at commit [`63f8d3d`](https://github.com/apache/spark/commit/63f8d3d2b98d79ae214593d06047f93255ab66ef).
* 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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98437 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98437/testReport)** for PR 22723 at commit [`ccac0a2`](https://github.com/apache/spark/commit/ccac0a2b321467adfb78159a2f5ae1377dc67073).
* 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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98437/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98166 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98166/testReport)** for PR 22723 at commit [`9c3cc61`](https://github.com/apache/spark/commit/9c3cc618fc102efe8bbd42e769aa56b1d8d422a9).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/22723
Kind of. As you say it lets a particular `InputFormat` decide this. But it still uses the minPartitions as input. See again https://issues.apache.org/jira/browse/SPARK-22357 for some subtlety, and that much I agree with. I think that's the substance of your change too. But then, if that's true, isn't this a no-op for the case you're interested in? it does affect, then, other `InputFormat`s, but I'm not clear that's valid. Or then: why just make this change in one place?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #98171 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98171/testReport)** for PR 22723 at commit [`9c3cc61`](https://github.com/apache/spark/commit/9c3cc618fc102efe8bbd42e769aa56b1d8d422a9).
* 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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22723
Thanks, yes, you are right.
After you reminded, I realized there were other places, such as `HadoopRDD`.
But I wonder if it's better to just modify `WholeTextFileInputFormat`, like `StreamFileInputFormat`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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/4571/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22723
**[Test build #97375 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/97375/testReport)** for PR 22723 at commit [`b80bf66`](https://github.com/apache/spark/commit/b80bf66a8109faa7f58d45b92417a981666866a0).
* 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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22723
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 #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22723
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/3970/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22723: [SPARK-25729][CORE]It is better to replace `minPartition...
Posted by 10110346 <gi...@git.apache.org>.
Github user 10110346 commented on the issue:
https://github.com/apache/spark/pull/22723
What you say is reasonable.
But from the perspective of resource utilization, I think it is better to replace `minPartitions` with `defaultParallelism`, we can see `BinaryFileRDD` and `createNonBucketedReadRDD`, both of them were done like this.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org