You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GregOwen <gi...@git.apache.org> on 2014/06/10 23:26:06 UTC
[GitHub] spark pull request: [SPARK-2086]
GitHub user GregOwen opened a pull request:
https://github.com/apache/spark/pull/1039
[SPARK-2086]
Changes RDD.toDebugString() to show hierarchy and shuffle transformations more clearly
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/GregOwen/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1039.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 #1039
----
commit a8ba352fb96c0dccd7288c3ae5557e3b270a57df
Author: Gregory Owen <gr...@gmail.com>
Date: 2014-06-10T20:51:09Z
[SPARK-2086] Updated RDD.toDebugString to show hierarchy and shuffle operations more clearly
commit bac5a52b59c927ef6d9a880ab58ddc94a35b2493
Author: Gregory Owen <gr...@gmail.com>
Date: 2014-06-10T21:06:10Z
Merge branch 'master' of github.com:GregOwen/spark
----
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by GregOwen <gi...@git.apache.org>.
Github user GregOwen commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48648011
@pwendell Fixed up those stylistic things.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by GregOwen <gi...@git.apache.org>.
Github user GregOwen commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45682232
@pwendell Added `+` and `|` markings
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642925
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
+ (frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]]))
+ ++ debugString(endDep.rdd, prefix, endDep.isInstanceOf[ShuffleDependency[_,_]], true))
+ }
+ }
+ // The first RDD in the dependency stack has no parents, so no need for a +-
+ def firstDebugString(rdd: RDD[_]): Seq[String] = {
--- End diff --
btw - not sure scala allows the private identifier in inner functions, so you'll have to see.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48287559
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/16402/
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45679822
Merged build finished.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by GregOwen <gi...@git.apache.org>.
Github user GregOwen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14781971
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
--- End diff --
I had actually hoped to do something like that, but we need access to `d` itself for the call to `d.isInstanceOf[ShuffleDependency[_,_]]`
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45809821
Merged build finished. All automated tests 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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by GregOwen <gi...@git.apache.org>.
Github user GregOwen closed the pull request at:
https://github.com/apache/spark/pull/1039
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48286947
Jenkins, retest this please.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48655412
Some kind of merge foo made this diff really messed up. Did you rebase on a different branch other than the current master?
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48287336
Build started.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45801302
Merged build started.
---
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.
---
[GitHub] spark pull request: [SPARK-2086]
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45674189
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642874
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
+ (frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]]))
+ ++ debugString(endDep.rdd, prefix, endDep.isInstanceOf[ShuffleDependency[_,_]], true))
+ }
+ }
+ // The first RDD in the dependency stack has no parents, so no need for a +-
+ def firstDebugString(rdd: RDD[_]): Seq[String] = {
+ val partitionStr = "(" + rdd.partitions.size + ")"
+ val leftOffset = (partitionStr.length - 1)/2
+ val nextPrefix = (" " * leftOffset) + "|" + (" " * (partitionStr.length - leftOffset))
+ Seq(partitionStr + " " + rdd) ++ debugChildren(rdd, nextPrefix)
+ }
+ def shuffleDebugString(rdd: RDD[_], prefix: String = "", isLastChild: Boolean): Seq[String] = {
+ val partitionStr = "(" + rdd.partitions.size + ")"
+ val thisPrefix = prefix.replaceAll("\\|\\s+$", "")
+ val leftOffset = (partitionStr.length - 1)/2
+ val nextPrefix = (
+ thisPrefix
+ + (if (isLastChild) " " else "| ")
+ + (" " * leftOffset) + "|" + (" " * (partitionStr.length - leftOffset)))
+ Seq(thisPrefix + "+-" + partitionStr + " " + rdd) ++ debugChildren(rdd, nextPrefix)
+ }
+ def debugString(rdd: RDD[_], prefix: String = "", isShuffle: Boolean = true, isLastChild: Boolean = false): Seq[String] = {
+ if (isShuffle) { shuffleDebugString(rdd, prefix, isLastChild) }
--- End diff --
This if/else should be broken onto multiple
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45675858
Merged build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45809824
All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15687/
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45675477
Jenkins, test this please
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642790
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
+ (frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]]))
+ ++ debugString(endDep.rdd, prefix, endDep.isInstanceOf[ShuffleDependency[_,_]], true))
+ }
+ }
+ // The first RDD in the dependency stack has no parents, so no need for a +-
+ def firstDebugString(rdd: RDD[_]): Seq[String] = {
+ val partitionStr = "(" + rdd.partitions.size + ")"
+ val leftOffset = (partitionStr.length - 1)/2
--- End diff --
The `/` operator should be surrounded by spaces
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45677035
Hey @GregOwen a couple thoughts.
1. If you run this in the spark shell you'll get examples than from within the unit test harness. The issue is that the code we use to detect the "call site" of the RDD (the user function that called it) gets messed up when running unit tests because they are within the spark package.
2. I might slightly prefer adding `+` icons next to the main RDD. /cc @rxin also who might have some thoughts on this.
```
(5) + FlatMappedValuesRDD[321] at apply at Transformer.scala:22
| MappedValuesRDD[320] at apply at Transformer.scala:22
| CoGroupedRDD[319] at apply at Transformer.scala:22
(4) + MappedRDD[318] at apply at Transformer.scala:22
| MappedRDD[316] at apply at Transformer.scala:22
| ParallelCollectionRDD[314] at apply at Transformer.scala:22
(5) + MappedRDD[317] at apply at Transformer.scala:22
| ParallelCollectionRDD[315] at apply at Transformer.scala:22
(6) + MappedRDD[317] at apply at Transformer.scala:22
```
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45836980
Hey @GregOwen sorry to keep changing the spec on you, but I thought a bit more and I think for more complicated graphs we might want to change things a bit. I looked at how some other similar tools work and I was thinking something like this:
```
scala> var a = rdd.map(x => (x, x)).groupByKey(5).map{ case (x, y) => y.sum}.map(x => (1, x))
scala> var b = sc.makeRDD(1 to 1000, 100).map(x => (x, x))
scala> a.join(b, 3).toDebugString
(3) FlatMappedValuesRDD[49] at join at <console>:19
| MappedValuesRDD[48] at join at <console>:19
| CoGroupedRDD[47] at join at <console>:19
+-(5) MappedRDD[38] at map at <console>:14
| | MappedRDD[37] at map at <console>:14
| | MappedValuesRDD[36] at groupByKey at <console>:14
| | MapPartitionsRDD[35] at groupByKey at <console>:14
| | ShuffledRDD[34] at groupByKey at <console>:14
| +-(10) MappedRDD[33] at map at <console>:14
| | ParallelCollectionRDD[0] at makeRDD at <console>:12
|
+-(100) MappedRDD[40] at map at <console>:12
ParallelCollectionRDD[39] at makeRDD at <console>:12
```
For comparison this is how it is now:
```
res15: String =
(3) + FlatMappedValuesRDD[63] at join at <console>:19
| MappedValuesRDD[62] at join at <console>:19
| CoGroupedRDD[61] at join at <console>:19
(5) + MappedRDD[55] at map at <console>:14
| MappedRDD[54] at map at <console>:14
| MappedValuesRDD[53] at groupByKey at <console>:14
| MapPartitionsRDD[52] at groupByKey at <console>:14
| ShuffledRDD[51] at groupByKey at <console>:14
(10) + MappedRDD[50] at map at <console>:14
| ParallelCollectionRDD[0] at makeRDD at <console>:12
(100) + MappedRDD[57] at map at <console>:12
| ParallelCollectionRDD[56] at makeRDD at <console>:12
```
The benefit of the newer structure is that if you have two siblings that are somewhat far apart (because one or both have nested dependencies) then you can visually track it on the left by just following the dots. I stole this format mostly from what happens when you run `sbt/sbt dependency-tree`.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45801061
Jenkins, retest this please.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48286964
Hey @GregOwen this has fallen out of date, mind updating it? I'd like to get it merged soon.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45679823
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/15648/
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48287556
Build finished.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642705
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
--- End diff --
might be slightly cleaner to do:
```
val depRdd = rdd.dependencies.head.rdd
debugString(depRdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
```
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by GregOwen <gi...@git.apache.org>.
Github user GregOwen commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14785131
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
+ (frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]]))
+ ++ debugString(endDep.rdd, prefix, endDep.isInstanceOf[ShuffleDependency[_,_]], true))
+ }
+ }
+ // The first RDD in the dependency stack has no parents, so no need for a +-
+ def firstDebugString(rdd: RDD[_]): Seq[String] = {
--- End diff --
Turns out it does not allow you to use `private` on inner functions. "we refactored the original [object]... by transforming [a private method] into a local function of [another method]. To do so we removed the `private` modifier, which can only be applied (and is only needed) for methods..." Programming in Scala
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45801291
Merged build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642773
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
+ (frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]]))
--- End diff --
For this it would be better to avoid this complex expression:
```
val frontDeps = rdd.dependencies.take(len - 1)
val frontDepStrings = frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]])
val lastDep = rdd.dependencies.last
val lastDepString = debugString(lastDep.rdd, prefix, endDep.isInstanceOf[ShuffleDependency[_,_]], true)
(frontDepStrings ++ lastDepString)
```
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642636
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
--- End diff --
maybe call this `lastDep` and just do `rdd.dependencies.last`
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-48287316
Build triggered.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/1039#discussion_r14642915
--- Diff: core/src/main/scala/org/apache/spark/rdd/RDD.scala ---
@@ -1235,11 +1235,43 @@ abstract class RDD[T: ClassTag](
/** A description of this RDD and its recursive dependencies for debugging. */
def toDebugString: String = {
- def debugString(rdd: RDD[_], prefix: String = ""): Seq[String] = {
- Seq(prefix + rdd + " (" + rdd.partitions.size + " partitions)") ++
- rdd.dependencies.flatMap(d => debugString(d.rdd, prefix + " "))
+ // Apply a different rule to the last child
+ def debugChildren(rdd: RDD[_], prefix: String): Seq[String] = {
+ val len = rdd.dependencies.length
+ len match {
+ case 0 => Seq.empty
+ case 1 =>
+ val d = rdd.dependencies.head
+ debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]], true)
+ case _ =>
+ val frontDeps = rdd.dependencies.take(len - 1)
+ val endDep = rdd.dependencies.takeRight(1).head
+ (frontDeps.flatMap(d => debugString(d.rdd, prefix, d.isInstanceOf[ShuffleDependency[_,_]]))
+ ++ debugString(endDep.rdd, prefix, endDep.isInstanceOf[ShuffleDependency[_,_]], true))
+ }
+ }
+ // The first RDD in the dependency stack has no parents, so no need for a +-
+ def firstDebugString(rdd: RDD[_]): Seq[String] = {
--- End diff --
It might be good to make all these inner functions private to avoid having to deal with binary checker changes in the future.
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45791858
Hey @GregOwen this build is actually failing tests because of some checks we do for binary compatibility of Spark across versions. This case you've changed the signature of an inner function and it (falsely) detects this as an incompatible change. You can fix this by adding an exception in `project/MimaExcludes.scala` under 1.1:
```
ProblemFilters.exclude[MissingMethodProblem]("org.apache.spark.rdd.RDD.org$apache$spark$rdd$RDD$$debugString$1")
```
---
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.
---
[GitHub] spark pull request: [SPARK-2086] Improve output of toDebugString t...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/1039#issuecomment-45675870
Merged build started.
---
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.
---