[SPARK-29546][TESTS] Recover jersey-guava test dependency in docker-integration-tests #26203

dongjoon-hyun · 2019-10-22T07:55:52Z

What changes were proposed in this pull request?

Currently, docker-integration-tests is broken in both JDK8/11.
This PR aims to recover JDBC integration test for JDK8/11.

Why are the changes needed?

While SPARK-28737 upgraded Jersey to 2.29 for JDK11, docker-integration-tests is broken because com.spotify.docker-client still depends on jersey-guava. The latest com.spotify.docker-client also has this problem.

https://mvnrepository.com/artifact/com.spotify/docker-client/5.0.2
-> https://mvnrepository.com/artifact/org.glassfish.jersey.core/jersey-client/2.19
-> https://mvnrepository.com/artifact/org.glassfish.jersey.core/jersey-common/2.19
-> https://mvnrepository.com/artifact/org.glassfish.jersey.bundles.repackaged/jersey-guava/2.19

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Manual because this is an integration test suite.

$ java -version
openjdk version "1.8.0_222"
OpenJDK Runtime Environment (AdoptOpenJDK)(build 1.8.0_222-b10)
OpenJDK 64-Bit Server VM (AdoptOpenJDK)(build 25.222-b10, mixed mode)

$ build/mvn install -DskipTests

$ build/mvn -Pdocker-integration-tests -pl :spark-docker-integration-tests_2.12 test

$ java -version
openjdk version "11.0.5" 2019-10-15
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.5+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.5+10, mixed mode)

$ build/mvn install -DskipTests

$ build/mvn -Pdocker-integration-tests -pl :spark-docker-integration-tests_2.12 test

BEFORE

*** RUN ABORTED ***
  com.spotify.docker.client.exceptions.DockerException: java.util.concurrent.ExecutionException: javax.ws.rs.ProcessingException: java.lang.NoClassDefFoundError: jersey/repackaged/com/google/common/util/concurrent/MoreExecutors
  at com.spotify.docker.client.DefaultDockerClient.propagate(DefaultDockerClient.java:1607)
  at com.spotify.docker.client.DefaultDockerClient.request(DefaultDockerClient.java:1538)
  at com.spotify.docker.client.DefaultDockerClient.ping(DefaultDockerClient.java:387)
  at org.apache.spark.sql.jdbc.DockerJDBCIntegrationSuite.beforeAll(DockerJDBCIntegrationSuite.scala:81)

AFTER

Run completed in 47 seconds, 999 milliseconds.
Total number of tests run: 30
Suites: completed 6, aborted 0
Tests: succeeded 30, failed 0, canceled 0, ignored 6, pending 0
All tests passed.

…ntegration-tests

dongjoon-hyun · 2019-10-22T08:14:59Z

cc @srowen

dongjoon-hyun · 2019-10-22T08:35:53Z

To reviewers. Please ignore GitHub Action (Linters) flaky failures which is happening on master branch, too.

SparkQA · 2019-10-22T10:24:59Z

Test build #112449 has finished for PR 26203 at commit bfec288.

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

srowen

My short-term concern is that we're about to make a preview release, and if this causes test failures on JDK 11, might not be worth doing right now. Is there a short term fix for JDK 11?

external/docker-integration-tests/pom.xml

dongjoon-hyun · 2019-10-22T15:52:44Z

Thank you for review. I guess we need to find a different library instead of spotify library for this test suite.

dongjoon-hyun · 2019-10-22T16:06:57Z

Retest this please.

SparkQA · 2019-10-22T18:36:08Z

Test build #112471 has finished for PR 26203 at commit cd826ee.

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

SparkQA · 2019-10-22T18:36:40Z

Test build #112472 has finished for PR 26203 at commit cd826ee.

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

dongjoon-hyun · 2019-10-22T21:12:58Z

Hi, @srowen .
Could you review this once more? I added the additional comment with IDed TODO and JDK11 UT also passed (because this is only affecting Integration Test).

srowen · 2019-10-22T23:23:04Z

The comment is good, but, can we do this if it still breaks JDK11? why does it break -- 2.25.1 doesn't work with JDK 11?

dongjoon-hyun · 2019-10-22T23:44:09Z

Technically, SPARK-28737 broke this test suite for both JDK8/11.
I don't think this is a release blocker for JDK11 because this is an integration test suite.
However, it would be great if we find a single solution for both. I'll investigate more, @srowen .

srowen · 2019-10-22T23:50:14Z

Yes that's fair enough. It's an optional test suite, I suppose.

