Skip to content

Commit 075dd62

Browse files
ankuriitgMarcelo Vanzin
authored andcommitted
[SPARK-25586][CORE] Remove outer objects from logdebug statements in 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 closures aren't implemented with outerclasses in Scala 2.12 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 Closes apache#22616 from ankuriitg/ankur/SPARK-25586. Authored-by: ankurgupta <[email protected]> Signed-off-by: Marcelo Vanzin <[email protected]>
1 parent d7ae36a commit 075dd62

File tree

1 file changed

+7
-8
lines changed

1 file changed

+7
-8
lines changed

core/src/main/scala/org/apache/spark/util/ClosureCleaner.scala

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,6 @@ private[spark] object ClosureCleaner extends Logging {
285285
innerClasses.foreach { c => logDebug(s" ${c.getName}") }
286286
logDebug(s" + outer classes: ${outerClasses.size}" )
287287
outerClasses.foreach { c => logDebug(s" ${c.getName}") }
288-
logDebug(s" + outer objects: ${outerObjects.size}")
289-
outerObjects.foreach { o => logDebug(s" $o") }
290288
}
291289

292290
// Fail fast if we detect return statements in closures
@@ -318,19 +316,20 @@ private[spark] object ClosureCleaner extends Logging {
318316
if (outerPairs.nonEmpty) {
319317
val (outermostClass, outermostObject) = outerPairs.head
320318
if (isClosure(outermostClass)) {
321-
logDebug(s" + outermost object is a closure, so we clone it: ${outerPairs.head}")
319+
logDebug(s" + outermost object is a closure, so we clone it: ${outermostClass}")
322320
} else if (outermostClass.getName.startsWith("$line")) {
323321
// SPARK-14558: if the outermost object is a REPL line object, we should clone
324322
// and clean it as it may carray a lot of unnecessary information,
325323
// e.g. hadoop conf, spark conf, etc.
326-
logDebug(s" + outermost object is a REPL line object, so we clone it: ${outerPairs.head}")
324+
logDebug(s" + outermost object is a REPL line object, so we clone it:" +
325+
s" ${outermostClass}")
327326
} else {
328327
// The closure is ultimately nested inside a class; keep the object of that
329328
// class without cloning it since we don't want to clone the user's objects.
330329
// Note that we still need to keep around the outermost object itself because
331330
// we need it to clone its child closure later (see below).
332-
logDebug(" + outermost object is not a closure or REPL line object," +
333-
"so do not clone it: " + outerPairs.head)
331+
logDebug(s" + outermost object is not a closure or REPL line object," +
332+
s" so do not clone it: ${outermostClass}")
334333
parent = outermostObject // e.g. SparkContext
335334
outerPairs = outerPairs.tail
336335
}
@@ -341,7 +340,7 @@ private[spark] object ClosureCleaner extends Logging {
341340
// Clone the closure objects themselves, nulling out any fields that are not
342341
// used in the closure we're working on or any of its inner closures.
343342
for ((cls, obj) <- outerPairs) {
344-
logDebug(s" + cloning the object $obj of class ${cls.getName}")
343+
logDebug(s" + cloning instance of class ${cls.getName}")
345344
// We null out these unused references by cloning each object and then filling in all
346345
// required fields from the original object. We need the parent here because the Java
347346
// language specification requires the first constructor parameter of any closure to be
@@ -351,7 +350,7 @@ private[spark] object ClosureCleaner extends Logging {
351350
// If transitive cleaning is enabled, we recursively clean any enclosing closure using
352351
// the already populated accessed fields map of the starting closure
353352
if (cleanTransitively && isClosure(clone.getClass)) {
354-
logDebug(s" + cleaning cloned closure $clone recursively (${cls.getName})")
353+
logDebug(s" + cleaning cloned closure recursively (${cls.getName})")
355354
// No need to check serializable here for the outer closures because we're
356355
// only interested in the serializability of the starting closure
357356
clean(clone, checkSerializable = false, cleanTransitively, accessedFields)

0 commit comments

Comments
 (0)