Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -139,33 +139,60 @@ private[ui] class ThriftServerPage(parent: ThriftServerTab) extends WebUIPage(""

/** Generate stats of batch sessions of the thrift server program */
private def generateSessionStatsTable(request: HttpServletRequest): Seq[Node] = {
val sessionList = listener.getSessionList
val numBatches = sessionList.size
val table = if (numBatches > 0) {
val dataRows = sessionList.sortBy(_.startTimestamp).reverse
val headerRow = Seq("User", "IP", "Session ID", "Start Time", "Finish Time", "Duration",
"Total Execute")
def generateDataRow(session: SessionInfo): Seq[Node] = {
val sessionLink = "%s/%s/session/?id=%s".format(
UIUtils.prependBaseUri(request, parent.basePath), parent.prefix, session.sessionId)
<tr>
<td> {session.userName} </td>
<td> {session.ip} </td>
<td> <a href={sessionLink}> {session.sessionId} </a> </td>
<td> {formatDate(session.startTimestamp)} </td>
<td> {if (session.finishTimestamp > 0) formatDate(session.finishTimestamp)} </td>
<td sorttable_customkey={session.totalTime.toString}>
{formatDurationOption(Some(session.totalTime))} </td>
<td> {session.totalExecution.toString} </td>
</tr>
val numSessions = listener.getSessionList.size
val table = if (numSessions > 0) {

val sessionTableTag = "sessionstat"

val parameterOtherTable = request.getParameterMap().asScala
.filterNot(_._1.startsWith(sessionTableTag))
.map { case (name, vals) =>
name + "=" + vals(0)
}

val parameterSessionTablePage = request.getParameter(s"$sessionTableTag.page")
val parameterSessionTableSortColumn = request.getParameter(s"$sessionTableTag.sort")
val parameterSessionTableSortDesc = request.getParameter(s"$sessionTableTag.desc")
val parameterSessionPageSize = request.getParameter(s"$sessionTableTag.pageSize")

val sessionTablePage = Option(parameterSessionTablePage).map(_.toInt).getOrElse(1)
val sessionTableSortColumn = Option(parameterSessionTableSortColumn).map { sortColumn =>
UIUtils.decodeURLParameter(sortColumn)
}.getOrElse("Start Time")
val sessionTableSortDesc = Option(parameterSessionTableSortDesc).map(_.toBoolean).getOrElse(
// Old session should be shown above new session by default.
!(sessionTableSortColumn == "Start Time")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AngersZhuuuu By default new session should come above right? (Screenshot from master branch)
Screenshot 2019-10-25 at 11 04 15 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AngersZhuuuu By default new session should come above right? (Screenshot from master branch)

Updated, I could have been misled by the modified thriftserver I was using

)
val sessionTablePageSize = Option(parameterSessionPageSize).map(_.toInt).getOrElse(100)

try {
Some(new SessionStatsPagedTable(
request,
parent,
listener.getSessionList,
"sqlserver",
UIUtils.prependBaseUri(request, parent.basePath),
parameterOtherTable,
sessionTableTag,
pageSize = sessionTablePageSize,
sortColumn = sessionTableSortColumn,
desc = sessionTableSortDesc
).table(sessionTablePage))
} catch {
case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) =>
Some(<div class="alert alert-error">
<p>Error while rendering job table:</p>
<pre>
{Utils.exceptionString(e)}
</pre>
</div>)
}
Some(UIUtils.listingTable(headerRow, generateDataRow, dataRows, true, None, Seq(null), false))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Here for the sessions table fixedWidth is true. right? Are we making the paged table also fixed width?

@AngersZhuuuu AngersZhuuuu Oct 27, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixedWidth

Changed. Not so familiar with UI HTML part, thanks a lot for your suggestion

} else {
None
}

val content =
<h5 id="sessionstat">Session Statistics ({numBatches})</h5> ++
<h5 id="sessionstat">Session Statistics ({numSessions})</h5> ++
<div>
<ul class="unstyled">
{table.getOrElse("No statistics have been generated yet.")}
Expand Down Expand Up @@ -377,6 +404,164 @@ private[ui] class SqlStatsPagedTable(
"%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, parent.basePath), jobId)
}

private[ui] class SessionStatsPagedTable(
request: HttpServletRequest,
parent: ThriftServerTab,
data: Seq[SessionInfo],
subPath: String,
basePath: String,
parameterOtherTable: Iterable[String],
sessionStatsTableTag: String,
pageSize: Int,
sortColumn: String,
desc: Boolean) extends PagedTable[SessionInfo] {

override val dataSource = new SessionStatsTableDataSource(data, pageSize, sortColumn, desc)

private val parameterPath = s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"

override def tableId: String = sessionStatsTableTag

override def tableCssClass: String =
"table table-bordered table-condensed table-striped " +
"table-head-clickable table-cell-width-limited"

override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, UTF_8.name())
parameterPath +
s"&$pageNumberFormField=$page" +
s"&$sessionStatsTableTag.sort=$encodedSortColumn" +
s"&$sessionStatsTableTag.desc=$desc" +
s"&$pageSizeFormField=$pageSize"
}

override def pageSizeFormField: String = s"$sessionStatsTableTag.pageSize"

override def pageNumberFormField: String = s"$sessionStatsTableTag.page"

override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, UTF_8.name())
s"$parameterPath&$sessionStatsTableTag.sort=$encodedSortColumn&$sessionStatsTableTag.desc=$desc"
}

