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.

Sign up for GitHub

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

HBASE-28879 Bump hbase-thirdparty to 4.1.9 #6295

Merged
merged 5 commits into from
Sep 30, 2024
Merged

HBASE-28879 Bump hbase-thirdparty to 4.1.9 #6295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a51c132
Test with hbase-thirdparty 4.1.9RC0
ndimiduk Sep 25, 2024
7373efd
bump protobuf version in hbase-protocol-shaded and hbase-examples
ndimiduk Sep 25, 2024
a8a25b6
reformat protobuf comment
ndimiduk Sep 25, 2024
623cfb3
spotless:apply
ndimiduk Sep 26, 2024
a37b900
remove staging repository
ndimiduk Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions hbase-examples/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@
<name>Apache HBase - Examples</name>
<description>Examples of HBase usage</description>
<properties>
<!--Version of protobuf that hbase uses internally (we shade our pb)
Must match what is out in hbase-thirdparty include.
<!--
Version of protobuf that hbase uses internally (we shade our pb) Must match what is out
in hbase-thirdparty include.
-->
<internal.protobuf.version>4.27.3</internal.protobuf.version>
<internal.protobuf.version>4.28.2</internal.protobuf.version>
</properties>
<dependencies>
<dependency>
Expand Down
7 changes: 4 additions & 3 deletions hbase-protocol-shaded/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@
<description>Shaded protobuf protocol classes used by HBase internally.</description>
<properties>
<maven.javadoc.skip>true</maven.javadoc.skip>
<!--Version of protobuf that hbase uses internally (we shade our pb)
Must match what is out in hbase-thirdparty include.
<!--
Version of protobuf that hbase uses internally (we shade our pb) Must match what is out
in hbase-thirdparty include.
-->
<internal.protobuf.version>4.27.3</internal.protobuf.version>
<internal.protobuf.version>4.28.2</internal.protobuf.version>
</properties>
<dependencies>
<!--BE CAREFUL! Any dependency added here needs to be
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@
databind] must be kept in sync with the version of jackson-jaxrs-json-provider shipped in
hbase-thirdparty.
-->
<hbase-thirdparty.version>4.1.8</hbase-thirdparty.version>
<hbase-thirdparty.version>4.1.9</hbase-thirdparty.version>
Copy link
Contributor

Choose a reason for hiding this comment

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

we may also want to sync versions of error prone and netty4 with hbase-thirdparty

Copy link
Member Author

Choose a reason for hiding this comment

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

This is getting out of hand. See how many fix-up commits I had to push to various projects? We should be managing all these version numbers with a BOM import.

This blog post has a nice write-up on the idea, https://www.garretwilson.com/blog/2023/06/14/improve-maven-bom-pattern

I think that hbase-thirdparty could publish a BOM pom file that can be imported into any of the downstream hbase projects that make use of that release of hbase-thirdparty. That will centralize management of these dependencies in the hbase-thirdparty repo, and we won't have to play whack-a-mole in the main project poms.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I was thinking about the same. Because it does not make sense to waste time doing this and still miss out on one or the another. We already have something similar for org.mockito:mockito-bom:

hbase/pom.xml

Line 1626 in 449c446

<artifactId>mockito-bom</artifactId>

Copy link
Contributor

@NihalJain NihalJain Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We should file a jira to handle this. At least will have a better way of doing thirdparty change with a one liner change with next thirdparty release, which is how it should have been ideally.

I can help with implementing this, please lmk if you do not plan to fix this yourself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Filed https://issues.apache.org/jira/browse/HBASE-28883

Copy link
Contributor

Choose a reason for hiding this comment

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

Updating error prone is not trivial, sometimes it may cause some compile errors since they may introduced some new check rules, so usually I will open a new issue for updating error prone.

Copy link
Contributor

Choose a reason for hiding this comment

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

And on netty4 dependencies, hbase does not depend on it directly since we use the shaded one in hbase thirdparty, it is mainly introduced by hadoop and zookeeper. And in hadoop 3.4.0, IIRC hadoop also shaded and relocated netty in their thirdparty jar, so maybe we need to discuss whether we will need to force netty dependencies in hbase, maybe just leave it as is.

Copy link
Contributor

Choose a reason for hiding this comment

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

And on netty4 dependencies, hbase does not depend on it directly since we use the shaded one in hbase thirdparty, it is mainly introduced by hadoop and zookeeper. And in hadoop 3.4.0, IIRC hadoop also shaded and relocated netty in their thirdparty jar, so maybe we need to discuss whether we will need to force netty dependencies in hbase, maybe just leave it as is.

So we need not necessarily align netty with thirdparty.
Same for error prone.

Filed https://issues.apache.org/jira/browse/HBASE-28883

Please see apache/hbase-thirdparty#124, NihalJain@0b51c71

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay so we do not need to maintain alignment on netty or errorprone? Jackson and protobuf are the only two that require alignment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me explain more clear.

For error prone, we'd better also bump the error prone to the same version with thirparty, but it is not trivial sometimes, and since hbase-thirdparty only depends on the annotation jar(even not shaded), so it is not likely to introduce any conflicts. So I think it is better to have a separated issue for bumping it, after upgrading the hbase-thirdparty.

For netty, since hbase does not depend on netty4 directly, we do not need to align the netty version with the one in hbase-thirdparty.
We maintain it in our pom is because the conflicts between zookeeper and hadoop. So if there are no CVEs for netty, we do not need to bump it in hbase. And after hadoop 3.4.0, since hadoop also shade netty(IIRC), maybe we even do not need to do this any more. If there are new CVEs for netty, maybe we just need to bump the zookeeper dependency?

<!-- Coverage properties -->
<jacoco.version>0.8.8</jacoco.version>
<jacocoArgLine/>
Expand Down