Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/23030#discussion_r233948023
  
    --- Diff: 
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala
 ---
    @@ -598,13 +598,24 @@ private[yarn] class YarnAllocator(
                 (false, s"Container ${containerId}${onHostStr} was preempted.")
               // Should probably still count memory exceeded exit codes 
towards task failures
               case VMEM_EXCEEDED_EXIT_CODE =>
    -            (true, memLimitExceededLogMessage(
    -              completedContainer.getDiagnostics,
    -              VMEM_EXCEEDED_PATTERN))
    +            val suggestion = if 
(conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
    +              YarnConfiguration.DEFAULT_NM_VMEM_CHECK_ENABLED)) {
    +              s"Consider disabling 
${YarnConfiguration.NM_VMEM_CHECK_ENABLED} because of YARN-4714"
    +            } else {
    +              ""
    +            }
    +            val matcher = 
VMEM_EXCEEDED_PATTERN.matcher(completedContainer.getDiagnostics)
    +            val diag = if (matcher.find()) " " + matcher.group() + "." 
else ""
    +            val message =
    +              s"Container killed by YARN for exceeding virtual memory 
limits.$diag $suggestion."
    +            (true, message)
               case PMEM_EXCEEDED_EXIT_CODE =>
    -            (true, memLimitExceededLogMessage(
    -              completedContainer.getDiagnostics,
    -              PMEM_EXCEEDED_PATTERN))
    +            val suggestion = s"Consider boosting 
${EXECUTOR_MEMORY_OVERHEAD.key}"
    +            val matcher = 
PMEM_EXCEEDED_PATTERN.matcher(completedContainer.getDiagnostics)
    +            val diag = if (matcher.find()) " " + matcher.group() + "." 
else ""
    +            val message =
    +              s"Container killed by YARN for exceeding physical memory 
limits.$diag $suggestion."
    --- End diff --
    
    Ignoring the above comment, `suggestion` is always set, so it could be 
inlined here.
    
    Separately, it's always sub-optimal when formatting is scattered among 
different variables (e.g. the period at the end of the suggestion is here, the 
space before the diag is in the diag message itself).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to