You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by vgankidi <gi...@git.apache.org> on 2017/11/01 20:35:44 UTC
[GitHub] spark pull request #19634: [SPARK-22412][SQL] Fix incorrect comment in DataS...
GitHub user vgankidi opened a pull request:
https://github.com/apache/spark/pull/19634
[SPARK-22412][SQL] Fix incorrect comment in DataSourceScanExec
## What changes were proposed in this pull request?
Next fit decreasing bin packing algorithm is used to combine splits in DataSourceScanExec but the comment incorrectly states that first fit decreasing algorithm is used. The current implementation doesn't go back to a previously used bin other than the bin that the last element was put into.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vgankidi/spark SPARK-22412
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19634.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 #19634
----
commit b8f38a8e1645b9109ec2ab7e4684dd8ead47c116
Author: Vinitha Gankidi <vg...@netflix.com>
Date: 2017-11-01T20:33:04Z
[SPARK-22412][SQL] Fix incorrect comment in DataSourceScanExec
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19634
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19634
**[Test build #3973 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3973/testReport)** for PR 19634 at commit [`b8f38a8`](https://github.com/apache/spark/commit/b8f38a8e1645b9109ec2ab7e4684dd8ead47c116).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by vgankidi <gi...@git.apache.org>.
Github user vgankidi commented on the issue:
https://github.com/apache/spark/pull/19634
@gatorsmile I also wanted to discuss if we should consider other bin packing algorithms. According to this http://www.math.unl.edu/~s-sjessie1/203Handouts/Bin%20Packing.pdf, next fit decreasing is the least efficient of all but it is easiest to implement and has O(N) run time.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19634
Fewer combined splits might not matter in this case.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19634
**[Test build #3973 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3973/testReport)** for PR 19634 at commit [`b8f38a8`](https://github.com/apache/spark/commit/b8f38a8e1645b9109ec2ab7e4684dd8ead47c116).
* 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 #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by vgankidi <gi...@git.apache.org>.
Github user vgankidi commented on the issue:
https://github.com/apache/spark/pull/19634
We will end up having fewer combined splits. That reduces the number of files that the job produces and also reduces the number of tasks in the downstream jobs. In some tests I have noticed about 10% reduction in the combined splits. However, the simple implementation of FFD has O(n^2) run time.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19634
Thanks! Merged to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19634: [SPARK-22412][SQL] Fix incorrect comment in DataS...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/19634#discussion_r148937385
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala ---
@@ -469,7 +469,7 @@ case class FileSourceScanExec(
currentSize = 0
}
- // Assign files to partitions using "First Fit Decreasing" (FFD)
+ // Assign files to partitions using "Next Fit Decreasing"
--- End diff --
This is correct.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/19634
@vgankidi Does it help the performance of our file reading?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19634: [SPARK-22412][SQL] Fix incorrect comment in DataS...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/19634#discussion_r148875065
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/DataSourceScanExec.scala ---
@@ -469,7 +469,7 @@ case class FileSourceScanExec(
currentSize = 0
}
- // Assign files to partitions using "First Fit Decreasing" (FFD)
+ // Assign files to partitions using "Next Fit Decreasing"
--- End diff --
@liancheng do you agree?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19634: [SPARK-22412][SQL] Fix incorrect comment in DataSourceSc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19634
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #19634: [SPARK-22412][SQL] Fix incorrect comment in DataS...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19634
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org