You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by aosagie <gi...@git.apache.org> on 2017/07/01 10:42:28 UTC

[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

GitHub user aosagie opened a pull request:

    https://github.com/apache/spark/pull/18499

    [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy all workers and applications

    ## What changes were proposed in this pull request?
    
    Currently, each application and each worker creates their own proxy servlet. Each proxy servlet is backed by its own HTTP client and a relatively large number of selector threads. This is excessive but was fixed (to an extent) by https://github.com/apache/spark/pull/18437.
    
    However, a single HTTP client (backed by a single selector thread) should be enough to handle all proxy requests. This PR creates a single proxy servlet no matter how many applications and workers there are.
    
    ## How was this patch tested?
    
    The unit tests for rewriting proxied locations and headers were updated. I then spun up a 100 node cluster to ensure that proxy'ing worked correctly and was scalable.
    
    @jiangxb1987 Please let me know if there's anything else I can do to help push this thru. Thanks!


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aosagie/spark fix/minimize-proxy-threads

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/18499.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #18499
    
----
commit 2b907df1f56e36c6265e8ada59843bb456a24943
Author: Anderson Osagie <os...@gmail.com>
Date:   2017-07-01T08:02:48Z

    [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy all workers and applications

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by ajbozarth <gi...@git.apache.org>.
Github user ajbozarth commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    I don't see any code issues, but I'm confident enough in my proxy knowledge to give this a "lgtm"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by aosagie <gi...@git.apache.org>.
Github user aosagie commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Yes, I agree with @IngoSchuster 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    thanks, merging to master!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    **[Test build #80355 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80355/testReport)** for PR 18499 at commit [`70c93b0`](https://github.com/apache/spark/commit/70c93b064c6fba4ce6afae0cca1a85340f2f811f).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/18499


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18499#discussion_r131119963
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -194,30 +194,26 @@ private[spark] object JettyUtils extends Logging {
       }
     
       /** Create a handler for proxying request to Workers and Application Drivers */
    -  def createProxyHandler(
    -      prefix: String,
    -      target: String): ServletContextHandler = {
    +  def createProxyHandler(idToUiAddress: String => Option[String]): ServletContextHandler = {
         val servlet = new ProxyServlet {
           override def rewriteTarget(request: HttpServletRequest): String = {
    -        val rewrittenURI = createProxyURI(
    -          prefix, target, request.getRequestURI(), request.getQueryString())
    -        if (rewrittenURI == null) {
    -          return null
    -        }
    -        if (!validateDestination(rewrittenURI.getHost(), rewrittenURI.getPort())) {
    -          return null
    -        }
    -        rewrittenURI.toString()
    +        val path = request.getPathInfo
    +        if (path == null) return null
    +
    +        val prefixTrailingSlashIndex = path.indexOf('/', 1)
    +        val prefix = path.substring(0,
    +          if (prefixTrailingSlashIndex == -1) path.length else prefixTrailingSlashIndex)
    --- End diff --
    
    I think we can save use a variable to save the second parameter.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by IngoSchuster <gi...@git.apache.org>.
Github user IngoSchuster commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18499#discussion_r125665745
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -194,30 +194,26 @@ private[spark] object JettyUtils extends Logging {
       }
     
       /** Create a handler for proxying request to Workers and Application Drivers */
    -  def createProxyHandler(
    -      prefix: String,
    -      target: String): ServletContextHandler = {
    +  def createProxyHandler(idToUiAddress: String => Option[String]): ServletContextHandler = {
         val servlet = new ProxyServlet {
           override def rewriteTarget(request: HttpServletRequest): String = {
    -        val rewrittenURI = createProxyURI(
    -          prefix, target, request.getRequestURI(), request.getQueryString())
    -        if (rewrittenURI == null) {
    -          return null
    -        }
    -        if (!validateDestination(rewrittenURI.getHost(), rewrittenURI.getPort())) {
    -          return null
    -        }
    -        rewrittenURI.toString()
    +        val path = request.getPathInfo
    +        if (path == null) return null
    +
    +        val prefixTrailingSlashIndex = path.indexOf('/', 1)
    +        val prefix = path.substring(0,
    +          if (prefixTrailingSlashIndex == -1) path.length else prefixTrailingSlashIndex)
    +        val id = prefix.drop(1)
    +
    +        idToUiAddress(id)
    +          .map(createProxyURI(prefix, _, path, request.getQueryString))
    +          .filter(uri => uri != null && validateDestination(uri.getHost, uri.getPort))
    +          .map(_.toString)
    +          .orNull
           }
     
           override def newHttpClient(): HttpClient = {
    -        // SPARK-21176: Use the Jetty logic to calculate the number of selector threads (#CPUs/2),
    -        // but limit it to 8 max.
    -        // Otherwise, it might happen that we exhaust the threadpool since in reverse proxy mode
    -        // a proxy is instantiated for each executor. If the head node has many processors, this
    -        // can quickly add up to an unreasonably high number of threads.
    -        val numSelectors = math.max(1, math.min(8, Runtime.getRuntime().availableProcessors() / 2))
    -        new HttpClient(new HttpClientTransportOverHTTP(numSelectors), null)
    +        new HttpClient(new HttpClientTransportOverHTTP(1), null)
    --- End diff --
    
    I fully agree @aosagie - it's much better if we don't scale the number of selector threads up with the number of executors.
    But since we don't serve simple html pages but also page(elements) that might have a higher latency than simply serving static files, I think we should not risk that the page is build up from serial requests where latency adds up - my server for example has 170ms network latency.
    I suggest that we set the number of selector threads to 8. This is certainly not excessive but still allows to serve a page with several elements in parallel.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    **[Test build #80216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80216/testReport)** for PR 18499 at commit [`45dd13c`](https://github.com/apache/spark/commit/45dd13cbe03e40721706e30eec3fe46d2cd72db8).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18499#discussion_r131085567
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -194,30 +194,26 @@ private[spark] object JettyUtils extends Logging {
       }
     
       /** Create a handler for proxying request to Workers and Application Drivers */
    -  def createProxyHandler(
    -      prefix: String,
    -      target: String): ServletContextHandler = {
    +  def createProxyHandler(idToUiAddress: String => Option[String]): ServletContextHandler = {
         val servlet = new ProxyServlet {
           override def rewriteTarget(request: HttpServletRequest): String = {
    -        val rewrittenURI = createProxyURI(
    -          prefix, target, request.getRequestURI(), request.getQueryString())
    -        if (rewrittenURI == null) {
    -          return null
    -        }
    -        if (!validateDestination(rewrittenURI.getHost(), rewrittenURI.getPort())) {
    -          return null
    -        }
    -        rewrittenURI.toString()
    +        val path = request.getPathInfo
    +        if (path == null) return null
    +
    +        val prefixTrailingSlashIndex = path.indexOf('/', 1)
    +        val prefix = path.substring(0,
    +          if (prefixTrailingSlashIndex == -1) path.length else prefixTrailingSlashIndex)
    +        val id = prefix.drop(1)
    +
    +        idToUiAddress(id)
    +          .map(createProxyURI(prefix, _, path, request.getQueryString))
    +          .filter(uri => uri != null && validateDestination(uri.getHost, uri.getPort))
    +          .map(_.toString)
    +          .orNull
    --- End diff --
    
    Could you add comments for the above code? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80355/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    **[Test build #80355 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80355/testReport)** for PR 18499 at commit [`70c93b0`](https://github.com/apache/spark/commit/70c93b064c6fba4ce6afae0cca1a85340f2f811f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Surely math.max is correct? or else the value is always 1.
    
    On Tue, Jul 4, 2017 at 3:28 PM IngoSchuster <no...@github.com>
    wrote:
    
    > Can we pull that also into 2.1.1? The original code in JettyUtils to limit
    > the number of selector threads is actually wrong.
    >
    > Instead of
    > val numSelectors = math.max(1, math.min(8,
    > Runtime.getRuntime().availableProcessors() / 2))
    > it should have been
    > val numSelectors = math.min(1, math.max(8,
    > Runtime.getRuntime().availableProcessors() / 2))
    >
    > With this PR, a fix for the bug above is no longer needed.
    >
    > —
    > You are receiving this because you are subscribed to this thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/spark/pull/18499#issuecomment-312891651>, or mute
    > the thread
    > <https://github.com/notifications/unsubscribe-auth/AAyM-hHnlELpIMQhDoh54FOG1XS5_DxCks5sKkvwgaJpZM4OLSCA>
    > .
    >



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by IngoSchuster <gi...@git.apache.org>.
Github user IngoSchuster commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18499#discussion_r125498872
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -194,30 +194,26 @@ private[spark] object JettyUtils extends Logging {
       }
     
       /** Create a handler for proxying request to Workers and Application Drivers */
    -  def createProxyHandler(
    -      prefix: String,
    -      target: String): ServletContextHandler = {
    +  def createProxyHandler(idToUiAddress: String => Option[String]): ServletContextHandler = {
         val servlet = new ProxyServlet {
           override def rewriteTarget(request: HttpServletRequest): String = {
    -        val rewrittenURI = createProxyURI(
    -          prefix, target, request.getRequestURI(), request.getQueryString())
    -        if (rewrittenURI == null) {
    -          return null
    -        }
    -        if (!validateDestination(rewrittenURI.getHost(), rewrittenURI.getPort())) {
    -          return null
    -        }
    -        rewrittenURI.toString()
    +        val path = request.getPathInfo
    +        if (path == null) return null
    +
    +        val prefixTrailingSlashIndex = path.indexOf('/', 1)
    +        val prefix = path.substring(0,
    +          if (prefixTrailingSlashIndex == -1) path.length else prefixTrailingSlashIndex)
    +        val id = prefix.drop(1)
    +
    +        idToUiAddress(id)
    +          .map(createProxyURI(prefix, _, path, request.getQueryString))
    +          .filter(uri => uri != null && validateDestination(uri.getHost, uri.getPort))
    +          .map(_.toString)
    +          .orNull
           }
     
           override def newHttpClient(): HttpClient = {
    -        // SPARK-21176: Use the Jetty logic to calculate the number of selector threads (#CPUs/2),
    -        // but limit it to 8 max.
    -        // Otherwise, it might happen that we exhaust the threadpool since in reverse proxy mode
    -        // a proxy is instantiated for each executor. If the head node has many processors, this
    -        // can quickly add up to an unreasonably high number of threads.
    -        val numSelectors = math.max(1, math.min(8, Runtime.getRuntime().availableProcessors() / 2))
    -        new HttpClient(new HttpClientTransportOverHTTP(numSelectors), null)
    +        new HttpClient(new HttpClientTransportOverHTTP(1), null)
    --- End diff --
    
    In my opinion, it's a bit extreme to just have a single selector thread. Browsers fetch elements of a page concurrently and it will help for a more responsive user experience if we serve them in parallel.
    When I reload the admin ui page, it seems as if 10+ requests are sent concurrently. For the sake of web ui latency I think we could be a bit more generous with selector threads - in particular with this great improvement that reduces the number of proxy servlets to just 1.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    retest this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/80216/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by IngoSchuster <gi...@git.apache.org>.
Github user IngoSchuster commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Can we pull that also into 2.1.1? The original code in JettyUtils to limit the number of selector threads is actually wrong.
    
    Instead of
    `  val numSelectors = math.max(1, math.min(8, Runtime.getRuntime().availableProcessors() / 2))`
    it should have been
    `  val numSelectors = math.min(1, math.max(8, Runtime.getRuntime().availableProcessors() / 2))`
    
    With this PR, a fix for the bug above is no longer needed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    **[Test build #3831 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3831/testReport)** for PR 18499 at commit [`2b907df`](https://github.com/apache/spark/commit/2b907df1f56e36c6265e8ada59843bb456a24943).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    cc @sarutak @gengliangwang  Could you look at this when you have time? Thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by aosagie <gi...@git.apache.org>.
Github user aosagie commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Hi @cloud-fan,
    Any reason the test results didn't get posted? Is there some step I need to take?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    **[Test build #3831 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3831/testReport)** for PR 18499 at commit [`2b907df`](https://github.com/apache/spark/commit/2b907df1f56e36c6265e8ada59843bb456a24943).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by gengliangwang <gi...@git.apache.org>.
Github user gengliangwang commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18499#discussion_r131118306
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -194,30 +194,26 @@ private[spark] object JettyUtils extends Logging {
       }
     
       /** Create a handler for proxying request to Workers and Application Drivers */
    -  def createProxyHandler(
    -      prefix: String,
    -      target: String): ServletContextHandler = {
    +  def createProxyHandler(idToUiAddress: String => Option[String]): ServletContextHandler = {
         val servlet = new ProxyServlet {
           override def rewriteTarget(request: HttpServletRequest): String = {
    -        val rewrittenURI = createProxyURI(
    -          prefix, target, request.getRequestURI(), request.getQueryString())
    -        if (rewrittenURI == null) {
    -          return null
    -        }
    -        if (!validateDestination(rewrittenURI.getHost(), rewrittenURI.getPort())) {
    -          return null
    -        }
    -        rewrittenURI.toString()
    +        val path = request.getPathInfo
    +        if (path == null) return null
    +
    +        val prefixTrailingSlashIndex = path.indexOf('/', 1)
    +        val prefix = path.substring(0,
    +          if (prefixTrailingSlashIndex == -1) path.length else prefixTrailingSlashIndex)
    +        val id = prefix.drop(1)
    +
    +        idToUiAddress(id)
    +          .map(createProxyURI(prefix, _, path, request.getQueryString))
    +          .filter(uri => uri != null && validateDestination(uri.getHost, uri.getPort))
    +          .map(_.toString)
    +          .orNull
           }
     
           override def newHttpClient(): HttpClient = {
    -        // SPARK-21176: Use the Jetty logic to calculate the number of selector threads (#CPUs/2),
    -        // but limit it to 8 max.
    -        // Otherwise, it might happen that we exhaust the threadpool since in reverse proxy mode
    -        // a proxy is instantiated for each executor. If the head node has many processors, this
    -        // can quickly add up to an unreasonably high number of threads.
    -        val numSelectors = math.max(1, math.min(8, Runtime.getRuntime().availableProcessors() / 2))
    -        new HttpClient(new HttpClientTransportOverHTTP(numSelectors), null)
    +        new HttpClient(new HttpClientTransportOverHTTP(8), null)
    --- End diff --
    
    I think we can just keep the previous code here. If the number of available processors is smaller than 8, e.g. only one processor,  we don't need to make it 8 threads.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet t...

Posted by aosagie <gi...@git.apache.org>.
Github user aosagie commented on a diff in the pull request:

    https://github.com/apache/spark/pull/18499#discussion_r125512070
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -194,30 +194,26 @@ private[spark] object JettyUtils extends Logging {
       }
     
       /** Create a handler for proxying request to Workers and Application Drivers */
    -  def createProxyHandler(
    -      prefix: String,
    -      target: String): ServletContextHandler = {
    +  def createProxyHandler(idToUiAddress: String => Option[String]): ServletContextHandler = {
         val servlet = new ProxyServlet {
           override def rewriteTarget(request: HttpServletRequest): String = {
    -        val rewrittenURI = createProxyURI(
    -          prefix, target, request.getRequestURI(), request.getQueryString())
    -        if (rewrittenURI == null) {
    -          return null
    -        }
    -        if (!validateDestination(rewrittenURI.getHost(), rewrittenURI.getPort())) {
    -          return null
    -        }
    -        rewrittenURI.toString()
    +        val path = request.getPathInfo
    +        if (path == null) return null
    +
    +        val prefixTrailingSlashIndex = path.indexOf('/', 1)
    +        val prefix = path.substring(0,
    +          if (prefixTrailingSlashIndex == -1) path.length else prefixTrailingSlashIndex)
    +        val id = prefix.drop(1)
    +
    +        idToUiAddress(id)
    +          .map(createProxyURI(prefix, _, path, request.getQueryString))
    +          .filter(uri => uri != null && validateDestination(uri.getHost, uri.getPort))
    +          .map(_.toString)
    +          .orNull
           }
     
           override def newHttpClient(): HttpClient = {
    -        // SPARK-21176: Use the Jetty logic to calculate the number of selector threads (#CPUs/2),
    -        // but limit it to 8 max.
    -        // Otherwise, it might happen that we exhaust the threadpool since in reverse proxy mode
    -        // a proxy is instantiated for each executor. If the head node has many processors, this
    -        // can quickly add up to an unreasonably high number of threads.
    -        val numSelectors = math.max(1, math.min(8, Runtime.getRuntime().availableProcessors() / 2))
    -        new HttpClient(new HttpClientTransportOverHTTP(numSelectors), null)
    +        new HttpClient(new HttpClientTransportOverHTTP(1), null)
    --- End diff --
    
    Thanks for the feedback @IngoSchuster 
    I can go with whichever number people find best. My main goal with this patch was to not have that number grow with the number of workers and applications (since my org runs a lot of applications and ran into issues with Master not being responsive).
    
    From my research, I found the following quote from a Jetty committer: "For 5k clients for normal HTML pages you can easily go by with 1 selector." (See: https://dev.eclipse.org/mhonarc/lists/jetty-users/msg04751.html). But, this was in the context of a server and not necessarily a client.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by IngoSchuster <gi...@git.apache.org>.
Github user IngoSchuster commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    You are right, a short moment of confusion - already deleted my comment... 
    ;-)
    Please disregard it
    
    
    Regards,
    Ingo Schuster
    
    Tel: +49-(0)7031-16-5156
    
    IBM Analytics
    ----------------------------------------------------------------------------
    IBM Deutschland Research & Development GmbH / Vorsitzende des 
    Aufsichtsrats: Martina Koederitz
    Geschäftsführung: Dirk Wittkopp
    Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, 
    HRB 243294 
    
    
    
    
    From:   Sean Owen <no...@github.com>
    To:     apache/spark <sp...@noreply.github.com>
    Cc:     IngoSchuster <in...@de.ibm.com>, Comment 
    <co...@noreply.github.com>
    Date:   04/07/2017 17:11
    Subject:        Re: [apache/spark] [SPARK-21176][WEB UI] Use a single 
    ProxyServlet to proxy all workers and applications (#18499)
    
    
    
    Surely math.max is correct? or else the value is always 1.
    
    On Tue, Jul 4, 2017 at 3:28 PM IngoSchuster <no...@github.com>
    wrote:
    
    > Can we pull that also into 2.1.1? The original code in JettyUtils to 
    limit
    > the number of selector threads is actually wrong.
    >
    > Instead of
    > val numSelectors = math.max(1, math.min(8,
    > Runtime.getRuntime().availableProcessors() / 2))
    > it should have been
    > val numSelectors = math.min(1, math.max(8,
    > Runtime.getRuntime().availableProcessors() / 2))
    >
    > With this PR, a fix for the bug above is no longer needed.
    >
    > ?
    > You are receiving this because you are subscribed to this thread.
    > Reply to this email directly, view it on GitHub
    > <https://github.com/apache/spark/pull/18499#issuecomment-312891651>, or 
    mute
    > the thread
    > <
    https://github.com/notifications/unsubscribe-auth/AAyM-hHnlELpIMQhDoh54FOG1XS5_DxCks5sKkvwgaJpZM4OLSCA
    >
    > .
    >
    ?
    You are receiving this because you commented.
    Reply to this email directly, view it on GitHub, or mute the thread.
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    OK to test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by aosagie <gi...@git.apache.org>.
Github user aosagie commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    @cloud-fan Hi, I just pushed a change to up the selector threads from 1 to 8. Can I get a retest please?
    
    @jiangxb1987 Sorry to bother, but is there anyone available to give guidance on what I need to do to push this PR forward? This bug renders the master completely unresponsive for us in production.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by aosagie <gi...@git.apache.org>.
Github user aosagie commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Hey @ajbozarth. Any chance you could provide a review or some guidance on anything I can do to make this PR more amenable?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    **[Test build #80216 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/80216/testReport)** for PR 18499 at commit [`45dd13c`](https://github.com/apache/spark/commit/45dd13cbe03e40721706e30eec3fe46d2cd72db8).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by jiangxb1987 <gi...@git.apache.org>.
Github user jiangxb1987 commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    cc @gurvindersingh Could you look at this please?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    @aosagie  do we still need the previous fix for SPARK-21176?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    cc @jiangxb1987 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18499: [SPARK-21176][WEB UI] Use a single ProxyServlet to proxy...

Posted by IngoSchuster <gi...@git.apache.org>.
Github user IngoSchuster commented on the issue:

    https://github.com/apache/spark/pull/18499
  
    Yes I believe it still makes sense to limit the number of selector threads. Jetty's default is tuned for a real web server and by default it creates (number of cpus / 2) selector threads. Right now we have limited the number of threads to 8 which I believe is more than enough to serve the spark admin UI.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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