Skip to content

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

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

Merged
merged 2 commits into from
Aug 18, 2020

Conversation

viirya
Copy link
Member

@viirya viirya commented Aug 12, 2020

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
Member Author

viirya commented Aug 12, 2020

cc @sunchao

@sunchao
Copy link
Member

sunchao commented Aug 12, 2020

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

@viirya
Copy link
Member Author

viirya commented Aug 12, 2020

@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
Member

sunchao commented Aug 12, 2020

@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
Member Author

viirya commented Aug 14, 2020

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

@sunchao
Copy link
Member

sunchao commented Aug 15, 2020

@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
Member Author

viirya commented Aug 15, 2020

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
Member Author

viirya commented Aug 16, 2020

@sunchao Can you reproduce the test failure locally?

@sunchao
Copy link
Member

sunchao commented Aug 17, 2020

@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
Member Author

viirya commented Aug 17, 2020

@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
Member

sunchao commented Aug 17, 2020

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

Copy link
Member

@sunchao sunchao left a comment

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-2 Aug 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 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.

3 participants