You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by robbinspg <gi...@git.apache.org> on 2015/12/21 23:48:59 UTC
[GitHub] spark pull request: [SPARK-12470] Fix size reduction calculation
GitHub user robbinspg opened a pull request:
https://github.com/apache/spark/pull/10421
[SPARK-12470] Fix size reduction calculation
also only allocate required buffer size
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robbinspg/spark-1 master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10421.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 #10421
----
commit 440cc51076d0f35cb30e8c7a6caf0d4607ba78bd
Author: Pete Robbins <ro...@gmail.com>
Date: 2015-12-21T22:21:21Z
Fix size reduction calculation
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168765217
Merged into master and 1.6 branch, thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168973101
created https://issues.apache.org/jira/browse/SPARK-12647 and associated PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10421
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168095060
**[Test build #2273 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2273/consoleFull)** for PR 10421 at commit [`1a09c7a`](https://github.com/apache/spark/commit/1a09c7a16a46936183b442fe80948ecb97c3e330).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] Fix size reduction calculation
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-166445814
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168091850
**[Test build #2273 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2273/consoleFull)** for PR 10421 at commit [`1a09c7a`](https://github.com/apache/spark/commit/1a09c7a16a46936183b442fe80948ecb97c3e330).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10421#discussion_r48639583
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeRowJoiner.scala ---
@@ -171,7 +171,7 @@ object GenerateUnsafeRowJoiner extends CodeGenerator[(StructType, StructType), U
| // row1: ${schema1.size} fields, $bitset1Words words in bitset
| // row2: ${schema2.size}, $bitset2Words words in bitset
| // output: ${schema1.size + schema2.size} fields, $outputBitsetWords words in bitset
- | final int sizeInBytes = row1.getSizeInBytes() + row2.getSizeInBytes();
+ | final int sizeInBytes = row1.getSizeInBytes() + row2.getSizeInBytes() - sizeReduction;
--- End diff --
$sizeReduction
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168048356
**[Test build #2268 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2268/consoleFull)** for PR 10421 at commit [`12724b3`](https://github.com/apache/spark/commit/12724b30d9f2260aa7337433861b2e484f6c256f).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-167738123
@rxin as the original author of this code could you please review the PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168172679
re-merged with latest master
Please retest
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168048077
LGTM, pending for tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168919543
cc @yhuai on this one
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168287520
I will let @davies merge it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-167738673
I think @davies did some pretty big refactoring of this later. @davies can you take a quick look at this? Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-167855300
@rxin I had finished the refactoring long time ago.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168048643
OK to test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168963270
I have a fix for the test failure. Should I create a new Jira and PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168048153
**[Test build #2268 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2268/consoleFull)** for PR 10421 at commit [`12724b3`](https://github.com/apache/spark/commit/12724b30d9f2260aa7337433861b2e484f6c256f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168919276
Merging this into the 1.6 stream has caused a test failure in
org.apache.spark.sql.execution.ExchangeCoordinatorSuite.determining the number of reducers: aggregate operator
There was a change in master in the ExchangeCoordinatorSuite which set the expected partition sizes to a new value. I do not understand why the change in this PR affects the input partition sizes but it does.
I think this is a test issue rather than an issue with this PR. Should I raise a new Jira to fix the expected partition sizes?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] Fix size reduction calculation
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-166585412
I believe this is uncovering a test failure in ExchangeCoordinatorSuite so this please hold this PR until I investigate further.
- determining the number of reducers: aggregate operator *** FAILED ***
3 did not equal 2 (ExchangeCoordinatorSuite.scala:316)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168268755
**[Test build #2282 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2282/consoleFull)** for PR 10421 at commit [`e5a8d1e`](https://github.com/apache/spark/commit/e5a8d1ed8f08b5cec16c207904c7bda6763179f2).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on a diff in the pull request:
https://github.com/apache/spark/pull/10421#discussion_r48599743
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeRowJoiner.scala ---
@@ -171,7 +171,7 @@ object GenerateUnsafeRowJoiner extends CodeGenerator[(StructType, StructType), U
| // row1: ${schema1.size} fields, $bitset1Words words in bitset
| // row2: ${schema2.size}, $bitset2Words words in bitset
| // output: ${schema1.size + schema2.size} fields, $outputBitsetWords words in bitset
- | final int sizeInBytes = row1.getSizeInBytes() + row2.getSizeInBytes();
+ | final int sizeInBytes = row1.getSizeInBytes() + row2.getSizeInBytes() - ($sizeReduction * 8);
--- End diff --
OK. sorted. Can this be run by Jenkins tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by robbinspg <gi...@git.apache.org>.
Github user robbinspg commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168091202
Fixed scala style check
Please retest
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/10421#discussion_r48562209
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/GenerateUnsafeRowJoiner.scala ---
@@ -171,7 +171,7 @@ object GenerateUnsafeRowJoiner extends CodeGenerator[(StructType, StructType), U
| // row1: ${schema1.size} fields, $bitset1Words words in bitset
| // row2: ${schema2.size}, $bitset2Words words in bitset
| // output: ${schema1.size + schema2.size} fields, $outputBitsetWords words in bitset
- | final int sizeInBytes = row1.getSizeInBytes() + row2.getSizeInBytes();
+ | final int sizeInBytes = row1.getSizeInBytes() + row2.getSizeInBytes() - ($sizeReduction * 8);
--- End diff --
It may be better to use number of bytes for `sizeReduction` (also update the comments).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12470] [SQL] Fix size reduction calcula...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10421#issuecomment-168259354
**[Test build #2282 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2282/consoleFull)** for PR 10421 at commit [`e5a8d1e`](https://github.com/apache/spark/commit/e5a8d1ed8f08b5cec16c207904c7bda6763179f2).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org