override def headers: Seq[Node] = {
val sessionTableHeaders =
Seq("User", "IP", "Session ID", "Start Time", "Finish Time", "Duration", "Total Execute")

val tooltips = Seq[Option[String]](None, None, None, None, None, None, None)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need tooltips for the table?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need tooltips for the table?

Ok for me to remove these. If we all think don't need and won't add some thing, just remove it.
By the way, thanks a lot for you nice job. This problem trouble me a lot

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is already a PR working on it, related to tool tips here, #26138

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is already a PR working on it, related to tool tips here, #26138

Very embarrassed..., for me , this table's information is so clear. Don't add tooltip is ok.


assert(sessionTableHeaders.length == tooltips.length)

val headerRow: Seq[Node] = {
sessionTableHeaders.zip(tooltips).map { case (header, tooltip) =>
if (header == sortColumn) {
val headerLink = Unparsed(
parameterPath +
s"&$sessionStatsTableTag.sort=${URLEncoder.encode(header, UTF_8.name())}" +
s"&$sessionStatsTableTag.desc=${!desc}" +
s"&$sessionStatsTableTag.pageSize=$pageSize" +
s"#$sessionStatsTableTag")
val arrow = if (desc) "&#x25BE;" else "&#x25B4;" // UP or DOWN

if (tooltip.nonEmpty) {
<th>
<a href={headerLink}>
<span data-toggle="tooltip" title={tooltip.get}>
{header}&nbsp;{Unparsed(arrow)}
</span>
</a>
</th>
} else {
<th>
<a href={headerLink}>
{header}&nbsp;{Unparsed(arrow)}
</a>
</th>
}
} else {
val headerLink = Unparsed(
parameterPath +
s"&$sessionStatsTableTag.sort=${URLEncoder.encode(header, UTF_8.name())}" +
s"&$sessionStatsTableTag.pageSize=$pageSize" +
s"#$sessionStatsTableTag")

if(tooltip.nonEmpty) {
<th>
<a href={headerLink}>
<span data-toggle="tooltip" title={tooltip.get}>
{header}
</span>
</a>
</th>
} else {
<th>
<a href={headerLink}>
{header}
</a>
</th>
}
}
}
}
<thead>
{headerRow}
</thead>
}

override def row(session: SessionInfo): Seq[Node] = {
val sessionLink = "%s/%s/session/?id=%s".format(
UIUtils.prependBaseUri(request, parent.basePath), parent.prefix, session.sessionId)
<tr>
<td> {session.userName} </td>
<td> {session.ip} </td>
<td> <a href={sessionLink}> {session.sessionId} </a> </td>
<td> {formatDate(session.startTimestamp)} </td>
<td> {if (session.finishTimestamp > 0) formatDate(session.finishTimestamp)} </td>
<td sorttable_customkey={session.totalTime.toString}>

@shahidki31 shahidki31 Oct 27, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is it necessary to add sorttable_customkey as we are already sorting using raw data in ordering method?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Is it necessary to add sorttable_customkey as we are already sorting using raw data in ordering method?

This seems to be the previous pr, change to use raw data for sort column, old version didn't have this and trouble me a lot. Don't need it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please confirm if the sorting is working properly after changing this. Thanks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, please confirm if the sorting is working properly after changing this. Thanks

Checked ok. Thanks

{formatDurationOption(Some(session.totalTime))} </td>

@shahidki31 shahidki31 Oct 25, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, we can directly call UIUtils.formatDurationVerbose , because totalTime is always defined. Also can remove the method formatDurationOption

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, we can directly call UIUtils.formatDurationVerbose , because totalTime is always defined. Also can remove the method formatDurationOption

Good suggestion. Removed other old unused code, feel comfortable now.

<td> {session.totalExecution.toString} </td>
</tr>
}

/**
* Returns a human-readable string representing a duration such as "5 second 35 ms"
*/
private def formatDurationOption(msOption: Option[Long]): String = {
msOption.map(formatDurationVerbose).getOrElse("-")
}

private def errorMessageCell(errorMessage: String): Seq[Node] = {
val isMultiline = errorMessage.indexOf('\n') >= 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method used anywhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this method used anywhere?

All remove

val errorSummary = StringEscapeUtils.escapeHtml4(
if (isMultiline) {
errorMessage.substring(0, errorMessage.indexOf('\n'))
} else {
errorMessage
})
val details = if (isMultiline) {
// scalastyle:off
<span onclick="this.parentNode.querySelector('.stacktrace-details').classList.toggle('collapsed')"
class="expand-details">
+ details
</span> ++
<div class="stacktrace-details collapsed">
<pre>
{errorMessage}
</pre>
</div>
// scalastyle:on
} else {
""
}
<td>
{errorSummary}{details}
</td>
}

private def jobURL(request: HttpServletRequest, jobId: String): String =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method also seems not used?

"%s/jobs/job/?id=%s".format(UIUtils.prependBaseUri(request, parent.basePath), jobId)
}

private[ui] class SqlStatsTableRow(
val jobId: Seq[String],
val duration: Long,
Expand Down Expand Up @@ -442,3 +627,45 @@ private[ui] class SqlStatsPagedTable(
}

}

private[ui] class SessionStatsTableDataSource(
info: Seq[SessionInfo],
pageSize: Int,
sortColumn: String,
desc: Boolean) extends PagedDataSource[SessionInfo](pageSize) {

// Sorting SessionInfo data
private val data = info.sorted(ordering(sortColumn, desc))

private var _slicedStartTime: Set[Long] = null

override def dataSize: Int = data.size

override def sliceData(from: Int, to: Int): Seq[SessionInfo] = {
val r = data.slice(from, to)
r.map(x => x)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: what does this do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: what does this do?

For paging, get selected pages data.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean, .map(x => x) is a no-op right? what is this line for?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I mean, .map(x => x) is a no-op right? what is this line for?

Same implement with #26215

Since same function... so don't check this method.
Remove both of these.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can remove the line.

_slicedStartTime = r.map(_.startTimestamp).toSet
r
}

/**
* Return Ordering according to sortColumn and desc.
*/
private def ordering(sortColumn: String, desc: Boolean): Ordering[SessionInfo] = {
val ordering: Ordering[SessionInfo] = sortColumn match {
case "User" => Ordering.by(_.userName)
case "IP" => Ordering.by(_.ip)
case "Session ID" => Ordering.by(_.sessionId)
case "Start Time" => Ordering by (_.startTimestamp)
case "Finish Time" => Ordering.by(_.finishTimestamp)
case "Duration" => Ordering.by(_.totalTime)
case "Total Execute" => Ordering.by(_.totalExecution)
case unknownColumn => throw new IllegalArgumentException(s"Unknown column: $unknownColumn")
}
if (desc) {
ordering.reverse
} else {
ordering
}
}
}