[GitHub] spark pull request #22876: [SPARK-25869] [YARN] the original diagnostics is ...

[srowen <gi...@git.apache.org>]

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

    https://github.com/apache/spark/pull/22876#discussion_r230053125
  
    --- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala ---
    @@ -293,6 +293,9 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments) extends
             }
     
             if (!unregistered) {
    +          logInfo("Waiting for " + sparkConf.get("spark.yarn.report.interval", "1000").toInt +"ms to unregister am," +
    --- End diff --
    
    Use interpolation. No need to call `.toInt`. am -> AM or Application master. msg -> message; these should be complete sentences. You get the config twice here. Is this the default used elsewhere?
    
    I don't know this code well and don't think a Thread.sleep is a great way to coordinate.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org