You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/20 12:46:30 UTC

[GitHub] [spark] rajatahujaatinmobi opened a new pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

rajatahujaatinmobi opened a new pull request #28879:
URL: https://github.com/apache/spark/pull/28879


   **What changes were proposed in this pull request?**
   When a Spark Job launched in Cluster mode with Yarn, Application Master sets `spark.ui.port` port to 0 which means Driver's web UI gets any random port even if we want to explicitly set the Port range for Driver's Web UI
   
   **Why are the changes needed?**
   We access Spark Web UI via Knox Proxy, and there are firewall restrictions due to which we can not access Spark Web UI since Web UI port range gets random port even if we set explicitly. 
   
   This Change will check if there is a specified port range explicitly mentioned so that it does not assign a random port. 
   
   Does this PR introduce any user-facing change?
   No
   
   How was this patch tested?
   Local Tested.


----------------------------------------------------------------
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


[GitHub] [spark] rajatahujaatinmobi commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
rajatahujaatinmobi commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-647069836


   > Hi, @rajatahujaatinmobi . If this is required for 3.1/3.0, please make a PR to `master` branch.
   
   Hi @dongjoon-hyun We are using Spark 2.4 as of now. So it is required in Spark 2.4 but yes I will make a separate Master Pull request too. 


----------------------------------------------------------------
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


[GitHub] [spark] dongjoon-hyun commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-647196274


   Thank you for confirming, @rajatahujaatinmobi . Apache Spark community have a backporting policy. To prevent any regression, we fix `master/branch-3.0` first always. Otherwise, Apache Spark 2.4.7 may have this fix while 3.0.1 has a regression due to lack of this patch. So, please close this first and make a PR to master branch.


----------------------------------------------------------------
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


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

Posted by GitBox <gi...@apache.org>.
vishwas-n commented on a change in pull request #28879:
URL: https://github.com/apache/spark/pull/28879#discussion_r443951411



##########
File path: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/ApplicationMaster.scala
##########
@@ -254,9 +254,11 @@ private[spark] class ApplicationMaster(args: ApplicationMasterArguments) extends
       var attemptID: Option[String] = None
 
       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("spark.ui.port", "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("spark.ui.port") != null) {

Review comment:
       @rajatahujaatinmobi , the check must use '==' operator and not '!='
   say spark.ui.port is set to 9999, above if condition equates to true and value is overridden to 0(random port)




----------------------------------------------------------------
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


[GitHub] [spark] dongjoon-hyun closed pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #28879:
URL: https://github.com/apache/spark/pull/28879


   


----------------------------------------------------------------
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


[GitHub] [spark] AmplabJenkins commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-646990454


   Can one of the admins verify this patch?


----------------------------------------------------------------
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


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-646990259


   Can one of the admins verify this patch?


----------------------------------------------------------------
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


[GitHub] [spark] rajatahujaatinmobi commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
rajatahujaatinmobi commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-647001483


   cc @Ngone51  @dongjoon-hyun Can you guys review?
   
   


----------------------------------------------------------------
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


[GitHub] [spark] AmplabJenkins commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-646990259


   Can one of the admins verify this patch?


----------------------------------------------------------------
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


[GitHub] [spark] dongjoon-hyun commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-647199097


   I left more comment on your your new PR. This issue was discussed before and technically SPARK-29465 has been tracking this as an `Improvement`. So, we cannot backport this. I'll close this first and let's gather our discussion on new PR against `master` branch.


----------------------------------------------------------------
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


[GitHub] [spark] dongjoon-hyun commented on pull request #28879: [SPARK-32039][YARN][WEBUI][2.4] Adding Check to not to set UI port (spark.ui.port) property if mentioned explicitly

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28879:
URL: https://github.com/apache/spark/pull/28879#issuecomment-647028541


   Hi, @rajatahujaatinmobi . If this is required for 3.1/3.0, please make a PR to `master` branch.


----------------------------------------------------------------
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