Skip to content
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-33831][UI] Update to jetty 9.4.34 #30828

Closed
wants to merge 2 commits into from

Conversation

srowen
Copy link
Member

@srowen srowen commented Dec 17, 2020

What changes were proposed in this pull request?

Update Jetty to 9.4.34

Why are the changes needed?

Picks up fixes and improvements, including a possible CVE fix.

https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.33.v20201020
https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.34.v20201102

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

@srowen srowen self-assigned this Dec 17, 2020
@github-actions github-actions bot added the BUILD label Dec 17, 2020
@SparkQA
Copy link

SparkQA commented Dec 17, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37569/

@SparkQA
Copy link

SparkQA commented Dec 17, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37569/

@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Test build #132966 has finished for PR 30828 at commit 188e81b.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member

maropu commented Dec 18, 2020

retest this please

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thanks, @srowen . The behavior of Jetty seems to change. This fails at

      // Test Jetty's built-in redirect to add the trailing slash to the context path.
      TestUtils.withHttpConnection(new URL(s"$serverAddr/ctx1")) { conn =>
        assert(conn.getResponseCode() === HttpServletResponse.SC_FOUND)
        val location = Option(conn.getHeaderFields().get("Location"))
          .map(_.get(0)).orNull
        assert(location === s"$proxyRoot/ctx1/")
      }

Could you check the root cause and adjust some configuration or the UT UiSuite.redirect with proxy server support? It would be great if we have the same behavior, but if it's inevitable, shall we add some comment on the migration guide?

[info] - redirect with proxy server support *** FAILED *** (32 milliseconds)
[info]   404 did not equal 302 (UISuite.scala:367)

@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37572/

@srowen
Copy link
Member Author

srowen commented Dec 18, 2020

Weird, doesn't happen on the previous 9.4.34 version. I'll just go with that for now, because either:

  • It's a bug in the Spark Jetty code, which I'd punt on this moment as I just want to backport fixes back to 2.x, and want to minimize change
  • Or, maybe it's an unintentional change that a slightly later Jetty will fix anyway

@srowen srowen changed the title [SPARK-33831][UI] Update to jetty 9.4.35 [SPARK-33831][UI] Update to jetty 9.4.34 Dec 18, 2020
@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37572/

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for updates, @srowen .
+1, LGTM (Pending CIs).

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Dec 18, 2020

cc @HyukjinKwon since he is the release manager of Apache Spark 3.1.0.

This issue is targeting all branches.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM too. Thanks @dongjoon-hyun for cc'ing me.

@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37577/

@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/37577/

dongjoon-hyun pushed a commit that referenced this pull request Dec 18, 2020
Update Jetty to 9.4.34

Picks up fixes and improvements, including a possible CVE fix.

https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.33.v20201020
https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.34.v20201102

No.

Existing tests.

Closes #30828 from srowen/SPARK-33831.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 131a23d)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun pushed a commit that referenced this pull request Dec 18, 2020
Update Jetty to 9.4.34

Picks up fixes and improvements, including a possible CVE fix.

https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.33.v20201020
https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.34.v20201102

No.

Existing tests.

Closes #30828 from srowen/SPARK-33831.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 131a23d)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun pushed a commit that referenced this pull request Dec 18, 2020
Update Jetty to 9.4.34

Picks up fixes and improvements, including a possible CVE fix.

https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.33.v20201020
https://github.com/eclipse/jetty.project/releases/tag/jetty-9.4.34.v20201102

No.

Existing tests.

Closes #30828 from srowen/SPARK-33831.

Authored-by: Sean Owen <srowen@gmail.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
(cherry picked from commit 131a23d)
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun
Copy link
Member

Merged to master/3.1/3.0/2.4. Thanks.

@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Test build #132976 has finished for PR 30828 at commit f5b67b1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Dec 18, 2020

Test build #132969 has finished for PR 30828 at commit 188e81b.

  • This patch fails from timeout after a configured wait of 500m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen srowen deleted the SPARK-33831 branch March 9, 2021 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants