You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ala <gi...@git.apache.org> on 2017/10/11 14:23:37 UTC
[GitHub] spark pull request #19473: [SPARK-22251] Metric 'aggregate time' is incorrec...
GitHub user ala opened a pull request:
https://github.com/apache/spark/pull/19473
[SPARK-22251] Metric 'aggregate time' is incorrect when codegen is off
## What changes were proposed in this pull request?
Adding the code for setting 'aggregate time' metric to non-codegen path in HashAggregateExec.
## How was this patch tested?
Tested manually.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ala/spark fix-agg-time
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/19473.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 #19473
----
commit 470b54f41ca7da5331d8ace22911a1a722c9c51b
Author: Ala Luszczak <al...@databricks.com>
Date: 2017-10-11T13:59:35Z
Fix 'aggregate time' when codegen is off.
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251] Metric 'aggregate time' is incorrect when ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19473
Could you add [SQL] to the title? That makes it easier for others to scan PRs.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19473
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82678/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19473
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 #19473: [SPARK-22251] Metric 'aggregate time' is incorrect when ...
Posted by ala <gi...@git.apache.org>.
Github user ala commented on the issue:
https://github.com/apache/spark/pull/19473
@hvanhovell
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19473
**[Test build #82678 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82678/testReport)** for PR 19473 at commit [`44430ab`](https://github.com/apache/spark/commit/44430ab9e309a438f9c42bc4539d227936fc4434).
* 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 #19473: [SPARK-22251] Metric 'aggregate time' is incorrect when ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19473
**[Test build #82637 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82637/testReport)** for PR 19473 at commit [`470b54f`](https://github.com/apache/spark/commit/470b54f41ca7da5331d8ace22911a1a722c9c51b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19473
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 #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19473
**[Test build #82637 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82637/testReport)** for PR 19473 at commit [`470b54f`](https://github.com/apache/spark/commit/470b54f41ca7da5331d8ace22911a1a722c9c51b).
* 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 #19473: [SPARK-22251] Metric 'aggregate time' is incorrect when ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19473
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/19473
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/82637/
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 #19473: [SPARK-22251][SQL] Metric 'aggregate time' is inc...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/19473
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the issue:
https://github.com/apache/spark/pull/19473
Merging to master. Thanks!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/19473
**[Test build #82678 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82678/testReport)** for PR 19473 at commit [`44430ab`](https://github.com/apache/spark/commit/44430ab9e309a438f9c42bc4539d227936fc4434).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19473
LGTM cc: @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19473
LGTM. btw, do we have any chance to add the same metrics in `SortAggregateExec` and `ObjectHashAggregateExec`? It seems we cannot simply add the metrics in both cuz they use the iterator to aggregate inputs. But, I think it might help for users.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by maropu <gi...@git.apache.org>.
Github user maropu commented on the issue:
https://github.com/apache/spark/pull/19473
yea, fair enough
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #19473: [SPARK-22251][SQL] Metric 'aggregate time' is incorrect ...
Posted by ala <gi...@git.apache.org>.
Github user ala commented on the issue:
https://github.com/apache/spark/pull/19473
@maropu I added aggregate time to `ObjectHashAggregateExec`. `SortAggregateExec` is a different case. First, because it processes input gradually, so it's hard to precisely measure the time. And second, because "aggregate time" is really "time it took to build the hash table", which doesn't make much sense for sort-based aggregation.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org