[SPARK-34635][UI] Add trailing slashes in URLs to reduce unnecessary redirects by gengliangwang · Pull Request #31753 · apache/spark

gengliangwang · 2021-03-05T05:47:44Z

What changes were proposed in this pull request?

Add trailing slashes in URLs of Spark UI pages.

Why are the changes needed?

When a user accesses a URL without a trailing slash, Spark UI always responds with a 302 redirect to a URL with a trailing slash.

Adding trailing slash to URLs in UI pages can reduce such unnecessary redirects

Does this PR introduce any user-facing change?

No

How was this patch tested?

Manual test. It's a very simple change.

dongjoon-hyun · 2021-03-05T05:53:35Z

Thank you, @gengliangwang .

dongjoon-hyun

+1, LGTM.

dongjoon-hyun · 2021-03-05T05:58:03Z

BTW, are these all places, @gengliangwang ?

sarutak

Thanks @gengliangwang . It LGTM.

gengliangwang · 2021-03-05T06:12:06Z

BTW, are these all places, @gengliangwang ?

@dongjoon-hyun That's all I can find for now.

AmplabJenkins · 2021-03-05T06:13:16Z

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135795/

dongjoon-hyun · 2021-03-05T07:24:25Z

Merged to master for Apache Spark 3.2.0. Thank you, @gengliangwang and all!

AmplabJenkins · 2021-03-05T07:58:07Z

Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/40377/

…Page` urls ### What changes were proposed in this pull request? This PR aims to use the same pattern for `logPage` urls of `WorkerPage` to make it work consistently when `spark.ui.reverseProxy=true`. ### Why are the changes needed? Since Apache Spark 3.2.0 (SPARK-34635, #31753), Apache Spark adds trailing slashes to reduce redirections for `logPage`. ```scala s"$workerUrlRef/logPage?driverId=$driverId&logType=stdout") s"$workerUrlRef/logPage/?driverId=$driverId&logType=stdout") ... <a href={s"$workerUrlRef/logPage?appId=${executor <a href={s"$workerUrlRef/logPage/?appId=${executor ``` This PR aims to fix a leftover in `WorkerPage` to make it work consistently in case of the reverse proxy situation via `spark.ui.reverseProxy`. Currently, in some proxy environments, `appId` link is working but `driverId` link is broken due to the redirections. This inconsistent behavior makes the users confused. ``` - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stdout"}>stdout</a> - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stderr"}>stderr</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stdout"}>stdout</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stderr"}>stderr</a> ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manual tests because it requires a reverse proxy. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #42959 from dongjoon-hyun/SPARK-45187. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…Page` urls ### What changes were proposed in this pull request? This PR aims to use the same pattern for `logPage` urls of `WorkerPage` to make it work consistently when `spark.ui.reverseProxy=true`. ### Why are the changes needed? Since Apache Spark 3.2.0 (SPARK-34635, #31753), Apache Spark adds trailing slashes to reduce redirections for `logPage`. ```scala s"$workerUrlRef/logPage?driverId=$driverId&logType=stdout") s"$workerUrlRef/logPage/?driverId=$driverId&logType=stdout") ... <a href={s"$workerUrlRef/logPage?appId=${executor <a href={s"$workerUrlRef/logPage/?appId=${executor ``` This PR aims to fix a leftover in `WorkerPage` to make it work consistently in case of the reverse proxy situation via `spark.ui.reverseProxy`. Currently, in some proxy environments, `appId` link is working but `driverId` link is broken due to the redirections. This inconsistent behavior makes the users confused. ``` - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stdout"}>stdout</a> - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stderr"}>stderr</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stdout"}>stdout</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stderr"}>stderr</a> ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manual tests because it requires a reverse proxy. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #42959 from dongjoon-hyun/SPARK-45187. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit f8f2735) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…Page` urls ### What changes were proposed in this pull request? This PR aims to use the same pattern for `logPage` urls of `WorkerPage` to make it work consistently when `spark.ui.reverseProxy=true`. ### Why are the changes needed? Since Apache Spark 3.2.0 (SPARK-34635, #31753), Apache Spark adds trailing slashes to reduce redirections for `logPage`. ```scala s"$workerUrlRef/logPage?driverId=$driverId&logType=stdout") s"$workerUrlRef/logPage/?driverId=$driverId&logType=stdout") ... <a href={s"$workerUrlRef/logPage?appId=${executor <a href={s"$workerUrlRef/logPage/?appId=${executor ``` This PR aims to fix a leftover in `WorkerPage` to make it work consistently in case of the reverse proxy situation via `spark.ui.reverseProxy`. Currently, in some proxy environments, `appId` link is working but `driverId` link is broken due to the redirections. This inconsistent behavior makes the users confused. ``` - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stdout"}>stdout</a> - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stderr"}>stderr</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stdout"}>stdout</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stderr"}>stderr</a> ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manual tests because it requires a reverse proxy. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #42959 from dongjoon-hyun/SPARK-45187. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit f8f2735) Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 84a053e) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…Page` urls ### What changes were proposed in this pull request? This PR aims to use the same pattern for `logPage` urls of `WorkerPage` to make it work consistently when `spark.ui.reverseProxy=true`. ### Why are the changes needed? Since Apache Spark 3.2.0 (SPARK-34635, #31753), Apache Spark adds trailing slashes to reduce redirections for `logPage`. ```scala s"$workerUrlRef/logPage?driverId=$driverId&logType=stdout") s"$workerUrlRef/logPage/?driverId=$driverId&logType=stdout") ... <a href={s"$workerUrlRef/logPage?appId=${executor <a href={s"$workerUrlRef/logPage/?appId=${executor ``` This PR aims to fix a leftover in `WorkerPage` to make it work consistently in case of the reverse proxy situation via `spark.ui.reverseProxy`. Currently, in some proxy environments, `appId` link is working but `driverId` link is broken due to the redirections. This inconsistent behavior makes the users confused. ``` - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stdout"}>stdout</a> - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stderr"}>stderr</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stdout"}>stdout</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stderr"}>stderr</a> ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manual tests because it requires a reverse proxy. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #42959 from dongjoon-hyun/SPARK-45187. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit f8f2735) Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 84a053e) Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit b4eb947) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…Page` urls ### What changes were proposed in this pull request? This PR aims to use the same pattern for `logPage` urls of `WorkerPage` to make it work consistently when `spark.ui.reverseProxy=true`. ### Why are the changes needed? Since Apache Spark 3.2.0 (SPARK-34635, apache#31753), Apache Spark adds trailing slashes to reduce redirections for `logPage`. ```scala s"$workerUrlRef/logPage?driverId=$driverId&logType=stdout") s"$workerUrlRef/logPage/?driverId=$driverId&logType=stdout") ... <a href={s"$workerUrlRef/logPage?appId=${executor <a href={s"$workerUrlRef/logPage/?appId=${executor ``` This PR aims to fix a leftover in `WorkerPage` to make it work consistently in case of the reverse proxy situation via `spark.ui.reverseProxy`. Currently, in some proxy environments, `appId` link is working but `driverId` link is broken due to the redirections. This inconsistent behavior makes the users confused. ``` - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stdout"}>stdout</a> - <a href={s"$workerUrlRef/logPage?driverId=${driver.driverId}&logType=stderr"}>stderr</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stdout"}>stdout</a> + <a href={s"$workerUrlRef/logPage/?driverId=${driver.driverId}&logType=stderr"}>stderr</a> ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Manual tests because it requires a reverse proxy. ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#42959 from dongjoon-hyun/SPARK-45187. Authored-by: Dongjoon Hyun <dhyun@apple.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit f8f2735) Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 84a053e) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

