Skip to content

HBASE-26622 Update error-prone to 2.10 - #3979

Closed
madrob wants to merge 2 commits into
apache:masterfrom
madrob:HBASE-26622
Closed

HBASE-26622 Update error-prone to 2.10#3979
madrob wants to merge 2 commits into
apache:masterfrom
madrob:HBASE-26622

Conversation

@madrob

@madrobmadrob commented Dec 23, 2021•
edited
Loading

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/HBASE-26622

Tested via mvn package -DskipTests -PerrorProne - it probably won't build with Java 8, but it's an opt-in profile so I'm not worried about strict compatibility. Did not run unit tests.

There are (still? newly?) hundreds of warnings that y'all can sort through as follow-on work.

@Apache-HBase

Apache-HBase commented Dec 23, 2021

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 27sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep1m 6sMaven dependency ordering for branch
+1 💚mvninstall3m 59smaster passed
+1 💚compile2m 49smaster passed
+1 💚shadedjars8m 22sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc3m 41smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for patch
+1 💚mvninstall3m 48sthe patch passed
+1 💚compile2m 50sthe patch passed
+1 💚javac2m 50sthe patch passed
+1 💚shadedjars8m 30spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc3m 36sthe patch passed
_ Other Tests _
-1 ❌unit28m 5sroot in the patch failed.
69m 45s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#3979
Optional Testsjavac javadoc unit shadedjars compile
unameLinux c4bc3f5a461b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 70703ca
Default JavaAdoptOpenJDK-1.8.0_282-b08
unithttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/testReport/
Max. process+thread count1122 (vs. ulimit of 30000)
modulesC: hbase-build-configuration hbase-common hbase-asyncfs hbase-server . U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Dec 23, 2021

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 32sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 54sMaven dependency ordering for branch
+1 💚mvninstall5m 10smaster passed
+1 💚compile3m 34smaster passed
+1 💚shadedjars9m 56sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc5m 4smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 24sMaven dependency ordering for patch
+1 💚mvninstall5m 2sthe patch passed
+1 💚compile3m 39sthe patch passed
+1 💚javac3m 38sthe patch passed
+1 💚shadedjars9m 26spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc4m 53sthe patch passed
_ Other Tests _
-1 ❌unit23m 51sroot in the patch failed.
74m 38s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#3979
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 0b86e1f59970 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 70703ca
Default JavaAdoptOpenJDK-11.0.10+9
unithttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-root.txt
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/testReport/
Max. process+thread count838 (vs. ulimit of 30000)
modulesC: hbase-build-configuration hbase-common hbase-asyncfs hbase-server . U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Dec 23, 2021

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 1sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗mvndep0m 52sMaven dependency ordering for branch
+1 💚mvninstall4m 4smaster passed
+1 💚compile8m 57smaster passed
+1 💚checkstyle2m 2smaster passed
+1 💚spotbugs13m 23smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for patch
+1 💚mvninstall3m 50sthe patch passed
-1 ❌compile0m 24sroot in the patch failed.
-0 ⚠️javac0m 24sroot in the patch failed.
-0 ⚠️checkstyle2m 0sroot: The patch generated 1 new + 81 unchanged - 1 fixed = 82 total (was 82)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚xml0m 3sThe patch has no ill-formed XML file.
+1 💚hadoopcheck19m 34sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotbugs14m 21sthe patch passed
_ Other Tests _
+1 💚asflicense1m 10sThe patch does not generate ASF License warnings.
81m 20s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#3979
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile xml
unameLinux f390188fd770 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 70703ca
Default JavaAdoptOpenJDK-1.8.0_282-b08
compilehttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-general-check/output/patch-compile-root.txt
javachttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-general-check/output/patch-compile-root.txt
checkstylehttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/artifact/yetus-general-check/output/diff-checkstyle-root.txt
Max. process+thread count141 (vs. ulimit of 30000)
modulesC: hbase-build-configuration hbase-common hbase-asyncfs hbase-server . U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/1/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Dec 23, 2021

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 2sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚hbaseanti0m 0sPatch does not have any anti-patterns.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for branch
+1 💚mvninstall3m 48smaster passed
+1 💚compile8m 58smaster passed
+1 💚checkstyle1m 57smaster passed
+1 💚spotbugs15m 57smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for patch
+1 💚mvninstall3m 50sthe patch passed
-1 ❌compile0m 22sroot in the patch failed.
-0 ⚠️javac0m 22sroot in the patch failed.
-0 ⚠️checkstyle1m 58sroot: The patch generated 1 new + 84 unchanged - 1 fixed = 85 total (was 85)
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚xml0m 3sThe patch has no ill-formed XML file.
+1 💚hadoopcheck19m 18sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotbugs17m 47sthe patch passed
_ Other Tests _
+1 💚asflicense1m 47sThe patch does not generate ASF License warnings.
87m 43s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#3979
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile xml
unameLinux eeb01cb91ce4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 70703ca
Default JavaAdoptOpenJDK-1.8.0_282-b08
compilehttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/artifact/yetus-general-check/output/patch-compile-root.txt
javachttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/artifact/yetus-general-check/output/patch-compile-root.txt
checkstylehttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/artifact/yetus-general-check/output/diff-checkstyle-root.txt
Max. process+thread count141 (vs. ulimit of 30000)
modulesC: hbase-build-configuration hbase-common hbase-asyncfs hbase-server hbase-mapreduce hbase-thrift hbase-endpoint . U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/console
versionsgit=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Dec 24, 2021

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 25sDocker mode activated.
-0 ⚠️yetus0m 3sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for branch
+1 💚mvninstall4m 39smaster passed
+1 💚compile3m 19smaster passed
+1 💚shadedjars8m 29sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc6m 11smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 21sMaven dependency ordering for patch
+1 💚mvninstall4m 43sthe patch passed
+1 💚compile3m 21sthe patch passed
+1 💚javac3m 21sthe patch passed
+1 💚shadedjars8m 37spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc6m 7sthe patch passed
_ Other Tests _
+1 💚unit192m 53sroot in the patch passed.
243m 57s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#3979
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 8b975504c594 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / 70703ca
Default JavaAdoptOpenJDK-11.0.10+9
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/testReport/
Max. process+thread count6922 (vs. ulimit of 30000)
modulesC: hbase-build-configuration hbase-common hbase-asyncfs hbase-server hbase-mapreduce hbase-thrift hbase-endpoint . U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9

