Changing autotuner memory error to warning in comments (#1418)

* Changing autotuner memory issue from error to warning in output Signed-off-by: mattahrens <[email protected]> * Changing autotuner memory issue from error to warning in output - updating unit tests Signed-off-by: mattahrens <[email protected]> * Removing trailing space in comment Signed-off-by: mattahrens <[email protected]> --------- Signed-off-by: mattahrens <[email protected]>
NVIDIA · Nov 14, 2024 · acec193 · acec193
1 parent 4fac4c3
commit acec193
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 8 deletions.
diff --git a/core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/AutoTuner.scala b/core/src/main/scala/com/nvidia/spark/rapids/tool/profiling/AutoTuner.scala
@@ -539,12 +539,12 @@ class AutoTuner(
         // For now just throw so we don't get any tunings and its obvious to user this isn't a good
         // setup. In the future we may just recommend them to use larger nodes. This would be more
         // ideal once we hook up actual executor heap from an eventlog vs what user passes in.
-        throwNotEnoughMemException(minExecHeapMem + minOverhead)
+        warnNotEnoughMem(minExecHeapMem + minOverhead)
         (0, 0, 0, false)
       } else {
         val leftOverMemUsingMinHeap = containerMem - minExecHeapMem
         if (leftOverMemUsingMinHeap < 0) {
-          throwNotEnoughMemException(minExecHeapMem + minOverhead)
+          warnNotEnoughMem(minExecHeapMem + minOverhead)
         }
         // Pinned memory uses any unused space up to 4GB. Spill memory is same size as pinned.
         val pinnedMem = Math.min(MAX_PINNED_MEMORY_MB, (leftOverMemUsingMinHeap / 2)).toLong
@@ -556,13 +556,12 @@ class AutoTuner(
     }
   }
 
-  private def throwNotEnoughMemException(minSize: Long): Unit = {
+  private def warnNotEnoughMem(minSize: Long): Unit = {
     // in the future it would be nice to enhance the error message with a recommendation of size
-    val msg = "This node/worker configuration is not ideal for using the Spark Rapids " +
-      "Accelerator because it doesn't have enough memory for the executors. " +
+    val msg = "This node/worker configuration is not ideal for using the Spark Rapids\n" +
+      "Accelerator because it doesn't have enough memory for the executors.\n" +
       s"We recommend using nodes/workers with more memory. Need at least ${minSize}MB memory."
-    logError(msg)
-    throw new IllegalArgumentException(msg)
+    appendComment(msg)
   }
 
   /**

diff --git a/core/src/test/scala/com/nvidia/spark/rapids/tool/profiling/AutoTunerSuite.scala b/core/src/test/scala/com/nvidia/spark/rapids/tool/profiling/AutoTunerSuite.scala
@@ -310,7 +310,15 @@ class AutoTunerSuite extends FunSuite with BeforeAndAfterEach with Logging {
     val platform = PlatformFactory.createInstance(PlatformNames.DATAPROC, clusterPropsOpt)
     val autoTuner = AutoTuner.buildAutoTunerFromProps(dataprocWorkerInfo,
       getGpuAppMockInfoProvider, platform)
-    assertThrows[IllegalArgumentException](autoTuner.getRecommendedProperties())
+    val (properties, comments) = autoTuner.getRecommendedProperties()
+    val autoTunerOutput = Profiler.getAutoTunerResultsAsString(properties, comments)
+    // scalastyle:off line.size.limit
+    val expectedComment =
+      s"""This node/worker configuration is not ideal for using the Spark Rapids
+Accelerator because it doesn't have enough memory for the executors.
+We recommend using nodes/workers with more memory. Need at least 7796MB memory.""".stripMargin.replaceAll("\n", "")
+    // scalastyle:on line.size.limit
+    assert(autoTunerOutput.replaceAll("\n", "").contains(expectedComment))
   }
 
   test("Load cluster properties with CPU memory missing") {