Skip to content

upgrading the parquet/avro version#16923

Merged
beinan merged 1 commit intoprestodb:masterfrom
ugurmeet:test-3.0.0-7-aweisberg-SNAPSHOT
Nov 3, 2021
Merged

upgrading the parquet/avro version#16923
beinan merged 1 commit intoprestodb:masterfrom
ugurmeet:test-3.0.0-7-aweisberg-SNAPSHOT

Conversation

@ugurmeet
Copy link
Contributor

@ugurmeet ugurmeet commented Oct 28, 2021

Test plan -
Existing tests

== RELEASE NOTES ==

General Changes
* parquet version upgraded from 1.10.1 (coming via presto-hive-apache 3.0.0-3) to 1.11.1 (coming via presto-hive-apache 3.0.0-7)
* avro version upgraded from 1.8.2 (coming via presto-hive-apache 3.0.0-3) to 1.9.2 (coming via presto-hive-apache 3.0.0-7)

@ugurmeet
Copy link
Contributor Author

@beinan , @shangxinli, Couple of Iceberg test failures now. Any ideas

Failures:
Error: TestIcebergSmoke.testCreateNestedPartitionedTable:611->testWithAllFileFormats:599->testCreateNestedPartitionedTable:636->AbstractTestQueryFramework.assertUpdate:210 » Runtime
Error: TestIcebergSmokeNative.testCreateNestedPartitionedTable:612->testWithAllFileFormats:600->testCreateNestedPartitionedTable:637->AbstractTestQueryFramework.assertUpdate:210 » Runtime
[INFO]
Error: Tests run: 993, Failures: 2, Errors: 0, Skipped: 16

Copy link
Member

Choose a reason for hiding this comment

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

@ugurmeet this change broke the iceberg test, I tried to change it back, then iceberg test looks ok. But looks like it would failed hive test, am I right?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I think the root cause is the parquet version in the root pom of presto is still 1.11.0, and iceberg connector is using parquet version in the root pom somehow. @ugurmeet could you upgrade the parquet in the root pom of presto to 1.11.1? then the issue should be fixed. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot @beinan , all tests are passing now

Copy link
Member

Choose a reason for hiding this comment

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

Thank you so much! You work unblocks a dozen of new features on hive and iceberg connector!

Copy link
Member

@beinan beinan left a comment

Choose a reason for hiding this comment

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

Great work! Thank you!

@beinan
Copy link
Member

beinan commented Nov 1, 2021

@ugurmeet could you change the release note part in the pr? Thanks!

Also we cannot merge it until hive 3.0.0.7 is ready on public maven repo.

@ugurmeet
Copy link
Contributor Author

ugurmeet commented Nov 1, 2021

Thanks @beinan . Updated the Release Notes. Waiting for the release 3.0.0-7 to be created. Will retest once the release jar is created.

@ugurmeet ugurmeet changed the title testing with the 3.0.0-7 presto-hive-apache upgrading the parquet/avro version Nov 1, 2021
@ugurmeet
Copy link
Contributor Author

ugurmeet commented Nov 2, 2021

Hi @beinan, changed the use release 3.0.0-7. Let me know if anything else needs to be done. Thanks

Copy link
Member

@beinan beinan left a comment

Choose a reason for hiding this comment

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

lgtm, @ugurmeet could squash the commits into one?

An example of the commit message:


Foo was pack ratting ByteBuffers causing OOM.

Cherry-pick of https://github.com/foo/bar/pull/123 (https://github.com/foo/bar/pull/123)

Co-authored-by: Foo Bar <foo@bar.com>

@ugurmeet ugurmeet force-pushed the test-3.0.0-7-aweisberg-SNAPSHOT branch from e495f25 to 16b8b8b Compare November 2, 2021 22:46
Copy link
Member

@beinan beinan left a comment

Choose a reason for hiding this comment

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

all good, merging it

@beinan beinan merged commit b6b6234 into prestodb:master Nov 3, 2021
@aweisberg
Copy link
Contributor

Just a heads up the first line of the commit message was too long. It shouldn't wrap in the GH ui :-)

@beinan
Copy link
Member

beinan commented Nov 3, 2021

Just a heads up the first line of the commit message was too long. It shouldn't wrap in the GH ui :-)

Thank you for the reminding! I will take care next time! thanks!

@ugurmeet
Copy link
Contributor Author

ugurmeet commented Nov 3, 2021

Thanks a lot @beinan, @aweisberg for help with debugging and pushing this through.

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