Apache9 commented Dec 27, 2021

Copy link
Copy Markdown
Contributor

Long time no see @madrob

Seems there is something wrong while building with error prone enabled?

@madrob

madrob commented Dec 27, 2021

Copy link
Copy Markdown
ContributorAuthor

Yes, the latest version doesn’t work with java 8. Still should fix the other bugs tho

@Apache9

Apache9 commented Dec 27, 2021

Copy link
Copy Markdown
Contributor

Maybe we could change the pre commit compile stage to use java11 with --release=8?

ndimiduk
ndimiduk requested changes Jan 5, 2022
View reviewed changes

@ndimidukndimiduk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is great @madrob !

Comment threadhbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
*/
@Override
public KeyValue clone() throws CloneNotSupportedException {
super.clone();

@ndimidukndimidukJan 5, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

wtf was this even doing?

Comment threadhbase-build-configuration/pom.xml
<activation>
<activeByDefault>false</activeByDefault>
</activation>
<properties>

@ndimidukndimidukJan 5, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since this check is run by precommit testing only, I'm fine with bumping this check over to JDK11. Please make sure that test-patch is calling it from the jdk11 incantation instead of the jdk8 incantation.

@ndimiduk

ndimiduk commented Jan 5, 2022

Copy link
Copy Markdown
Member

Maybe we could change the pre commit compile stage to use java11 with --release=8?

Doesn't work, #3974

@ndimiduk

ndimiduk commented Jan 5, 2022

Copy link
Copy Markdown
Member

Yes, the latest version doesn’t work with java 8. Still should fix the other bugs tho

Per my read of https://errorprone.info/docs/installation#jdk-8, 2.10.0 is the last version with support for JDK8.

@ndimidukndimiduk mentioned this pull request Jan 6, 2022
Merged
@ndimiduk

ndimiduk commented Jan 12, 2022

Copy link
Copy Markdown
Member

@madrob I applied this patch but reverted your changes to hbase-build-configuration/pom.xml. I then reproduced the ErrorProne violation from #3968. The upgraded version of ErrorProne fails with a meaningful error message. Which means, the upgrade here does fix the the pre-commit issue I saw over there.

[ERROR] /Users/ndimiduk/repos/apache/hbase/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java:[1604,50] error: [RestrictedApi] Should only be called in self or ZKUtil

Please revert the change to hbase-build-configuration/pom.xml, keeping ErrorProne configured for JDK8, and you have my +1 on this change.

If you want me to take it over, let me know.

Thanks for the drive-by 👍

@ndimiduk

ndimiduk commented Jan 12, 2022

Copy link
Copy Markdown
Member

Also, the checkstyle violation reported by the build bot is valid, https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3979/2/artifact/yetus-general-check/output/diff-checkstyle-root.txt

@madrob

madrob commented Jan 12, 2022

Copy link
Copy Markdown
ContributorAuthor

Fair Nick, if you can find the time
To drive this task complete with care
Then I would much be in your debt
And credit for the work we'd share

madrob
madrob commented Jan 12, 2022
View reviewed changes
Comment threadhbase-common/src/main/java/org/apache/hadoop/hbase/KeyValue.java
System.arraycopy(this.bytes, this.offset, b, 0, this.length);
KeyValue ret = new KeyValue(b, 0, b.length);
KeyValue ret = (KeyValue) super.clone();
ret.bytes = new byte[this.length];

@madrobmadrobJan 12, 2022

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Maybe simpler to use Arrays.copyOfRange here?

@ndimidukndimiduk mentioned this pull request Feb 24, 2022
Merged
@ndimiduk

ndimiduk commented Mar 2, 2022

Copy link
Copy Markdown
Member

Resolved via #4133

@ndimidukndimiduk closed this Mar 2, 2022
@ndimiduk

ndimiduk commented Mar 2, 2022

Copy link
Copy Markdown
Member

Thanks for getting us started, @madrob !

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

Reviewers

@ndimidukndimidukndimiduk requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@madrob@Apache-HBase@Apache9@ndimiduk