add trailing slash for reducing unnecessary redirect

a3b203e

gengliangwang requested review from dongjoon-hyun and sarutak March 5, 2021 05:48

github-actions bot added CORE WEB UI labels Mar 5, 2021

dongjoon-hyun approved these changes Mar 5, 2021

View reviewed changes

sarutak approved these changes Mar 5, 2021

View reviewed changes

HyukjinKwon approved these changes Mar 5, 2021

View reviewed changes

dongjoon-hyun closed this in 358697b Mar 5, 2021

dongjoon-hyun mentioned this pull request Sep 17, 2023

[SPARK-45187][CORE] Fix WorkerPage to use the same pattern for logPage urls #42959

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-34635][UI] Add trailing slashes in URLs to reduce unnecessary redirects#31753

[SPARK-34635][UI] Add trailing slashes in URLs to reduce unnecessary redirects#31753
gengliangwang wants to merge 1 commit intoapache:masterfrom
gengliangwang:reduceRedirect

gengliangwang commented Mar 5, 2021

Uh oh!

dongjoon-hyun commented Mar 5, 2021

Uh oh!

dongjoon-hyun left a comment

Uh oh!

dongjoon-hyun commented Mar 5, 2021

Uh oh!

sarutak left a comment

Uh oh!

gengliangwang commented Mar 5, 2021

Uh oh!

AmplabJenkins commented Mar 5, 2021

Uh oh!

dongjoon-hyun commented Mar 5, 2021

Uh oh!

AmplabJenkins commented Mar 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

gengliangwang commented Mar 5, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

dongjoon-hyun commented Mar 5, 2021

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Mar 5, 2021

Uh oh!

sarutak left a comment

Choose a reason for hiding this comment

Uh oh!

gengliangwang commented Mar 5, 2021

Uh oh!

AmplabJenkins commented Mar 5, 2021

Uh oh!

dongjoon-hyun commented Mar 5, 2021

Uh oh!

AmplabJenkins commented Mar 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants