You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by rdblue <gi...@git.apache.org> on 2016/04/16 03:25:21 UTC
[GitHub] spark pull request: SPARK-14679: Fix UI DAG visualization OOM.
GitHub user rdblue opened a pull request:
https://github.com/apache/spark/pull/12437
SPARK-14679: Fix UI DAG visualization OOM.
## What changes were proposed in this pull request?
The DAG visualization can cause an OOM when generating the DOT file.
This happens because clusters are not correctly deduped by a contains
check because they use the default equals implementation. This adds a
working equals implementation.
## How was this patch tested?
This adds a test suite that checks the new equals implementation.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rdblue/spark SPARK-14679-fix-ui-oom
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12437.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 #12437
----
commit e0bda13cb037649c2b9dd7247b3b90e76ae20cbc
Author: Ryan Blue <bl...@apache.org>
Date: 2016-04-16T01:22:16Z
SPARK-14679: Fix UI DAG visualization OOM.
The DAG visualization can cause an OOM when generating the DOT file.
This happens because clusters are not correctly deduped by a contains
check because they use the default equals implementation.
----
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12437
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r60257825
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
--- End diff --
It's similar, but it's calling a method on the other object to see if it would reject equality. There's a good explanation of [`canEqual` on StackOverflow](http://stackoverflow.com/questions/32093526/canequal-in-the-scala-equals-trait).
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SaintBacchus <gi...@git.apache.org>.
Github user SaintBacchus commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211227098
@rdblue Can this PR fix the case like this:
```java
2016-02-24 15:40:20,260 | ERROR | [qtp1927776715-4120] | Failed to make dot file of stage 619 | org.apache.spark.Logging$class.logError(Logging.scala:96)
java.lang.OutOfMemoryError: Requested array size exceeds VM limit
at java.util.Arrays.copyOf(Arrays.java:3332)
at java.lang.AbstractStringBuilder.expandCapacity(AbstractStringBuilder.java:137)
```
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211550321
@SaintBacchus, I think that error message is essentially the same as that we were seeing, it's just hitting a different limitation when expanding:
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-210725718
Merged build finished. Test PASSed.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by joan38 <gi...@git.apache.org>.
Github user joan38 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r60261647
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
--- End diff --
The only benefit you get with `canEqual` over `this.getClass == that.getClass` is that it's a little bit less strict.
See https://www.artima.com/lejava/articles/equality.html (look for "getClass" in the text to avoid reading the whole thing)
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-210725720
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55986/
Test PASSed.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r59965498
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
+ _childClusters == that._childClusters &&
+ id == that.id &&
+ _name == that._name
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val state = Seq(_childClusters, id, _name)
+ state.map(_.hashCode()).foldLeft(0)((a, b) => 31 * a + b)
--- End diff --
Nit: might be safer to use `Objects.hashCode` to handle null
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211549864
**[Test build #56123 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56123/consoleFull)** for PR 12437 at commit [`9764b43`](https://github.com/apache/spark/commit/9764b43e18a5196ba920341faf76e7d8c6dd2c1a).
* 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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211550713
**[Test build #56125 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56125/consoleFull)** for PR 12437 at commit [`f05edc1`](https://github.com/apache/spark/commit/f05edc15a1ff6f3e0c654989dd6efbdeeeeacb7c).
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211551771
**[Test build #56125 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56125/consoleFull)** for PR 12437 at commit [`f05edc1`](https://github.com/apache/spark/commit/f05edc15a1ff6f3e0c654989dd6efbdeeeeacb7c).
* This patch **fails build dependency 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-14679] [UI] Fix UI DAG visualization OO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211551783
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56125/
Test FAILed.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r59973851
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
+ _childClusters == that._childClusters &&
+ id == that.id &&
+ _name == that._name
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val state = Seq(_childClusters, id, _name)
+ state.map(_.hashCode()).foldLeft(0)((a, b) => 31 * a + b)
--- End diff --
Nit: Isn't it allmost shorter to write out the hashcode?
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-212483444
Thank you @srowen!
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211549270
**[Test build #56123 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56123/consoleFull)** for PR 12437 at commit [`9764b43`](https://github.com/apache/spark/commit/9764b43e18a5196ba920341faf76e7d8c6dd2c1a).
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r60122812
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
+ _childClusters == that._childClusters &&
+ id == that.id &&
+ _name == that._name
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val state = Seq(_childClusters, id, _name)
+ state.map(_.hashCode()).foldLeft(0)((a, b) => 31 * a + b)
--- End diff --
I updated this to use `java.util.Objects.hashCode`, assuming you meant that and not the Guava Objects.hashCode. I opted for this form because it was fairly concise and there doesn't seem to be an established common practice in the rest of the codebase from a quick search. Just let me know what the consensus on this is and I'll change it. 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-14679] [UI] Fix UI DAG visualization OO...
Posted by joan38 <gi...@git.apache.org>.
Github user joan38 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r60091207
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
+ _childClusters == that._childClusters &&
+ id == that.id &&
+ _name == that._name
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val state = Seq(_childClusters, id, _name)
+ state.map(_.hashCode()).foldLeft(0)((a, b) => 31 * a + b)
--- End diff --
The advantage of the map/fold notation is that you don't write calls to `hashCode` and `31` multiple times. Probably better from 3 fields and more.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211556266
The test failure looks unrelated to my changes.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r60205073
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
--- End diff --
OK this is another way or saying "this.getClass == that.getClass", essentially? I think it is OK.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-212370131
Merged to master/1.6
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211776124
**[Test build #2828 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2828/consoleFull)** for PR 12437 at commit [`f05edc1`](https://github.com/apache/spark/commit/f05edc15a1ff6f3e0c654989dd6efbdeeeeacb7c).
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-210725635
**[Test build #55986 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55986/consoleFull)** for PR 12437 at commit [`e0bda13`](https://github.com/apache/spark/commit/e0bda13cb037649c2b9dd7247b3b90e76ae20cbc).
* 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-14679] [UI] Fix UI DAG visualization OO...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r59989555
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
+ _childClusters == that._childClusters &&
+ id == that.id &&
+ _name == that._name
+ case _ => false
+ }
+
+ override def hashCode(): Int = {
+ val state = Seq(_childClusters, id, _name)
+ state.map(_.hashCode()).foldLeft(0)((a, b) => 31 * a + b)
--- End diff --
Yeah, we had a similar discussion at https://github.com/apache/spark/pull/12157 just now. This is a decent way to write it when there are a lot of fields; for 3 fields, I'm neutral about it. The alternative is `31 * (31 * Objects.hashCode(_childClusters) + Objects.hashCode(id)) + Objects.hashCode(_name)`
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211551781
Merged build finished. Test FAILed.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-210704904
**[Test build #55986 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55986/consoleFull)** for PR 12437 at commit [`e0bda13`](https://github.com/apache/spark/commit/e0bda13cb037649c2b9dd7247b3b90e76ae20cbc).
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211825360
**[Test build #2828 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2828/consoleFull)** for PR 12437 at commit [`f05edc1`](https://github.com/apache/spark/commit/f05edc15a1ff6f3e0c654989dd6efbdeeeeacb7c).
* 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-14679] [UI] Fix UI DAG visualization OO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211549868
Merged build finished. Test FAILed.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by rdblue <gi...@git.apache.org>.
Github user rdblue commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r60121340
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
--- End diff --
This check ensures that equality is symmetric if subclasses have a different definition of equals. As I understand scala best practices, it should be there if the class can be extended. Right now this isn't extended, but I thought it may be in the future so I included it. I'm fine either way.
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12437#discussion_r59965493
--- Diff: core/src/main/scala/org/apache/spark/ui/scope/RDDOperationGraph.scala ---
@@ -72,6 +72,22 @@ private[ui] class RDDOperationCluster(val id: String, private var _name: String)
def getCachedNodes: Seq[RDDOperationNode] = {
_childNodes.filter(_.cached) ++ _childClusters.flatMap(_.getCachedNodes)
}
+
+ def canEqual(other: Any): Boolean = other.isInstanceOf[RDDOperationCluster]
+
+ override def equals(other: Any): Boolean = other match {
+ case that: RDDOperationCluster =>
+ (that canEqual this) &&
--- End diff --
Do we need `canEqual` since `that` is already known to be a `RDDOperationCluster`?
---
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-14679] [UI] Fix UI DAG visualization OO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12437#issuecomment-211549872
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56123/
Test FAILed.
---
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