From 086a4f62b95b455e9e789ea820b0e1f8b7e8d643 Mon Sep 17 00:00:00 2001 From: Marco Gaido Date: Sun, 10 Jun 2018 12:44:01 +0200 Subject: [PATCH 1/4] [SPARK-24506][UI] Add UI filters also to thriftserver tab --- core/src/main/scala/org/apache/spark/ui/WebUI.scala | 2 ++ .../spark/sql/hive/thriftserver/ui/ThriftServerTab.scala | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/org/apache/spark/ui/WebUI.scala b/core/src/main/scala/org/apache/spark/ui/WebUI.scala index 8b75f5d8fe1a..9fc958e700f4 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -59,6 +59,8 @@ private[spark] abstract class WebUI( def getTabs: Seq[WebUITab] = tabs def getHandlers: Seq[ServletContextHandler] = handlers def getSecurityManager: SecurityManager = securityManager + def handlersForPage(page: WebUIPage): Seq[ServletContextHandler] = + pageToHandlers.getOrElse(page, Seq.empty) /** Attach a tab to this UI, along with all of its attached pages. */ def attachTab(tab: WebUITab) { diff --git a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala index db2066009b35..93041f3bb60c 100644 --- a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala +++ b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala @@ -21,7 +21,7 @@ import org.apache.spark.{SparkContext, SparkException} import org.apache.spark.internal.Logging import org.apache.spark.sql.hive.thriftserver.HiveThriftServer2 import org.apache.spark.sql.hive.thriftserver.ui.ThriftServerTab._ -import org.apache.spark.ui.{SparkUI, SparkUITab} +import org.apache.spark.ui.{JettyUtils, SparkUI, SparkUITab} /** * Spark Web UI tab that shows statistics of jobs running in the thrift server. @@ -39,6 +39,10 @@ private[thriftserver] class ThriftServerTab(sparkContext: SparkContext) attachPage(new ThriftServerSessionPage(this)) parent.attachTab(this) + // We need to add the filters to the handlers generated here since the SparkUI has been already + // started. + JettyUtils.addFilters(this.pages.flatMap(parent.handlersForPage), parent.conf) + def detach() { getSparkUI(sparkContext).detachTab(this) } From 0ac1848efcb5d95ea3b3ab29aca8ff14dcc17dd2 Mon Sep 17 00:00:00 2001 From: Marco Gaido Date: Mon, 11 Jun 2018 14:33:26 +0200 Subject: [PATCH 2/4] address comments --- .../org/apache/spark/deploy/history/HistoryServer.scala | 1 - core/src/main/scala/org/apache/spark/ui/JettyUtils.scala | 2 +- core/src/main/scala/org/apache/spark/ui/WebUI.scala | 7 ++++++- .../spark/sql/hive/thriftserver/ui/ThriftServerTab.scala | 4 ---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala b/core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala index a9a4d5a4ec6a..066275e8f842 100644 --- a/core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala +++ b/core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala @@ -152,7 +152,6 @@ class HistoryServer( assert(serverInfo.isDefined, "HistoryServer must be bound before attaching SparkUIs") handlers.synchronized { ui.getHandlers.foreach(attachHandler) - addFilters(ui.getHandlers, conf) } } diff --git a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala index d6a025a6f12d..252b8da22528 100644 --- a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala +++ b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala @@ -263,7 +263,7 @@ private[spark] object JettyUtils extends Logging { filters.foreach { case filter : String => if (!filter.isEmpty) { - logInfo("Adding filter: " + filter) + logInfo(s"Adding filter $filter to ${handlers.map(_.getContextPath).mkString(", ")}.") val holder : FilterHolder = new FilterHolder() holder.setClassName(filter) // Get any parameters for each filter diff --git a/core/src/main/scala/org/apache/spark/ui/WebUI.scala b/core/src/main/scala/org/apache/spark/ui/WebUI.scala index 9fc958e700f4..31ffcb90026c 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -93,7 +93,12 @@ private[spark] abstract class WebUI( /** Attach a handler to this UI. */ def attachHandler(handler: ServletContextHandler) { handlers += handler - serverInfo.foreach(_.addHandler(handler)) + serverInfo.foreach { sInfo => + sInfo.addHandler(handler) + // If the UI has already been bound, we need to add the filters to the newly attached + // handlers. Otherwise, they will be attached when binding. + addFilters(Seq(handler), conf) + } } /** Detach a handler from this UI. */ diff --git a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala index 93041f3bb60c..7118dc25f913 100644 --- a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala +++ b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala @@ -39,10 +39,6 @@ private[thriftserver] class ThriftServerTab(sparkContext: SparkContext) attachPage(new ThriftServerSessionPage(this)) parent.attachTab(this) - // We need to add the filters to the handlers generated here since the SparkUI has been already - // started. - JettyUtils.addFilters(this.pages.flatMap(parent.handlersForPage), parent.conf) - def detach() { getSparkUI(sparkContext).detachTab(this) } From 08cd576f71b9ed4e7f6af62c2d90b3a650f6d82c Mon Sep 17 00:00:00 2001 From: Marco Gaido Date: Mon, 11 Jun 2018 14:41:37 +0200 Subject: [PATCH 3/4] remove unneeded changes --- core/src/main/scala/org/apache/spark/ui/WebUI.scala | 2 -- .../apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/ui/WebUI.scala b/core/src/main/scala/org/apache/spark/ui/WebUI.scala index 31ffcb90026c..b4e125d77ded 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -59,8 +59,6 @@ private[spark] abstract class WebUI( def getTabs: Seq[WebUITab] = tabs def getHandlers: Seq[ServletContextHandler] = handlers def getSecurityManager: SecurityManager = securityManager - def handlersForPage(page: WebUIPage): Seq[ServletContextHandler] = - pageToHandlers.getOrElse(page, Seq.empty) /** Attach a tab to this UI, along with all of its attached pages. */ def attachTab(tab: WebUITab) { diff --git a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala index 7118dc25f913..db2066009b35 100644 --- a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala +++ b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerTab.scala @@ -21,7 +21,7 @@ import org.apache.spark.{SparkContext, SparkException} import org.apache.spark.internal.Logging import org.apache.spark.sql.hive.thriftserver.HiveThriftServer2 import org.apache.spark.sql.hive.thriftserver.ui.ThriftServerTab._ -import org.apache.spark.ui.{JettyUtils, SparkUI, SparkUITab} +import org.apache.spark.ui.{SparkUI, SparkUITab} /** * Spark Web UI tab that shows statistics of jobs running in the thrift server. From 02a9f4e444b3c587733dde08560ed078ff4f3c50 Mon Sep 17 00:00:00 2001 From: Marco Gaido Date: Tue, 12 Jun 2018 15:51:01 +0200 Subject: [PATCH 4/4] address comments --- core/src/main/scala/org/apache/spark/ui/JettyUtils.scala | 6 ++++-- core/src/main/scala/org/apache/spark/ui/WebUI.scala | 7 +------ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala index 252b8da22528..52a955111231 100644 --- a/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala +++ b/core/src/main/scala/org/apache/spark/ui/JettyUtils.scala @@ -407,7 +407,7 @@ private[spark] object JettyUtils extends Logging { } pool.setMaxThreads(math.max(pool.getMaxThreads, minThreads)) - ServerInfo(server, httpPort, securePort, collection) + ServerInfo(server, httpPort, securePort, conf, collection) } catch { case e: Exception => server.stop() @@ -507,10 +507,12 @@ private[spark] case class ServerInfo( server: Server, boundPort: Int, securePort: Option[Int], + conf: SparkConf, private val rootHandler: ContextHandlerCollection) { - def addHandler(handler: ContextHandler): Unit = { + def addHandler(handler: ServletContextHandler): Unit = { handler.setVirtualHosts(JettyUtils.toVirtualHosts(JettyUtils.SPARK_CONNECTOR_NAME)) + JettyUtils.addFilters(Seq(handler), conf) rootHandler.addHandler(handler) if (!handler.isStarted()) { handler.start() diff --git a/core/src/main/scala/org/apache/spark/ui/WebUI.scala b/core/src/main/scala/org/apache/spark/ui/WebUI.scala index b4e125d77ded..8b75f5d8fe1a 100644 --- a/core/src/main/scala/org/apache/spark/ui/WebUI.scala +++ b/core/src/main/scala/org/apache/spark/ui/WebUI.scala @@ -91,12 +91,7 @@ private[spark] abstract class WebUI( /** Attach a handler to this UI. */ def attachHandler(handler: ServletContextHandler) { handlers += handler - serverInfo.foreach { sInfo => - sInfo.addHandler(handler) - // If the UI has already been bound, we need to add the filters to the newly attached - // handlers. Otherwise, they will be attached when binding. - addFilters(Seq(handler), conf) - } + serverInfo.foreach(_.addHandler(handler)) } /** Detach a handler from this UI. */