You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ankuriitg <gi...@git.apache.org> on 2018/10/02 18:50:51 UTC
[GitHub] spark pull request #22616: [SPARK-25586][Core] Remove logdebug statement for...
GitHub user ankuriitg opened a pull request:
https://github.com/apache/spark/pull/22616
[SPARK-25586][Core] Remove logdebug statement for outer objects from
ClosureCleaner
## What changes were proposed in this pull request?
Cause: Recently test_glr_summary failed for PR of SPARK-25118, which enables
spark-shell to run with default log level. It failed because this logdebug was
called for GeneralizedLinearRegressionTrainingSummary which invoked its toString
method, which started a Spark Job and ended up running into an infinite loop.
Fix: Remove logDebug statement for outer objects, as in Scala 2.12, closures
aren't implemented with outerclasses and this debug statement looses its purpose
## How was this patch tested?
Ran python pyspark-ml tests on top of PR for SPARK-25118 and ClosureCleaner unit
tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ankuriitg/spark ankur/SPARK-25586
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/22616.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 #22616
----
commit cc8926d6a69786412e7630b03d738da8fda7effe
Author: ankurgupta <an...@...>
Date: 2018-10-02T16:24:20Z
[SPARK-25586][Core] Remove logdebug statement for outer objects from
ClosureCleaner
Cause: Recently a test_glr_summary failed for PR of SPARK-25118, which enables
spark-shell to run with default log level. It failed because this logdebug was
called for GeneralizedLinearRegressionTrainingSummary which invoked its toString
method, which started a Spark Job and ended up running into an infinite loop.
Fix: Remove logDebug statement for outer objects, as in Scala 2.12, closures
aren't implemented with outerclasses and this debug statement looses its purpose
Testing Done:
Ran python pyspark-ml tests on top of PR for SPARK-25118 and ClosureCleaner unit
tests
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22616
**[Test build #96875 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96875/testReport)** for PR 22616 at commit [`cc8926d`](https://github.com/apache/spark/commit/cc8926d6a69786412e7630b03d738da8fda7effe).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22616
**[Test build #96898 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96898/testReport)** for PR 22616 at commit [`dd08905`](https://github.com/apache/spark/commit/dd089054027d36c46ce53dece8525a6f7a50f4c5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22616
**[Test build #96898 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96898/testReport)** for PR 22616 at commit [`dd08905`](https://github.com/apache/spark/commit/dd089054027d36c46ce53dece8525a6f7a50f4c5).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` logDebug(s\" + cloning instance of class $`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96876/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
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 #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by ankuriitg <gi...@git.apache.org>.
Github user ankuriitg commented on the issue:
https://github.com/apache/spark/pull/22616
@srowen @yanboliang
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22616
**[Test build #96876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96876/testReport)** for PR 22616 at commit [`2723b47`](https://github.com/apache/spark/commit/2723b47ded05c2eb2ca6cdb3f893965120c04920).
* 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 #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22616
Known flaky test. Merging to master.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22616: [SPARK-25586][Core] Remove outer objects from log...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22616#discussion_r222249333
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -341,7 +340,7 @@ private[spark] object ClosureCleaner extends Logging {
// Clone the closure objects themselves, nulling out any fields that are not
// used in the closure we're working on or any of its inner closures.
for ((cls, obj) <- outerPairs) {
- logDebug(s" + cloning the object $obj of class ${cls.getName}")
+ logDebug(s" + cloning the object of class ${cls.getName}")
--- End diff --
Nit: maybe "cloning instance of class .."
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22616
**[Test build #96876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96876/testReport)** for PR 22616 at commit [`2723b47`](https://github.com/apache/spark/commit/2723b47ded05c2eb2ca6cdb3f893965120c04920).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22616: [SPARK-25586][Core] Remove logdebug statement for...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22616#discussion_r222122237
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -285,8 +285,6 @@ private[spark] object ClosureCleaner extends Logging {
innerClasses.foreach { c => logDebug(s" ${c.getName}") }
logDebug(s" + outer classes: ${outerClasses.size}" )
outerClasses.foreach { c => logDebug(s" ${c.getName}") }
- logDebug(s" + outer objects: ${outerObjects.size}")
- outerObjects.foreach { o => logDebug(s" $o") }
--- End diff --
You could keep this (and just change to `o.getClass()`).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
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 #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
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 pull request #22616: [SPARK-25586][Core] Remove logdebug statement for...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22616#discussion_r222122081
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -318,19 +316,20 @@ private[spark] object ClosureCleaner extends Logging {
if (outerPairs.nonEmpty) {
val (outermostClass, outermostObject) = outerPairs.head
if (isClosure(outermostClass)) {
- logDebug(s" + outermost object is a closure, so we clone it: ${outerPairs.head}")
+ logDebug(s" + outermost object is a closure, so we clone it: ${outerPairs.head._1}")
} else if (outermostClass.getName.startsWith("$line")) {
// SPARK-14558: if the outermost object is a REPL line object, we should clone
// and clean it as it may carray a lot of unnecessary information,
// e.g. hadoop conf, spark conf, etc.
- logDebug(s" + outermost object is a REPL line object, so we clone it: ${outerPairs.head}")
+ logDebug(s" + outermost object is a REPL line object, so we clone it:" +
+ s" ${outerPairs.head._1}")
} else {
// The closure is ultimately nested inside a class; keep the object of that
// class without cloning it since we don't want to clone the user's objects.
// Note that we still need to keep around the outermost object itself because
// we need it to clone its child closure later (see below).
logDebug(" + outermost object is not a closure or REPL line object," +
- "so do not clone it: " + outerPairs.head)
+ "so do not clone it: " + outerPairs.head._1)
--- End diff --
Use `outermostClass`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96898/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22616: [SPARK-25586][Core] Remove outer objects from log...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/22616#discussion_r222249281
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -318,19 +316,20 @@ private[spark] object ClosureCleaner extends Logging {
if (outerPairs.nonEmpty) {
val (outermostClass, outermostObject) = outerPairs.head
if (isClosure(outermostClass)) {
- logDebug(s" + outermost object is a closure, so we clone it: ${outerPairs.head}")
+ logDebug(s" + outermost object is a closure, so we clone it: ${outermostClass}")
} else if (outermostClass.getName.startsWith("$line")) {
// SPARK-14558: if the outermost object is a REPL line object, we should clone
// and clean it as it may carray a lot of unnecessary information,
// e.g. hadoop conf, spark conf, etc.
- logDebug(s" + outermost object is a REPL line object, so we clone it: ${outerPairs.head}")
+ logDebug(s" + outermost object is a REPL line object, so we clone it:" +
+ s" ${outermostClass}")
} else {
// The closure is ultimately nested inside a class; keep the object of that
// class without cloning it since we don't want to clone the user's objects.
// Note that we still need to keep around the outermost object itself because
// we need it to clone its child closure later (see below).
logDebug(" + outermost object is not a closure or REPL line object," +
- "so do not clone it: " + outerPairs.head)
+ "so do not clone it: " + outermostClass)
--- End diff --
Nit: this could use interpolation too
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
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 #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/22616
**[Test build #96875 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/96875/testReport)** for PR 22616 at commit [`cc8926d`](https://github.com/apache/spark/commit/cc8926d6a69786412e7630b03d738da8fda7effe).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` logDebug(s\" + cloning the object of class $`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22616: [SPARK-25586][Core] Remove logdebug statement for...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/22616#discussion_r222126406
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -285,8 +285,6 @@ private[spark] object ClosureCleaner extends Logging {
innerClasses.foreach { c => logDebug(s" ${c.getName}") }
logDebug(s" + outer classes: ${outerClasses.size}" )
outerClasses.foreach { c => logDebug(s" ${c.getName}") }
- logDebug(s" + outer objects: ${outerObjects.size}")
- outerObjects.foreach { o => logDebug(s" $o") }
--- End diff --
👍
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/22616
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 #22616: [SPARK-25586][Core] Remove logdebug statement for outer ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
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 #22616: [SPARK-25586][Core] Remove outer objects from logdebug s...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/22616
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/96875/
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 #22616: [SPARK-25586][Core] Remove logdebug statement for...
Posted by ankuriitg <gi...@git.apache.org>.
Github user ankuriitg commented on a diff in the pull request:
https://github.com/apache/spark/pull/22616#discussion_r222125433
--- Diff: core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala ---
@@ -285,8 +285,6 @@ private[spark] object ClosureCleaner extends Logging {
innerClasses.foreach { c => logDebug(s" ${c.getName}") }
logDebug(s" + outer classes: ${outerClasses.size}" )
outerClasses.foreach { c => logDebug(s" ${c.getName}") }
- logDebug(s" + outer objects: ${outerObjects.size}")
- outerObjects.foreach { o => logDebug(s" $o") }
--- End diff --
I removed this statement altogether because outerclasses are printed in the statement above.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #22616: [SPARK-25586][Core] Remove outer objects from log...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/22616
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org