You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by frb502 <gi...@git.apache.org> on 2018/05/26 10:23:48 UTC
[GitHub] spark pull request #21438: Improve SQLAppStatusListener.aggregateMetrics() t...
GitHub user frb502 opened a pull request:
https://github.com/apache/spark/pull/21438
Improve SQLAppStatusListener.aggregateMetrics() too show
## What changes were proposed in this pull request?
JIRA Issue: https://issues.apache.org/jira/browse/SPARK-24398
## How was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/frb502/spark SPARK-24398
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21438.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 #21438
----
commit 6b906b11de5544cf4fc97dcd10ecec2d7d2ec506
Author: 傅荣滨 <fu...@...>
Date: 2018-05-26T10:15:10Z
Improve SQLAppStatusListener.aggregateMetrics() too show
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: Improve SQLAppStatusListener.aggregateMetrics() too show
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
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 issue #21438: Improve SQLAppStatusListener.aggregateMetrics() too show
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/21438
please see http://spark.apache.org/contributing.html on "Pull Request"
also fix the PR title to start with `[SPARK-24398]`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: Improve SQLAppStatusListener.aggregateMetrics() too show
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
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 issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/21438
I think filtering off `metricIds` still make sense right? @gatorsmile
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91641/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
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 #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/21438
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
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 #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/21438
Please update the PR description.
Also do you mean "show" -> "slow"?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21438
**[Test build #91641 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91641/testReport)** for PR 21438 at commit [`eb87d2d`](https://github.com/apache/spark/commit/eb87d2d595374f3325a91ac53f0c11bff2b978e7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21438
@felixcheung `metricIds` is not needed. We can get rid of it by using `metricTypes `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21438
Just following up on this old small PR @gatorsmile @frb502 -- was there a different solution here that makes sense? should this be closed?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21438
**[Test build #91234 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91234/testReport)** for PR 21438 at commit [`eb87d2d`](https://github.com/apache/spark/commit/eb87d2d595374f3325a91ac53f0c11bff2b978e7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91234/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/21438
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21438
**[Test build #91641 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91641/testReport)** for PR 21438 at commit [`eb87d2d`](https://github.com/apache/spark/commit/eb87d2d595374f3325a91ac53f0c11bff2b978e7).
* 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 #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21438
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 issue #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener.aggrega...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21438
**[Test build #91234 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91234/testReport)** for PR 21438 at commit [`eb87d2d`](https://github.com/apache/spark/commit/eb87d2d595374f3325a91ac53f0c11bff2b978e7).
* 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 pull request #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener....
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/21438#discussion_r191285757
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala ---
@@ -159,7 +159,7 @@ class SQLAppStatusListener(
}
private def aggregateMetrics(exec: LiveExecutionData): Map[Long, String] = {
- val metricIds = exec.metrics.map(_.accumulatorId).sorted
+ val metricIds = exec.metrics.map(_.accumulatorId).toSet
--- End diff --
I think we can get rid of `metricIds `
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21438: [SPARK-24398] [SQL] Improve SQLAppStatusListener....
Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:
https://github.com/apache/spark/pull/21438#discussion_r194522227
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala ---
@@ -159,7 +159,7 @@ class SQLAppStatusListener(
}
private def aggregateMetrics(exec: LiveExecutionData): Map[Long, String] = {
- val metricIds = exec.metrics.map(_.accumulatorId).sorted
+ val metricIds = exec.metrics.map(_.accumulatorId).toSet
--- End diff --
+1， we can use `metricTypes`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org