dongjoon-hyun · 2019-10-23T04:48:34Z

Oh, @srowen . This patch recovers both JDK8/JDK11. Last time, it seems I was confused during testing. I updated the PR description and comment.

$ java -version
openjdk version "11.0.5" 2019-10-15
OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.5+10)
OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.5+10, mixed mode)

...
Run completed in 4 minutes, 14 seconds.
Total number of tests run: 30
Suites: completed 6, aborted 0
Tests: succeeded 30, failed 0, canceled 0, ignored 6, pending 0
All tests passed.
19/10/22 22:00:39 INFO ShutdownHookManager: Shutdown hook called
19/10/22 22:00:39 INFO ShutdownHookManager: Deleting directory /Users/dongjoon/PRS/SPARK-29546/external/docker-integration-tests/target/tmp/spark-b6ad8892-8d58-4611-8a51-c47b4949be89
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  04:20 min
[INFO] Finished at: 2019-10-22T22:00:39-07:00
[INFO] ------------------------------------------------------------------------

dongjoon-hyun · 2019-10-23T05:49:04Z

Hi, @maropu . Could you review this please?

maropu · 2019-10-23T06:00:31Z

Yea, sure! I'll do now.

dongjoon-hyun · 2019-10-23T06:04:49Z

Thank you so much, @maropu !

SparkQA · 2019-10-23T07:05:02Z

Test build #112510 has finished for PR 26203 at commit fafb24c.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

maropu · 2019-10-23T07:11:46Z

retest this please

maropu

I checked that the current master failed because of this dependency issue then this pr could fix the issue on JDK8/JDK11.

dongjoon-hyun · 2019-10-23T07:14:09Z

Thank you so much, @maropu !
Since this PR already passed Jenkins twice (JDK8/JDK11) and the last two commit is comment-only changes, I'll merge this. Merged to master.

srowen

Yep, looks good.

[SPARK-29546][TESTS] Recover jersey-guava test dependency in docker-i…

bfec288

…ntegration-tests

dongjoon-hyun added the TESTS label Oct 22, 2019

srowen reviewed Oct 22, 2019

View reviewed changes

external/docker-integration-tests/pom.xml Show resolved Hide resolved

Address comment

cd826ee

dongjoon-hyun changed the title ~~[SPARK-29546][TESTS] Recover jersey-guava test dependency in docker-integration-tests~~ [SPARK-29546][TESTS][test-hadoop3.2][test-java11] Recover jersey-guava test dependency in docker-integration-tests Oct 22, 2019

Update

fafb24c

dongjoon-hyun changed the title ~~[SPARK-29546][TESTS][test-hadoop3.2][test-java11] Recover jersey-guava test dependency in docker-integration-tests~~ [SPARK-29546][TESTS] Recover jersey-guava test dependency in docker-integration-tests Oct 23, 2019

maropu approved these changes Oct 23, 2019

View reviewed changes

dongjoon-hyun closed this in 8c34690 Oct 23, 2019

dongjoon-hyun deleted the SPARK-29546 branch October 23, 2019 07:15

srowen reviewed Oct 23, 2019

View reviewed changes

[SPARK-29546][TESTS] Recover jersey-guava test dependency in docker-integration-tests #26203

[SPARK-29546][TESTS] Recover jersey-guava test dependency in docker-integration-tests #26203

Uh oh!

Conversation

dongjoon-hyun commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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 Oct 22, 2019

Uh oh!

dongjoon-hyun commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dongjoon-hyun commented Oct 22, 2019

Uh oh!

dongjoon-hyun commented Oct 22, 2019

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

SparkQA commented Oct 22, 2019

Uh oh!

dongjoon-hyun commented Oct 22, 2019

Uh oh!

srowen commented Oct 22, 2019

Uh oh!

dongjoon-hyun commented Oct 22, 2019

Uh oh!

srowen commented Oct 22, 2019

Uh oh!

dongjoon-hyun commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Oct 23, 2019

Uh oh!

maropu commented Oct 23, 2019

Uh oh!

dongjoon-hyun commented Oct 23, 2019

Uh oh!

SparkQA commented Oct 23, 2019

Uh oh!

maropu commented Oct 23, 2019

Uh oh!

maropu left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Oct 23, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dongjoon-hyun commented Oct 22, 2019 •

edited

Loading

dongjoon-hyun commented Oct 22, 2019 •

edited

Loading

dongjoon-hyun commented Oct 23, 2019 •

edited

Loading

maropu left a comment •

edited

Loading