Skip to content
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

HIVE-27585 Upgrade kryo serialization lib to latest version #4570

Merged
merged 2 commits into from Aug 22, 2023

Conversation

suprithcs
Copy link
Contributor

@suprithcs suprithcs commented Aug 9, 2023

What changes were proposed in this pull request?

Upgrade kryo serialization lib to latest version.
https://issues.apache.org/jira/browse/HIVE-27585

Why are the changes needed?

Consumes some latest improvements and bug fixes
EsotericSoftware/kryo@kryo-parent-5.2.0...kryo-parent-5.5.0.

Does this PR introduce any user-facing change?

No

Is the change a dependency upgrade?

Yes. Attached file

How was this patch tested?

Existing tests

@suprithcs
Copy link
Contributor Author

mvn_dependency_tree.txt

@sonarcloud
Copy link

sonarcloud bot commented Aug 9, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

warning The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@suprithcs
Copy link
Contributor Author

@ayushtkn PTAL if you don't mind

Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

LGTM

@suprithcs
Copy link
Contributor Author

@ayushtkn Thank you for the quick review, when do you plan to merge this?

@@ -29,7 +29,7 @@
<kryo-shaded.version>4.0.3</kryo-shaded.version>
<iceberg.mockito-core.version>3.4.4</iceberg.mockito-core.version>
<iceberg.avro.version>1.11.1</iceberg.avro.version>
<iceberg.kryo.version>5.2.0</iceberg.kryo.version>
<iceberg.kryo.version>5.5.0</iceberg.kryo.version>
Copy link
Member

Choose a reason for hiding this comment

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

this may break hive - spark interoperability if spark is not on the same version of kryo.
Can you please check once?

@ayushtkn
Copy link
Member

@simhadri-g What version of spark uses? We are already on 5.2.0, shouldn't spark also upgrade if they want to use our latest version of hive.

Copy link

@aturoczy aturoczy left a comment

Choose a reason for hiding this comment

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

+1
Could you please insert the jira ticket in the description?

@simhadri-g
Copy link
Member

simhadri-g commented Aug 22, 2023

Seems like spark is not very keen on upgrading kryo version . They are most likely on 4.0.2 :

  1. https://issues.apache.org/jira/browse/SPARK-40685

  2. https://issues.apache.org/jira/browse/SPARK-34023?focusedCommentId=17259996&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17259996

I think we can go ahead with the upgrade in hive.

Copy link
Member

@simhadri-g simhadri-g left a comment

Choose a reason for hiding this comment

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

LGTM+1

@ayushtkn ayushtkn merged commit 9ec4292 into apache:master Aug 22, 2023
7 checks passed
scarlin-cloudera pushed a commit to scarlin-cloudera/hive that referenced this pull request Aug 29, 2023
…4570). (Suprith Chandrashekharachar, reviewed by Ayush Saxena, Attila Turoczy, Simhadri Govindappa)
tarak271 pushed a commit to tarak271/hive-1 that referenced this pull request Dec 19, 2023
…4570). (Suprith Chandrashekharachar, reviewed by Ayush Saxena, Attila Turoczy, Simhadri Govindappa)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants