From 4a486f40cf81f9a602e62e2e2bb050a6ac175f57 Mon Sep 17 00:00:00 2001
From: Minchu Yang <minyang@minyang-mn3.linkedin.biz>
Date: Mon, 13 Sep 2021 23:23:33 -0500
Subject: [PATCH] [SPARK-36705][FOLLOW-UP] Fix unnecessary logWarning when
 PUSH_BASED_SHUFFLE_ENABLED is set to false

### What changes were proposed in this pull request?

Only throw logWarning when `PUSH_BASED_SHUFFLE_ENABLED` is set to true and `canDoPushBasedShuffle` is false

### Why are the changes needed?

Currently, this logWarning will still be printed out even when `PUSH_BASED_SHUFFLE_ENABLED` is set to false, which is unnecessary.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Passed existing UT.

Closes #33984 from rmcyang/SPARK-36705-follow-up.

Authored-by: Minchu Yang <minyang@minyang-mn3.linkedin.biz>
Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>
(cherry picked from commit 2d7dc7c7ce6d524a232f37927ca179f162ad9971)
Signed-off-by: Mridul Muralidharan <mridulatgmail.com>
---
 .../scala/org/apache/spark/util/Utils.scala   | 34 +++++++++++--------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/core/src/main/scala/org/apache/spark/util/Utils.scala b/core/src/main/scala/org/apache/spark/util/Utils.scala
index a112214ab8..1a276f3144 100644
--- a/core/src/main/scala/org/apache/spark/util/Utils.scala
+++ b/core/src/main/scala/org/apache/spark/util/Utils.scala
@@ -2598,23 +2598,27 @@ private[spark] object Utils extends Logging {
    *   - serializer(such as KryoSerializer) supports relocation of serialized objects
    */
   def isPushBasedShuffleEnabled(conf: SparkConf): Boolean = {
-    val serializer = Utils.classForName(conf.get(SERIALIZER)).getConstructor(classOf[SparkConf])
-      .newInstance(conf).asInstanceOf[Serializer]
-    val canDoPushBasedShuffle =
-      conf.get(PUSH_BASED_SHUFFLE_ENABLED) &&
-        (conf.get(IS_TESTING).getOrElse(false) ||
-          (conf.get(SHUFFLE_SERVICE_ENABLED) &&
-            conf.get(SparkLauncher.SPARK_MASTER, null) == "yarn" &&
-            // TODO: [SPARK-36744] needs to support IO encryption for push-based shuffle
-            !conf.get(IO_ENCRYPTION_ENABLED) &&
-            serializer.supportsRelocationOfSerializedObjects))
+    val pushBasedShuffleEnabled = conf.get(PUSH_BASED_SHUFFLE_ENABLED)
+    if (pushBasedShuffleEnabled) {
+      val serializer = Utils.classForName(conf.get(SERIALIZER)).getConstructor(classOf[SparkConf])
+        .newInstance(conf).asInstanceOf[Serializer]
+      val canDoPushBasedShuffle = conf.get(IS_TESTING).getOrElse(false) ||
+        (conf.get(SHUFFLE_SERVICE_ENABLED) &&
+          conf.get(SparkLauncher.SPARK_MASTER, null) == "yarn" &&
+          // TODO: [SPARK-36744] needs to support IO encryption for push-based shuffle
+          !conf.get(IO_ENCRYPTION_ENABLED) &&
+          serializer.supportsRelocationOfSerializedObjects)
 
-    if (!canDoPushBasedShuffle) {
-      logWarning("Push-based shuffle can only be enabled when the application is submitted" +
-        "to run in YARN mode, with external shuffle service enabled, IO encryption disabled, and" +
-        "relocation of serialized objects supported.")
+      if (!canDoPushBasedShuffle) {
+        logWarning("Push-based shuffle can only be enabled when the application is submitted " +
+          "to run in YARN mode, with external shuffle service enabled, IO encryption disabled, " +
+          "and relocation of serialized objects supported.")
+      }
+
+      canDoPushBasedShuffle
+    } else {
+      false
     }
-    canDoPushBasedShuffle
   }
 
   /**