[GitHub] [spark] tgravescs commented on a change in pull request #28880: [SPARK-29465][YARN][WEBUI] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

[GitBox <gi...@apache.org>]

tgravescs commented on a change in pull request #28880:
URL: https://github.com/apache/spark/pull/28880#discussion_r443540504



##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala
##########
@@ -211,9 +211,11 @@ private[spark] class ApplicationMaster(
   final def run(): Int = {
     try {
       val attemptID = if (isClusterMode) {
-        // Set the web ui port to be ephemeral for yarn so we don't conflict with
-        // other spark processes running on the same box
-        System.setProperty(UI_PORT.key, "0")
+        // Set the web ui port to be ephemeral for yarn if not set explicitly
+        // so we don't conflict with other spark processes running on the same box
+        if (System.getProperty(UI_PORT.key) != null) {
+          System.setProperty(UI_PORT.key, "0")
+        }

Review comment:
       yes, please see the discussions there, I think a range of ports would be much better. A single port set on a multi-tenant yarn cluster could easily end up with conflicts. What requirements do you have from Knox, a certain range or ports?
   Normally on yarn I would expect you to use the resource manager web ui proxy on a secure cluster so you wouldn't be accessing the UI directly.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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