You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tianyi <gi...@git.apache.org> on 2015/04/28 03:40:20 UTC

[GitHub] spark pull request: add webui for thriftserver

GitHub user tianyi opened a pull request:

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

    add webui for thriftserver

    This PR is mainly focused on creating an independent tab for the thrift server in spark web UI.
    Features:
    1 Session related statistics ( username and IP are only supported in hive-0.13.1 )
    2 List all the SQL executing or executed on this server
    3 Provide links to the job generated by SQL
    4 Provide link to show all SQL executing or executed in a specified session
    
    Prototype snapshots:
    
    This is the main page for thrift server
    ![image](https://cloud.githubusercontent.com/assets/1411869/7361379/df7dcc64-ed89-11e4-9964-4df0b32f475e.png)
    
    
    


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

    $ git pull https://github.com/tianyi/spark SPARK-5100

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

    https://github.com/apache/spark/pull/5730.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 #5730
    
----
commit 98301998dd9faa28a5679bbdb4075e57dccf4c9a
Author: tianyi <ti...@gmail.com>
Date:   2015-04-23T05:33:35Z

    add webui for thriftserver

----


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211143
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    --- End diff --
    
    According to [this section] [1] of Databricks Scala Guide, I think we prefer `System.nanoTime` to do timing.
    
    [1]: https://github.com/databricks/scala-style-guide#prefer-nanotime-over-currenttimemillis


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96956692
  
      [Test build #31129 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31129/consoleFull) for   PR 5730 at commit [`8809157`](https://github.com/apache/spark/commit/8809157c3d37c524c029b0e4d81eec60e499c967).


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29213643
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerPage.scala ---
    @@ -0,0 +1,189 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.hive.thriftserver.ui
    +
    +import java.util.Calendar
    +import javax.servlet.http.HttpServletRequest
    +
    +import org.apache.commons.lang3.StringEscapeUtils
    +import org.apache.spark.Logging
    +import org.apache.spark.sql.hive.thriftserver.HiveThriftServer2.{SessionInfo, ExecutionState, ExecutionInfo}
    +import org.apache.spark.ui.UIUtils._
    +import org.apache.spark.ui._
    +
    +import scala.xml.Node
    +
    +/** Page for Spark Web UI that shows statistics of a streaming job */
    +private[ui] class ThriftServerPage(parent: ThriftServerTab) extends WebUIPage("") with Logging {
    +
    +  private val listener = parent.listener
    +  private val startTime = Calendar.getInstance().getTime()
    +  private val emptyCell = "-"
    +
    +  /** Render the page */
    +  def render(request: HttpServletRequest): Seq[Node] = {
    +    val content =
    +      generateBasicStats() ++
    +      <br/> ++
    +      <h4>
    +        Total {listener.sessionList.size} session online,
    +        Total {listener.totalRunning} sql running
    --- End diff --
    
    ```scala
    {listener.sessionList.size} session(s) are online,
    running {listener.totalRunning) SQL statement(s)
    ```


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211406
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    --- End diff --
    
    Please remove the `;` after changing the outer `()` to `{}`.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96965368
  
      [Test build #31129 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31129/consoleFull) for   PR 5730 at commit [`8809157`](https://github.com/apache/spark/commit/8809157c3d37c524c029b0e4d81eec60e499c967).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29213610
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerPage.scala ---
    @@ -0,0 +1,189 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.hive.thriftserver.ui
    +
    +import java.util.Calendar
    +import javax.servlet.http.HttpServletRequest
    +
    +import org.apache.commons.lang3.StringEscapeUtils
    +import org.apache.spark.Logging
    +import org.apache.spark.sql.hive.thriftserver.HiveThriftServer2.{SessionInfo, ExecutionState, ExecutionInfo}
    +import org.apache.spark.ui.UIUtils._
    +import org.apache.spark.ui._
    +
    +import scala.xml.Node
    +
    +/** Page for Spark Web UI that shows statistics of a streaming job */
    +private[ui] class ThriftServerPage(parent: ThriftServerTab) extends WebUIPage("") with Logging {
    +
    +  private val listener = parent.listener
    +  private val startTime = Calendar.getInstance().getTime()
    +  private val emptyCell = "-"
    +
    +  /** Render the page */
    +  def render(request: HttpServletRequest): Seq[Node] = {
    +    val content =
    +      generateBasicStats() ++
    +      <br/> ++
    +      <h4>
    +        Total {listener.sessionList.size} session online,
    --- End diff --
    
    "session online" => "session(s) online"


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211781
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    +    }
    +
    +    def onSessionCreated(ip: String, sessionId: String, userName: String = "UNKNOWN"): Unit = {
    +      val info = new SessionInfo(sessionId, System.currentTimeMillis, ip, userName)
    +      sessionList(sessionId) = info
    +      trimSessionIfNecessary
    +    }
    +
    +    def onSessionClosed(sessionId: String): Unit = {
    +      sessionList(sessionId).finishTimestamp = System.currentTimeMillis
    +    }
    +
    +    def onStatementStart(
    +        id: String,
    +        sessionId: String,
    +        statement: String,
    +        groupId: String,
    +        userName: String = "UNKNOWN"): Unit = {
    +      val info = new ExecutionInfo(statement, sessionId, System.currentTimeMillis, userName)
    +      info.state = ExecutionState.STARTED
    +      executeList(id) = info
    +      trimExecutionIfNecessary
    +      sessionList(sessionId).totalExecute += 1
    +      executeList(id).groupId = groupId
    +      totalRunning += 1
    +    }
    +
    +    def onStatementParse(id: String, executePlan: String): Unit = {
    +      executeList(id).executePlan = executePlan
    +      executeList(id).state = ExecutionState.COMPILED
    +    }
    +
    +    def onStatementError(id: String, errorMessage: String, errorTrace: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).detail = errorMessage
    +      executeList(id).state = ExecutionState.FAILED
    +      totalRunning -= 1
    +    }
    +
    +    def onStatementFinish(id: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).state = ExecutionState.FINISHED
    +      totalRunning -= 1
    +    }
    +
    +    private def trimExecutionIfNecessary = synchronized {
    +      if (executeList.size > retainedStatements) {
    +        val toRemove = math.max(retainedStatements / 10, 1)
    +        executeList.toList.sortBy(_._2.startTimestamp).take(toRemove).foreach { s =>
    +          executeList.remove(s._1)
    +        }
    +      }
    +    }
    +
    +    private def trimSessionIfNecessary = synchronized {
    --- End diff --
    
    Append `()` to the method name since it causes side effect.


---
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: [SPARK-5100][SQL]add webui for thriftserver

Posted by tianyi <gi...@git.apache.org>.
Github user tianyi commented on the pull request:

    https://github.com/apache/spark/pull/5730#issuecomment-97374381
  
    @liancheng The test codes and  is ready to review. 


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97319612
  
    Merged build started.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96895450
  
      [Test build #31097 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31097/consoleFull) for   PR 5730 at commit [`9830199`](https://github.com/apache/spark/commit/98301998dd9faa28a5679bbdb4075e57dccf4c9a).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  class HiveThriftServer2Listener(`
    
     * This patch does not change any dependencies.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29214768
  
    --- Diff: sql/hive-thriftserver/v0.13.1/src/main/scala/org/apache/spark/sql/hive/thriftserver/Shim13.scala ---
    @@ -227,11 +236,14 @@ private[hive] class SparkSQLSessionManager(hiveContext: HiveContext)
           withImpersonation: Boolean,
           delegationToken: String): SessionHandle = {
         hiveContext.openSession()
    -
    -    super.openSession(protocol, username, passwd, sessionConf, withImpersonation, delegationToken)
    +    val sessionHandle = super.openSession(protocol, username, passwd, sessionConf, withImpersonation, delegationToken)
    --- End diff --
    
    100 columns exceeded.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97371745
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31256/
    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 pull request: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211717
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    --- End diff --
    
    I think the whole thing can be simplified to:
    
    ```scala
    for {
      props <- Option(jobStart.properties)
      groupId <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
      (_, info) <- executeList if info.groupId == groupId
    } {
      info.jobId += jobStart.jobId.toString
      info.groupId = groupId
    }
    ```


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97319569
  
     Merged build triggered.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97328481
  
      [Test build #31256 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31256/consoleFull) for   PR 5730 at commit [`cfd14c7`](https://github.com/apache/spark/commit/cfd14c7d44a9e0781ae79ed84034f4e4eea49b73).


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211340
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    --- End diff --
    
    `executionList`?


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29213358
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerPage.scala ---
    @@ -0,0 +1,189 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.hive.thriftserver.ui
    +
    +import java.util.Calendar
    +import javax.servlet.http.HttpServletRequest
    +
    +import org.apache.commons.lang3.StringEscapeUtils
    +import org.apache.spark.Logging
    +import org.apache.spark.sql.hive.thriftserver.HiveThriftServer2.{SessionInfo, ExecutionState, ExecutionInfo}
    +import org.apache.spark.ui.UIUtils._
    +import org.apache.spark.ui._
    +
    +import scala.xml.Node
    --- End diff --
    
    Import order should be `java`, `javax`, `scala`, and then others.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97328328
  
     Merged build triggered.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97371741
  
    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 pull request: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96968035
  
      [Test build #31132 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31132/consoleFull) for   PR 5730 at commit [`aa20408`](https://github.com/apache/spark/commit/aa20408837af0b31b55ec097e3df2e0e51e6217b).


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97328344
  
    Merged build started.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29212930
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    +    }
    +
    +    def onSessionCreated(ip: String, sessionId: String, userName: String = "UNKNOWN"): Unit = {
    +      val info = new SessionInfo(sessionId, System.currentTimeMillis, ip, userName)
    +      sessionList(sessionId) = info
    +      trimSessionIfNecessary
    +    }
    +
    +    def onSessionClosed(sessionId: String): Unit = {
    +      sessionList(sessionId).finishTimestamp = System.currentTimeMillis
    +    }
    +
    +    def onStatementStart(
    +        id: String,
    +        sessionId: String,
    +        statement: String,
    +        groupId: String,
    +        userName: String = "UNKNOWN"): Unit = {
    +      val info = new ExecutionInfo(statement, sessionId, System.currentTimeMillis, userName)
    +      info.state = ExecutionState.STARTED
    +      executeList(id) = info
    +      trimExecutionIfNecessary
    +      sessionList(sessionId).totalExecute += 1
    +      executeList(id).groupId = groupId
    +      totalRunning += 1
    +    }
    +
    +    def onStatementParse(id: String, executePlan: String): Unit = {
    --- End diff --
    
    Maybe `onStatementCompiled`? Also, `executePlan` should be `executionPlan`.


---
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: [SPARK-5100][SQL]add webui for thriftserver

Posted by tianyi <gi...@git.apache.org>.
Github user tianyi commented on the pull request:

    https://github.com/apache/spark/pull/5730#issuecomment-96956131
  
    @JoshRosen I just add some test for thrift server UI. But I'm not sure if I need to add test cases for all the feature. Any suggestion?


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96963419
  
      [Test build #31128 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31128/consoleFull) for   PR 5730 at commit [`c9df6f9`](https://github.com/apache/spark/commit/c9df6f9f178c053713cc8ec34f9120eb4b86072c).
     * This patch **fails RAT tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211137
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    --- End diff --
    
    `totalExecution`?


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96955450
  
      [Test build #31128 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31128/consoleFull) for   PR 5730 at commit [`c9df6f9`](https://github.com/apache/spark/commit/c9df6f9f178c053713cc8ec34f9120eb4b86072c).


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29212594
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    +    }
    +
    +    def onSessionCreated(ip: String, sessionId: String, userName: String = "UNKNOWN"): Unit = {
    +      val info = new SessionInfo(sessionId, System.currentTimeMillis, ip, userName)
    +      sessionList(sessionId) = info
    +      trimSessionIfNecessary
    +    }
    +
    +    def onSessionClosed(sessionId: String): Unit = {
    +      sessionList(sessionId).finishTimestamp = System.currentTimeMillis
    +    }
    +
    +    def onStatementStart(
    +        id: String,
    +        sessionId: String,
    +        statement: String,
    +        groupId: String,
    +        userName: String = "UNKNOWN"): Unit = {
    +      val info = new ExecutionInfo(statement, sessionId, System.currentTimeMillis, userName)
    +      info.state = ExecutionState.STARTED
    +      executeList(id) = info
    +      trimExecutionIfNecessary
    +      sessionList(sessionId).totalExecute += 1
    +      executeList(id).groupId = groupId
    +      totalRunning += 1
    +    }
    +
    +    def onStatementParse(id: String, executePlan: String): Unit = {
    +      executeList(id).executePlan = executePlan
    +      executeList(id).state = ExecutionState.COMPILED
    +    }
    +
    +    def onStatementError(id: String, errorMessage: String, errorTrace: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).detail = errorMessage
    +      executeList(id).state = ExecutionState.FAILED
    +      totalRunning -= 1
    +    }
    +
    +    def onStatementFinish(id: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).state = ExecutionState.FINISHED
    +      totalRunning -= 1
    +    }
    +
    +    private def trimExecutionIfNecessary = synchronized {
    +      if (executeList.size > retainedStatements) {
    +        val toRemove = math.max(retainedStatements / 10, 1)
    +        executeList.toList.sortBy(_._2.startTimestamp).take(toRemove).foreach { s =>
    +          executeList.remove(s._1)
    +        }
    +      }
    +    }
    +
    +    private def trimSessionIfNecessary = synchronized {
    --- End diff --
    
    Same as above.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211295
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    --- End diff --
    
    Sorry, didn't notice that we need to display wall time in the web UI, thus `System.nanoTime` isn't proper here.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211792
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    +    }
    +
    +    def onSessionCreated(ip: String, sessionId: String, userName: String = "UNKNOWN"): Unit = {
    +      val info = new SessionInfo(sessionId, System.currentTimeMillis, ip, userName)
    +      sessionList(sessionId) = info
    +      trimSessionIfNecessary
    +    }
    +
    +    def onSessionClosed(sessionId: String): Unit = {
    +      sessionList(sessionId).finishTimestamp = System.currentTimeMillis
    +    }
    +
    +    def onStatementStart(
    +        id: String,
    +        sessionId: String,
    +        statement: String,
    +        groupId: String,
    +        userName: String = "UNKNOWN"): Unit = {
    +      val info = new ExecutionInfo(statement, sessionId, System.currentTimeMillis, userName)
    +      info.state = ExecutionState.STARTED
    +      executeList(id) = info
    +      trimExecutionIfNecessary
    +      sessionList(sessionId).totalExecute += 1
    +      executeList(id).groupId = groupId
    +      totalRunning += 1
    +    }
    +
    +    def onStatementParse(id: String, executePlan: String): Unit = {
    +      executeList(id).executePlan = executePlan
    +      executeList(id).state = ExecutionState.COMPILED
    +    }
    +
    +    def onStatementError(id: String, errorMessage: String, errorTrace: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).detail = errorMessage
    +      executeList(id).state = ExecutionState.FAILED
    +      totalRunning -= 1
    +    }
    +
    +    def onStatementFinish(id: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).state = ExecutionState.FINISHED
    +      totalRunning -= 1
    +    }
    +
    +    private def trimExecutionIfNecessary = synchronized {
    --- End diff --
    
    Append `()` to the method name since it causes side effect.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97371709
  
      [Test build #31256 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31256/consoleFull) for   PR 5730 at commit [`cfd14c7`](https://github.com/apache/spark/commit/cfd14c7d44a9e0781ae79ed84034f4e4eea49b73).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96988229
  
      [Test build #31132 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31132/consoleFull) for   PR 5730 at commit [`aa20408`](https://github.com/apache/spark/commit/aa20408837af0b31b55ec097e3df2e0e51e6217b).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29214742
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/ui/ThriftServerSessionPage.scala ---
    @@ -0,0 +1,197 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.sql.hive.thriftserver.ui
    +
    +import java.util.Calendar
    +import javax.servlet.http.HttpServletRequest
    +
    +import org.apache.commons.lang3.StringEscapeUtils
    +import org.apache.spark.Logging
    +import org.apache.spark.sql.hive.thriftserver.HiveThriftServer2.{ExecutionInfo, ExecutionState}
    +import org.apache.spark.ui.UIUtils._
    +import org.apache.spark.ui._
    +
    +import scala.xml.Node
    --- End diff --
    
    Import order is wrong here.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97324367
  
    Merged build finished. Test FAILed.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-96871978
  
      [Test build #31097 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31097/consoleFull) for   PR 5730 at commit [`9830199`](https://github.com/apache/spark/commit/98301998dd9faa28a5679bbdb4075e57dccf4c9a).


---
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: [SPARK-5100][SQL]add webui for thriftserver

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/5730#issuecomment-96913392
  
    This generally looks good, left some comments, most are about styling issues.
    
    @JoshRosen Would you mind to double check the web UI changes? This should be generally the same as #3946. Thanks! @tianyi will add Selenium based test cases later.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97324366
  
      [Test build #31254 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31254/consoleFull) for   PR 5730 at commit [`0efe3d5`](https://github.com/apache/spark/commit/0efe3d525bd9865dedb008010d6e37d3161cc563).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.
     * This patch does not change any dependencies.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97319823
  
      [Test build #31254 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31254/consoleFull) for   PR 5730 at commit [`0efe3d5`](https://github.com/apache/spark/commit/0efe3d525bd9865dedb008010d6e37d3161cc563).


---
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: [SPARK-5100][SQL]add webui for thriftserver

Posted by WangTaoTheTonic <gi...@git.apache.org>.
Github user WangTaoTheTonic commented on the pull request:

    https://github.com/apache/spark/pull/5730#issuecomment-96902704
  
    Sorry for not follow the previous comments but could you tell the difference between this tab and Thrift Server's driver UI?
    Once this is added, will Thrift Server's Driver UI still be reserved?


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211337
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    --- End diff --
    
    We probably should move these two configurations to `SQLConf` and rename them to `spark.sql.thriftserver.ui.xxx`.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211457
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    --- End diff --
    
    Please always use
    
    ```scala
    collection.map { element =>
      ...
    }
    ```
    
    instead of
    
    ```scala
    collection.map(element => {
      ...
    })
    ```


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211392
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    --- End diff --
    
    NIT: We usually prefer `for { ... }` when the `for` expression is broken into multiple lines.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29212588
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    +        val ret = executeList.find( _ match {
    +          case (id: String, info: ExecutionInfo) => info.groupId == groupId
    +        })
    +        if (ret.isDefined) {
    +          ret.get._2.jobId += jobStart.jobId.toString
    +          ret.get._2.groupId = groupId
    +        }
    +      })
    +    }
    +
    +    def onSessionCreated(ip: String, sessionId: String, userName: String = "UNKNOWN"): Unit = {
    +      val info = new SessionInfo(sessionId, System.currentTimeMillis, ip, userName)
    +      sessionList(sessionId) = info
    +      trimSessionIfNecessary
    +    }
    +
    +    def onSessionClosed(sessionId: String): Unit = {
    +      sessionList(sessionId).finishTimestamp = System.currentTimeMillis
    +    }
    +
    +    def onStatementStart(
    +        id: String,
    +        sessionId: String,
    +        statement: String,
    +        groupId: String,
    +        userName: String = "UNKNOWN"): Unit = {
    +      val info = new ExecutionInfo(statement, sessionId, System.currentTimeMillis, userName)
    +      info.state = ExecutionState.STARTED
    +      executeList(id) = info
    +      trimExecutionIfNecessary
    +      sessionList(sessionId).totalExecute += 1
    +      executeList(id).groupId = groupId
    +      totalRunning += 1
    +    }
    +
    +    def onStatementParse(id: String, executePlan: String): Unit = {
    +      executeList(id).executePlan = executePlan
    +      executeList(id).state = ExecutionState.COMPILED
    +    }
    +
    +    def onStatementError(id: String, errorMessage: String, errorTrace: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).detail = errorMessage
    +      executeList(id).state = ExecutionState.FAILED
    +      totalRunning -= 1
    +    }
    +
    +    def onStatementFinish(id: String): Unit = {
    +      executeList(id).finishTimestamp = System.currentTimeMillis
    +      executeList(id).state = ExecutionState.FINISHED
    +      totalRunning -= 1
    +    }
    +
    +    private def trimExecutionIfNecessary = synchronized {
    --- End diff --
    
    I guess it would make more sense to replace this method with something like `insertExecution`, which handles both inserting a new element and retiring the eldest element.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29214771
  
    --- Diff: sql/hive-thriftserver/v0.13.1/src/main/scala/org/apache/spark/sql/hive/thriftserver/Shim13.scala ---
    @@ -227,11 +236,14 @@ private[hive] class SparkSQLSessionManager(hiveContext: HiveContext)
           withImpersonation: Boolean,
           delegationToken: String): SessionHandle = {
         hiveContext.openSession()
    -
    -    super.openSession(protocol, username, passwd, sessionConf, withImpersonation, delegationToken)
    +    val sessionHandle = super.openSession(protocol, username, passwd, sessionConf, withImpersonation, delegationToken)
    +    val session = super.getSession(sessionHandle)
    +    HiveThriftServer2.listener.onSessionCreated(session.getIpAddress, sessionHandle.getSessionId.toString, session.getUsername)
    --- End diff --
    
    100 columns exceeded.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#issuecomment-97324368
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31254/
    Test FAILed.


---
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: [SPARK-5100][SQL]add webui for thriftserver

Posted by tianyi <gi...@git.apache.org>.
Github user tianyi commented on the pull request:

    https://github.com/apache/spark/pull/5730#issuecomment-96908252
  
    @WangTaoTheTonic , the Thrift Server's driver UI you mentioned is a universal UI for all kinds of work on spark, and mainly focus on jobs, stages and tasks. 
    In this PR, we provide a new tab that focus on users, sessions and SQL.
    So the driver UI won't be replaced since they are entirely different.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29211503
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    +    val retainedStatements =
    +      conf.getInt("spark.thriftserver.ui.retainedStatements", 200)
    +    val retainedSessions =
    +      conf.getInt("spark.thriftserver.ui.retainedSessions", 200)
    +    var totalRunning = 0
    +
    +    override def onJobStart(jobStart: SparkListenerJobStart): Unit = {
    +      val jobGroup = for (
    +        props <- Option(jobStart.properties);
    +        statement <- Option(props.getProperty(SparkContext.SPARK_JOB_GROUP_ID))
    +      ) yield statement
    +
    +      jobGroup.map( groupId => {
    --- End diff --
    
    Use `.foreach` instead of `.map` here.


---
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: [SPARK-5100][SQL]add webui for thriftserver

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

    https://github.com/apache/spark/pull/5730#discussion_r29212502
  
    --- Diff: sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala ---
    @@ -73,15 +94,146 @@ object HiveThriftServer2 extends Logging {
         }
       }
     
    +  private[thriftserver] class SessionInfo(
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val ip: String,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var totalExecute: Int = 0
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis() - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +  private[thriftserver] object ExecutionState extends Enumeration {
    +    val STARTED, COMPILED, FAILED, FINISHED = Value
    +    type ExecutionState = Value
    +  }
    +
    +  private[thriftserver] class ExecutionInfo(
    +      val statement: String,
    +      val sessionId: String,
    +      val startTimestamp: Long,
    +      val userName: String) {
    +    var finishTimestamp: Long = 0L
    +    var executePlan: String = ""
    +    var detail: String = ""
    +    var state: ExecutionState.Value = ExecutionState.STARTED
    +    val jobId: ArrayBuffer[String] = ArrayBuffer[String]()
    +    var groupId: String = ""
    +    def totalTime: Long = {
    +      if (finishTimestamp == 0L) {
    +        System.currentTimeMillis - startTimestamp
    +      } else {
    +        finishTimestamp - startTimestamp
    +      }
    +    }
    +  }
    +
    +
       /**
        * A inner sparkListener called in sc.stop to clean up the HiveThriftServer2
        */
    -  class HiveThriftServer2Listener(val server: HiveServer2) extends SparkListener {
    +  class HiveThriftServer2Listener(
    +      val server: HiveServer2,
    +      val conf: SparkConf) extends SparkListener {
    +
         override def onApplicationEnd(applicationEnd: SparkListenerApplicationEnd): Unit = {
           server.stop()
         }
    -  }
     
    +    val sessionList = new mutable.HashMap[String, SessionInfo]
    +    val executeList = new mutable.HashMap[String, ExecutionInfo]
    --- End diff --
    
    Considering you need to retire old elements, `java.util.LinkedHashMap` would be a better choice.


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