Skip to content

Conversation

@yabola
Copy link
Contributor

@yabola yabola commented Dec 2, 2022

What changes were proposed in this pull request?

Stages UI page fails to load for proxy in some specific yarn environment.

Why are the changes needed?

My environment CDH 5.8 , click to enter the spark UI from the yarn Resource Manager page
when visit the stage URI, it fails to load, URI is
http://:8088/proxy/application_1669877165233_0021/stages/stage/?id=0&attempt=0

The issue is similar to, the final phenomenon of the issue is the same, because the parameter encode twice
SPARK-32467
SPARK-33611

The two issues solve two scenarios to avoid encode twice:

  1. https redirect proxy
  2. set reverse proxy enabled (spark.ui.reverseProxy) in Nginx

But if encode twice due to other reasons, such as this issue (yarn proxy), it will also fail when visit stage page.
It is better to decode parameter twice here.
Just like fix here SPARK-12708 codes

Does this PR introduce any user-facing change?

No

How was this patch tested?

new added UT

@github-actions github-actions bot added the CORE label Dec 2, 2022
Copy link
Contributor Author

@yabola yabola left a comment

Choose a reason for hiding this comment

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

The change in the second place is necessary, at least the page can be opened normally and code logic has not changed.

// Decode URI twice here to avoid percent-encoding twice on the query string
val decodeURI = URLDecoder.decode(uriInfo.getRequestUri.getRawQuery, UTF_8.name())
val uriQueryParameters = new ImmutableMultivaluedMap[String, String](
UriComponent.decodeQuery(decodeURI, true))
Copy link
Contributor Author

@yabola yabola Dec 2, 2022

Choose a reason for hiding this comment

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

Not sure if decoding twice is appropriate here.
If do not change here, the stage page can be opened, but the task sort column button cannot be clicked (locality level etc.)
Given the change before, I think it is reasonable.
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/ui/UIUtils.scala#L626

}
withUI(_.store.taskList(stageId, stageAttemptId, pageStartIndex, pageLength,
indexName(columnNameToSort), isAscendingStr.equalsIgnoreCase("asc")))
indexName(columnNameToSort), "asc".equalsIgnoreCase(isAscendingStr)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

isAscendingStr may be NPE if URI was encoded twice.
After change here, at least the stage page can be opened now

@yabola yabola changed the title [SPARK-41365][CORE-UI] Stages UI page fails to load for proxy in some yarn environment [SPARK-41365][CORE-UI] Stages UI page fails to load for proxy in specific yarn environment Dec 2, 2022
@yabola yabola changed the title [SPARK-41365][CORE-UI] Stages UI page fails to load for proxy in specific yarn environment [SPARK-41365][UI] Stages UI page fails to load for proxy in specific yarn environment Dec 2, 2022
@yabola
Copy link
Contributor Author

yabola commented Dec 2, 2022

Stages UI page failed
MXADK6BDM0

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@mridulm
Copy link
Contributor

mridulm commented Dec 5, 2022

+CC @gengliangwang

@gengliangwang
Copy link
Member

@yabola could you add reproduce steps or tests for the issue?

@yabola
Copy link
Contributor Author

yabola commented Dec 6, 2022

@gengliangwang
My hadoop environment is CDH 5.8 ( yarn 2.6.0+cdh5.8.0+1589). It can reproduce easily in any application.
Run the latest spark code using yarn-client mode (I haven't tried other modes yet).
Click ApplicationMaster URL from ResourceManager Web UI (http://:8088/proxy/application_1669877165233_0021/stages/stage/?id=0&attempt=0)

@yabola
Copy link
Contributor Author

yabola commented Dec 6, 2022

@gengliangwang
I see comments (the last one) from other people in this issue , I think he had the same problem, it is after this issues fixed time

@gengliangwang
Copy link
Member

@yabola From the screenshot you provided, I don't see any double-quoted URL like %255B....

@yabola
Copy link
Contributor Author

yabola commented Dec 7, 2022

@gengliangwang Yes, but the URI will be processed by yarn proxy (encoded twice). I collect URIInfo in the interface (org.apache.spark.status.api.v1.StagesResource#taskTable).

WEB UI request(screenshot): http://10.1.3.16:8088/proxy/application_1669877165233_0040/api/v1/applications/application_1669877165233_0040/stages/0/0/taskTable?draw=2&order%5B0%5D%5Bcolumn%5D=4&order%5B0%5D%5Bdir%5D=asc&start=0&length=20&search%5Bvalue%5D=&search%5Bregex%5D=false&numTasks=21&columnIndexToSort=4&columnNameToSort=Locality%20Level&_=1670382363732

uriInfo.getRequestUri : http://10.1.3.16:4040/api/v1/applications/application_1669877165233_0040/stages/0/0/taskTable?draw=2&order%255B0%255D%255Bcolumn%255D=4&order%255B0%255D%255Bdir%255D=asc&start=0&length=20&search%255Bvalue%255D=&search%255Bregex%255D=false&numTasks=21&columnIndexToSort=4&columnNameToSort=Locality%2520Level&_=1670382363732

@gengliangwang
Copy link
Member

@yabola I am not quite sure about the "yarn proxy" you mentioned.
Can we fix the issue in a narrow waist method? IIUC there are also ajax requests in the executor page.

@yabola
Copy link
Contributor Author

yabola commented Dec 7, 2022

@yabola I am not quite sure about the "yarn proxy" you mentioned. Can we fix the issue in a narrow waist method? IIUC there are also ajax requests in the executor page.

@gengliangwang I check Executor page requests don't have query params (not sure about all) , so it work well.
Do you mean find a general method to solve all query parameters ( web Filter etc.)? I tried to find, but I don't have a good idea right now.

@gengliangwang
Copy link
Member

@yabola ok, can you try adding a new test case for it?

@github-actions github-actions bot added the WEB UI label Dec 9, 2022
@yabola
Copy link
Contributor Author

yabola commented Dec 9, 2022

@gengliangwang I add new UT and change codes to carefully decode each parameter, I think it aligns with the previous behavior and is more accurate ( I reuse origin decodeURLParameter codes , It supports multiple decode). How do you feel it?

/**
* Decode URLParameter if URL is encoded by YARN-WebAppProxyServlet.
* Due to YARN-2844: WebAppProxyServlet cannot handle urls which contain encoded characters
* Therefore we need to decode it until we get the real URLParameter.
*/
def decodeURLParameter(urlParam: String): String = {
var param = urlParam
var decodedParam = URLDecoder.decode(param, UTF_8.name())
while (param != decodedParam) {
param = decodedParam
decodedParam = URLDecoder.decode(param, UTF_8.name())
}
param
}

@yabola
Copy link
Contributor Author

yabola commented Dec 15, 2022

@gengliangwang If you have time, please have a look, thanks. And do I need to change codes to decode parameters only twice here?

@gengliangwang
Copy link
Member

gengliangwang commented Dec 16, 2022

@yabola Thanks for the fix. The changes LGTM.
Merging to master.

@gengliangwang
Copy link
Member

@yabola There is merge conflict on branch-3.3. Could you create a backport PR against branch-3.3?

@yabola
Copy link
Contributor Author

yabola commented Dec 16, 2022

@gengliangwang @mridulm Thank you all~ I had created back port PR #39087

dongjoon-hyun pushed a commit that referenced this pull request Dec 16, 2022
…ific yarn environment

backport #38882

### What changes were proposed in this pull request?
Stages UI page fails to load for proxy in some specific yarn environment.

### Why are the changes needed?
My environment CDH 5.8 , click to enter the spark UI from the yarn Resource Manager page
when visit the stage URI, it fails to load,  URI is
http://<yarn-url>:8088/proxy/application_1669877165233_0021/stages/stage/?id=0&attempt=0

The issue is similar to, the final phenomenon of the issue is the same, because the parameter encode twice
[SPARK-32467](https://issues.apache.org/jira/browse/SPARK-32467)
[SPARK-33611](https://issues.apache.org/jira/browse/SPARK-33611)

The two issues solve two scenarios to avoid encode twice:
1. https redirect proxy
2. set reverse proxy enabled (spark.ui.reverseProxy)  in Nginx

But if encode twice due to other reasons, such as this issue (yarn proxy), it will also fail when visit stage page.
It is better to decode parameter twice here.
Just like fix here [SPARK-12708](https://issues.apache.org/jira/browse/SPARK-12708) [codes](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/ui/UIUtils.scala#L626)

### Does this PR introduce any user-facing change?
No

### How was this patch tested?
new added UT

Closes #39087 from yabola/fixui-backport.

Authored-by: chenliang.lu <marssss2929@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants