-
Notifications
You must be signed in to change notification settings - Fork 28.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-12708][UI] Sorting task error in Stages Page when yarn mode. #10663
Changes from 4 commits
fc45425
62c04ca
3678e30
3d508c6
384b990
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,7 @@ private[ui] class PoolPage(parent: StagesTab) extends WebUIPage("pool") { | |
|
||
def render(request: HttpServletRequest): Seq[Node] = { | ||
listener.synchronized { | ||
val poolName = request.getParameter("poolname") | ||
val poolName = UIUtils.decodeURLParameter(request.getParameter("poolname")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NullPointerException if |
||
require(poolName != null && poolName.nonEmpty, "Missing poolname parameter") | ||
|
||
val poolToActiveStages = listener.poolToActiveStages | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,8 @@ | |
|
||
package org.apache.spark.ui.jobs | ||
|
||
import java.net.URLEncoder | ||
|
||
import scala.collection.mutable.HashMap | ||
import scala.xml.Node | ||
|
||
|
@@ -59,7 +61,8 @@ private[ui] class PoolTable(pools: Seq[Schedulable], parent: StagesTab) { | |
case None => 0 | ||
} | ||
val href = "%s/stages/pool?poolname=%s" | ||
.format(UIUtils.prependBaseUri(parent.basePath), p.name) | ||
.format(UIUtils.prependBaseUri(parent.basePath), | ||
URLEncoder.encode(p.name, "UTF-8")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This two lines can be fit into one line. |
||
<tr> | ||
<td> | ||
<a href={href}>{p.name}</a> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,6 +67,20 @@ class UIUtilsSuite extends SparkFunSuite { | |
s"\nRunning progress bar should round down\n\nExpected:\n$expected\nGenerated:\n$generated") | ||
} | ||
|
||
test("decodeURLParameter (SPARK-12708: Sorting task error in Stages Page when yarn mode.)") { | ||
val encoded1 = "%252F" | ||
val decoded1 = "/" | ||
val encoded2 = "%253Cdriver%253E" | ||
val decoded2 = "<driver>" | ||
|
||
assert(decoded1.sameElements(decodeURLParameter(encoded1))) | ||
assert(decoded2.sameElements(decodeURLParameter(encoded2))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why don't you just compare by |
||
|
||
// verify that no affect to decoded URL. | ||
assert(decoded1.sameElements(decodeURLParameter(decoded1))) | ||
assert(decoded2.sameElements(decodeURLParameter(decoded2))) | ||
} | ||
|
||
private def verify( | ||
desc: String, expected: Elem, errorMsg: String = "", baseUrl: String = ""): Unit = { | ||
val generated = makeDescription(desc, baseUrl) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
decodeURLParameter
is not invoked.We should say like as follows right?