Skip to content

Conversation

@shangxinli
Copy link
Collaborator

@shangxinli shangxinli commented Jul 31, 2021

Test plan - (Please fill in how you tested your changes)

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
Upgrade hive-apache to new version 3.0.0-6

Hive Changes
Fix NoClassDefFoundError for JsonNode

@shangxinli
Copy link
Collaborator Author

shangxinli commented Aug 6, 2021

To solve the test error below, we need to make several other changes in Hive and presto-hive-apache repro.

java.lang.NoClassDefFoundError: com/facebook/presto/hive/$internal/org/codehaus/jackson/JsonNode
presto-worker_1 | at org.apache.hadoop.hive.serde2.avro.AvroSerDe.getSchemaFromCols(AvroSerDe.java:178)

  1. We need the PR to fix the above failure.
  2. The commit need to be ported to Hive 3.0+( presto-hive-apache uses 3.0.0-5). And then release a new version of Hive 3.0+, and then upgrade presto-hive-apache.

@beinan
Copy link
Member

beinan commented Aug 20, 2021

Thank you! Looks like this one skipped quite a few test, I might hesitate to merge it. Not sure the impact on FB side.

btw, could you please update the release note section in this PR? Thanks!

@shangxinli
Copy link
Collaborator Author

shangxinli commented Aug 23, 2021

Thanks @beinan for having a look! Yeah, this PR disabled quite a few tests. I am not sure if that is acceptable. The root cause is the new version of Avro changed the signature of Avro Schema(not backward compatible), and Hive has a hard dependency on the old signature. A fix in Hive is needed and that is done in Hive 2.x.x but not Hive 3.x.x, but our Hive-apache repo is using Hive 3.x.x.

So there are 3 solutions now. 1) Push Hive community to port the fix from 2.x.x to 3.x.x and have a new release. 2) Revert #15805 to let Presto have its own Parquet upgrading path independent of the hive-apache repo. 3) Disable tests(not sure if real user scenarios also having issues).

#1 seems a better solution but it seems the Hive community has no interest, for now, to help on releasing Hive 3.x.x. I started an email thread for asking Hive community but not response.

@beinan
Copy link
Member

beinan commented Aug 23, 2021

Thanks @beinan for having a look! Yeah, this PR disabled quite a few tests. I am not sure if that is acceptable. The root cause is the new version of Avro changed the signature of Avro Schema(not backward compatible), and Hive has a hard dependency on the old signature. A fix in Hive is needed and that is done in Hive 2.x.x but not Hive 3.x.x, but our Hive-apache repo is using Hive 3.x.x.

So there are 3 solutions now. 1) Push Hive community to port the fix from 2.x.x to 3.x.x and have a new release. 2) Revert #15805 to let Presto have its own Parquet upgrading path independent of the hive-apache repo. 3) Disable tests(not sure if real user scenarios also having issues).

#1 seems a better solution but it seems the Hive community has no interest, for now, to help on releasing Hive 3.x.x. I started an email thread for asking Hive community but not response.

Thanks a lot for these solutions!
Any chance to override the change you made in hive in presto-hive-apache? we might be able to put AvroDeserializer.java into here https://github.com/prestodb/presto-hive-apache/tree/master/src/main/java/org/apache/hadoop/hive/serde2/avro
I'm just guessing, not sure if it works or not. Thanks!

@shangxinli
Copy link
Collaborator Author

shangxinli commented Aug 25, 2021

@beinan The change I made in pesto-hive-apache helped JsonNode no class finding error. Now the failing tests are failing with the following stack.

at org.apache.hadoop.hive.serde2.avro.TypeInfoToSchema.createAvroField(TypeInfoToSchema.java:76)
at org.apache.hadoop.hive.serde2.avro.TypeInfoToSchema.convert(TypeInfoToSchema.java:61)
at org.apache.hadoop.hive.serde2.avro.AvroSerDe.getSchemaFromCols(AvroSerDe.java:179)
at org.apache.hadoop.hive.serde2.avro.AvroSerDe.initialize(AvroSerDe.java:117)
at com.facebook.presto.hive.HiveUtil.initializeDeserializer(HiveUtil.java:496)
at com.facebook.presto.hive.HiveUtil.getDeserializer(HiveUtil.java:456)
at com.facebook.presto.hive.GenericHiveRecordCursor.<init>(GenericHiveRecordCursor.java:144)
at com.facebook.presto.hive.GenericHiveRecordCursorProvider.lambda$createRecordCursor$1(GenericHiveRecordCursorProvider.java:79)
at com.facebook.presto.hive.authentication.NoHdfsAuthentication.doAs(NoHdfsAuthentication.java:23)
at com.facebook.presto.hive.HdfsEnvironment.doAs(HdfsEnvironment.java:81)
at com.facebook.presto.hive.GenericHiveRecordCursorProvider.createRecordCursor(GenericHiveRecordCursorProvider.java:75)
at com.facebook.presto.hive.HivePageSourceProvider.createHivePageSource(HivePageSourceProvider.java:466)
at com.facebook.presto.hive.HivePageSourceProvider.createPageSource(HivePageSourceProvider.java:184)
at com.facebook.presto.spi.connector.classloader.ClassLoaderSafeConnectorPageSourceProvider.createPageSource(ClassLoaderSafeConnectorPageSourceProvider.java:63)
at com.facebook.presto.split.PageSourceManager.createPageSource(PageSourceManager.java:80)
at com.facebook.presto.operator.TableScanOperator.getOutput(TableScanOperator.java:249)
at com.facebook.presto.operator.Driver.processInternal(Driver.java:418)
at com.facebook.presto.operator.Driver.lambda$processFor$9(Driver.java:301)
at com.facebook.presto.operator.Driver.tryWithLock(Driver.java:722)
at com.facebook.presto.operator.Driver.processFor(Driver.java:294)
at com.facebook.presto.execution.SqlTaskExecution$DriverSplitRunner.processFor(SqlTaskExecution.java:1077)
at com.facebook.presto.execution.executor.PrioritizedSplitRunner.process(PrioritizedSplitRunner.java:162)
at com.facebook.presto.execution.executor.TaskExecutor$TaskRunner.run(TaskExecutor.java:599)
at com.facebook.presto.$gen.Presto_0_261_SNAPSHOT_280493b____20210824_183044_1.run(Unknown Source)

