Skip to content

HIVE-23980: Shade Guava from hive-exec in Hive branch-2 - #1397

Merged
sunchao merged 2 commits into
apache:branch-2from
viirya:shading-guava-branch-2
Aug 18, 2020
Merged

HIVE-23980: Shade Guava from hive-exec in Hive branch-2#1397
sunchao merged 2 commits into
apache:branch-2from
viirya:shading-guava-branch-2

Conversation

@viirya

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

This PR proposes to shade Guava from hive-exec in Hive branch-2. This is basically for triggering test for #1356.

Why are the changes needed?

When trying to upgrade Guava in Spark, found the following error. A Guava method became package-private since Guava version 20. So there is incompatibility with Guava versions > 19.0.

sbt.ForkMain$ForkError: sbt.ForkMain$ForkError: java.lang.IllegalAccessError: tried to access method com.google.common.collect.Iterators.emptyIterator()Lcom/google/common/collect/UnmodifiableIterator; from class org.apache.hadoop.hive.ql.exec.FetchOperator
at org.apache.hadoop.hive.ql.exec.FetchOperator.<init>(FetchOperator.java:108)
at org.apache.hadoop.hive.ql.exec.FetchTask.initialize(FetchTask.java:87)
at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:541)
at org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:1317)
at org.apache.hadoop.hive.ql.Driver.runInternal(Driver.java:1457)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1237)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1227)

This is a problem for downstream clients. Hive project noticed that problem too in HIVE-22126, however that only targets 4.0.0. It'd be nicer if we can also shade Guava from current Hive versions, e.g. Hive 2.3 line.

Does this PR introduce any user-facing change?

Yes. Guava will be shaded from hive-exec.

How was this patch tested?

Built Hive locally and checked jar content.

@viirya

Copy link
Copy Markdown
MemberAuthor

cc @sunchao

@sunchao

Copy link
Copy Markdown
Member

@viirya seems there is no new testing failing which is good.

@viirya

Copy link
Copy Markdown
MemberAuthor

@sunchao But I saw some test failures?

There are 0 new tests failing, 636 existing failing and 231 skipped.

What existing failing means?

@sunchao

Copy link
Copy Markdown
Member

@viirya it probably means those tests were already failing previously. However I was not able to find a history for this. According to this comment, it seems many tests were failing after we enabled CI for branch-2.

cc @belugabehr - wonder if you have any info on this. Thanks.

@viirya

Copy link
Copy Markdown
MemberAuthor

Any idea about which tests I should take a look at?

@sunchao

Copy link
Copy Markdown
Member

@viirya I see most of these test failures are related to Calcite so seems not related, and they were failing previously. Maybe we can trigger the tests again by force push. Also it'd be great if we can verify locally by selecting a few test before and after the PR.

@viirya

Copy link
Copy Markdown
MemberAuthor

I tried to run failed tests locally.

mvn clean -Dtest=org.apache.hive.hcatalog.pig.TestParquetHCatStorer test
------------------------------------------------------- T E S T S ------------------------------------------------------- OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=256M; support was removed in 8.0 Running org.apache.hive.hcatalog.pig.TestParquetHCatStorer Tests run: 27, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 106.145 sec - in org.apache.hive.hcatalog.pig.TestParquetHCatStorer Results :
Tests run: 27, Failures: 0, Errors: 0, Skipped: 0
mvn clean -Dtest=org.apache.hive.hcatalog.mapreduce.TestHCatDynamicPartitioned test
-------------------------------------------------------
T E S T S
-------------------------------------------------------
OpenJDK 64-Bit Server VM warning: ignoring option MaxPermSize=256M; support was removed in 8.0
Running org.apache.hive.hcatalog.mapreduce.TestHCatDynamicPartitioned
Tests run: 14, Failures: 0, Errors: 0, Skipped: 4, Time elapsed: 143.976 sec - in org.apache.hive.hcatalog.mapreduce.TestHCatDynamicPartitioned
Results :
Tests run: 14, Failures: 0, Errors: 0, Skipped: 4

But I cannot reproduce the test failure.

@viirya

Copy link
Copy Markdown
MemberAuthor

@sunchao Can you reproduce the test failure locally?

@sunchao

Copy link
Copy Markdown
Member

@sunchao Can you reproduce the test failure locally?

NO I took two failed tests and can't reproduce them locally, with and without your PR.

@viirya

Copy link
Copy Markdown
MemberAuthor

@sunchao Can you reproduce the test failure locally?

NO I took two failed tests and can't reproduce them locally, with and without your PR.

Hm, it makes harder to debug...

@sunchao

Copy link
Copy Markdown
Member

Yeah, however I think this PR is trivial enough and I don't see the tests are related.

@sunchaosunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@sunchao
sunchao merged commit 8ee71e5 into apache:branch-2Aug 18, 2020
sunchao pushed a commit that referenced this pull request Aug 18, 2020
sunchao added a commit to sunchao/hive that referenced this pull request Sep 1, 2020
sunchao added a commit that referenced this pull request Sep 3, 2020
…g-Chi, reviewed by Chao Sun) (#1397)"
This reverts commit 82b8bbf. Seems it is causing some unit test failures.
sunchao added a commit that referenced this pull request Sep 3, 2020
…g-Chi, reviewed by Chao Sun) (#1397)"
This reverts commit 8ee71e5. Seems it is causing some unit test failures.
Sign up for freeto 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.

3 participants

@viirya@sunchao@kgyrtkirk