So I am not totally getting your point. Can you explain?

@beinan
Copy link
Member

beinan commented Aug 27, 2021

@beinan The change I made in pesto-hive-apache helped JsonNode no class finding error. Now the failing tests are failing with the following stack.

at org.apache.hadoop.hive.serde2.avro.TypeInfoToSchema.createAvroField(TypeInfoToSchema.java:76)
at org.apache.hadoop.hive.serde2.avro.TypeInfoToSchema.convert(TypeInfoToSchema.java:61)
at org.apache.hadoop.hive.serde2.avro.AvroSerDe.getSchemaFromCols(AvroSerDe.java:179)
at org.apache.hadoop.hive.serde2.avro.AvroSerDe.initialize(AvroSerDe.java:117)
at com.facebook.presto.hive.HiveUtil.initializeDeserializer(HiveUtil.java:496)
at com.facebook.presto.hive.HiveUtil.getDeserializer(HiveUtil.java:456)
at com.facebook.presto.hive.GenericHiveRecordCursor.<init>(GenericHiveRecordCursor.java:144)
at com.facebook.presto.hive.GenericHiveRecordCursorProvider.lambda$createRecordCursor$1(GenericHiveRecordCursorProvider.java:79)
at com.facebook.presto.hive.authentication.NoHdfsAuthentication.doAs(NoHdfsAuthentication.java:23)
at com.facebook.presto.hive.HdfsEnvironment.doAs(HdfsEnvironment.java:81)
at com.facebook.presto.hive.GenericHiveRecordCursorProvider.createRecordCursor(GenericHiveRecordCursorProvider.java:75)
at com.facebook.presto.hive.HivePageSourceProvider.createHivePageSource(HivePageSourceProvider.java:466)
at com.facebook.presto.hive.HivePageSourceProvider.createPageSource(HivePageSourceProvider.java:184)
at com.facebook.presto.spi.connector.classloader.ClassLoaderSafeConnectorPageSourceProvider.createPageSource(ClassLoaderSafeConnectorPageSourceProvider.java:63)
at com.facebook.presto.split.PageSourceManager.createPageSource(PageSourceManager.java:80)
at com.facebook.presto.operator.TableScanOperator.getOutput(TableScanOperator.java:249)
at com.facebook.presto.operator.Driver.processInternal(Driver.java:418)
at com.facebook.presto.operator.Driver.lambda$processFor$9(Driver.java:301)
at com.facebook.presto.operator.Driver.tryWithLock(Driver.java:722)
at com.facebook.presto.operator.Driver.processFor(Driver.java:294)
at com.facebook.presto.execution.SqlTaskExecution$DriverSplitRunner.processFor(SqlTaskExecution.java:1077)
at com.facebook.presto.execution.executor.PrioritizedSplitRunner.process(PrioritizedSplitRunner.java:162)
at com.facebook.presto.execution.executor.TaskExecutor$TaskRunner.run(TaskExecutor.java:599)
at com.facebook.presto.$gen.Presto_0_261_SNAPSHOT_280493b____20210824_183044_1.run(Unknown Source)

So I am not totally getting your point. Can you explain?

Sorry for my last misleading comment.
I mean we can shift the change you made in hive to presto-hive-apache. Because we can override hive's class in pesto-hive-apache

@shangxinli
Copy link
Collaborator Author

shangxinli commented Aug 27, 2021

@beinan Thanks for the explanation! The change is inside the Hive repo for the file TypeInfoToSchema.java (you can refer above call stack). If we want to shift the change to presto-hive-apache, we would need to copy all the code from hive.serde or at least hive.serde2.avro, right? Or do you suggest we can shade Hive 2.x.x and in HiveUtil.java we only call the shaded one?

BTW, if you have some ideas to implement it, feel free to take them over. I saw you have another PR for the same upgrading too.

@shangxinli
Copy link
Collaborator Author

The way I can think of is to shade the Hive jar. 1) Create a new project which references hive-2.3.8 and shade it 2) Include this project in presto-hive-apache 3) Change presto to reference the shaded class

@beinan
Copy link
Member

beinan commented Aug 27, 2021

The way I can think of is to shade the Hive jar. 1) Create a new project which references hive-2.3.8 and shade it 2) Include this project in presto-hive-apache 3) Change presto to reference the shaded class
gotcha, let me try something in this weekend and get back to you:)

@shangxinli
Copy link
Collaborator Author

Did you find something @beinan?

@shangxinli
Copy link
Collaborator Author

The way I can think of is to shade the Hive jar. 1) Create a new project which references hive-2.3.8 and shade it 2) Include this project in presto-hive-apache 3) Change presto to reference the shaded class
gotcha, let me try something in this weekend and get back to you:)

Do you have updates @beinan?

@beinan
Copy link
Member

beinan commented Sep 10, 2021

The way I can think of is to shade the Hive jar. 1) Create a new project which references hive-2.3.8 and shade it 2) Include this project in presto-hive-apache 3) Change presto to reference the shaded class
gotcha, let me try something in this weekend and get back to you:)

Do you have updates @beinan?

Hi @shangxinli , sorry for my delay, I post a PR for this issue prestodb/presto-hive-apache#51
I copied some of classes from hive and made change on it. The pr fixed some of the unit tests, but there still are tests failing. Could you help take a look? I think you would be more familiar with avro stuff. Thanks a lot! Feel free take over the PR if you got some ideas.

@shangxinli shangxinli changed the title Upgrade hive-apache to new version 3.0.0-5 Upgrade hive-apache to new version 3.0.0-6 Sep 17, 2021
@aweisberg
Copy link
Contributor

I'll take a look at this soon and try deploying the whole thing.

@aweisberg
Copy link
Contributor

I deployed this to a cluster and ran 20k test queries. There was one failure that I haven't resolved yet, but I am guessing it was transient and specific to that query and am retesting it now.

@beinan
Copy link
Member

beinan commented Oct 5, 2021

I deployed this to a cluster and ran 20k test queries. There was one failure that I haven't resolved yet, but I am guessing it was transient and specific to that query and am retesting it now.

Thank you @aweisberg ! any updates? looks like the ci is still failing.

@aweisberg
Copy link
Contributor

There is something wrong with snapshots. I can upload them and even see them in https://oss.sonatype.org/#nexus-search;gav~com.facebook.presto.hive~hive-apache~~~~kw,versionexpand, but I can't get them to download.

I also get the error

Unable to identify component: org.sonatype.nexus.proxy.ItemNotFoundException: Path com/facebook/presto/hive/hive-apache/3.0.0-7-SNAPSHOT/hive-apache-3.0.0-7-20211006.154738-15.jar not found in local storage of repository "Snapshots" [id=snapshots]

When I try to look at component info in the Nexus web UI.

How snapshots are deployed is defined in airbase and it looks correct.

    <repositories>
        <repository>
            <id>sonatype-nexus-snapshots</id>
            <name>Sonatype Nexus Snapshots</name>
            <url>https://oss.sonatype.org/content/repositories/snapshots</url>
            <releases>
                <enabled>false</enabled>
            </releases>
            <snapshots>
                <enabled>true</enabled>
            </snapshots>
        </repository>
    </repositories>

The build tries to fetch from that location

[ERROR] Failed to execute goal on project presto-hive-metastore: Could not resolve dependencies for project com.facebook.presto:presto-hive-metastore:jar:0.264-SNAPSHOT: Could not find artifact com.facebook.presto.hive:hive-apache:jar:3.0.0-7-20211006.154738-15 in sonatype-nexus-snapshots (https://oss.sonatype.org/content/repositories/snapshots) -> [Help 1]
org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal on project presto-hive-metastore: Could not resolve dependencies for project com.facebook.presto:presto-hive-metastore:jar:0.264-SNAPSHOT: Could not find artifact com.facebook.presto.hive:hive-apache:jar:3.0.0-7-20211006.154738-15 in sonatype-nexus-snapshots (https://oss.sonatype.org/content/repositories/snapshots)

@aweisberg
Copy link
Contributor

@aweisberg
Copy link
Contributor

I think it's working now. I had to deploy a few times to get the timestamps to match.

@beinan
Copy link
Member

beinan commented Oct 6, 2021

Awesome!!! @aweisberg Thank you!

Hello @shangxinli, looks like all good except the iceberg connector one, I will take a look soon.

@beinan
Copy link
Member

beinan commented Oct 6, 2021

Awesome!!! @aweisberg Thank you!

Hello @shangxinli, looks like all good except the iceberg connector one, I will take a look soon.

@shangxili I'm not sure the iceberg test is just flaky or not, could you rebase the master and push again? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants