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

Jump to bottom

Backport "HBASE-24163 MOB compactor implementations should use format specifiers when calling String.format" to branch-2 #4706

Merged
merged 1 commit into from Aug 17, 2022
Merged

Backport "HBASE-24163 MOB compactor implementations should use format specifiers when calling String.format" to branch-2 #4706

merged 1 commit into from Aug 17, 2022

Conversation

ndimiduk
Copy link
Member

Signed-off-by: stack stack@apache.org
Signed-off-by: Viraj Jasani vjasani@apache.org
Signed-off-by: Josh Elser elserj@apache.org

@busbey @ndimiduk
HBASE-24163 MOB compactor implementations should use format specifier…
1303562 
…s when calling String.format

Signed-off-by: stack <stack@apache.org>
Signed-off-by: Viraj Jasani <vjasani@apache.org>
Signed-off-by: Josh Elser <elserj@apache.org>
@ndimiduk ndimiduk added the backport This PR is a back port of some issue or issues already committed to master label Aug 15, 2022
@ndimiduk ndimiduk requested a review from apurtell August 15, 2022 09:41
@ndimiduk
Copy link
Member Author

FYI @BukrosSzabolcs

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 3s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ branch-2 Compile Tests _
+1 💚 mvninstall 2m 19s branch-2 passed
+1 💚 compile 2m 15s branch-2 passed
+1 💚 checkstyle 0m 33s branch-2 passed
+1 💚 spotless 0m 41s branch has no errors when running spotless:check.
+1 💚 spotbugs 1m 18s branch-2 passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 10s the patch passed
+1 💚 compile 2m 14s the patch passed
-0 ⚠️ javac 2m 14s hbase-server generated 1 new + 192 unchanged - 1 fixed = 193 total (was 193)
+1 💚 checkstyle 0m 31s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 11m 48s Patch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚 spotless 0m 39s patch has no errors when running spotless:check.
+1 💚 spotbugs 1m 23s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 11s The patch does not generate ASF License warnings.
32m 22s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #4706
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname Linux 8ce6af393516 5.4.0-122-generic #138-Ubuntu SMP Wed Jun 22 15:00:31 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision branch-2 / b83165c
Default Java AdoptOpenJDK-1.8.0_282-b08
javac https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-server.txt
Max. process+thread count 65 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/console
versions git=2.17.1 maven=3.6.3 spotbugs=4.2.2
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Copy link
Contributor

Apache9 commented Aug 15, 2022

This is for aligning the code?

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 54s Docker mode activated.
-0 ⚠️ yetus 0m 5s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+1 💚 mvninstall 2m 22s branch-2 passed
+1 💚 compile 0m 34s branch-2 passed
+1 💚 shadedjars 3m 53s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 23s branch-2 passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 4s the patch passed
+1 💚 compile 0m 34s the patch passed
+1 💚 javac 0m 34s the patch passed
+1 💚 shadedjars 3m 54s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 20s the patch passed
_ Other Tests _
+1 💚 unit 185m 39s hbase-server in the patch passed.
202m 14s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR #4706
Optional Tests javac javadoc unit shadedjars compile
uname Linux 7463f699fa97 5.4.0-1071-aws #76~18.04.1-Ubuntu SMP Mon Mar 28 17:49:57 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision branch-2 / b83165c
Default Java AdoptOpenJDK-1.8.0_282-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/testReport/
Max. process+thread count 2364 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 54s Docker mode activated.
-0 ⚠️ yetus 0m 4s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+1 💚 mvninstall 2m 47s branch-2 passed
+1 💚 compile 0m 42s branch-2 passed
+1 💚 shadedjars 4m 6s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 25s branch-2 passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 27s the patch passed
+1 💚 compile 0m 42s the patch passed
+1 💚 javac 0m 42s the patch passed
+1 💚 shadedjars 4m 6s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 23s the patch passed
_ Other Tests _
+1 💚 unit 184m 27s hbase-server in the patch passed.
203m 23s
Subsystem Report/Notes
Docker ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #4706
Optional Tests javac javadoc unit shadedjars compile
uname Linux 34b71f7a2224 5.4.0-1081-aws #88~18.04.1-Ubuntu SMP Thu Jun 23 16:29:17 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision branch-2 / b83165c
Default Java AdoptOpenJDK-11.0.10+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/testReport/
Max. process+thread count 2759 (vs. ulimit of 30000)
modules C: hbase-server U: hbase-server
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4706/1/console
versions git=2.17.1 maven=3.6.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@ndimiduk ndimiduk changed the title HBASE-24163 MOB compactor implementations should use format specifiers when calling String.format Backport "HBASE-24163 MOB compactor implementations should use format specifiers when calling String.format" to branch-2 Aug 15, 2022
@ndimiduk
Copy link
Member Author

This is for aligning the code?

@Apache9 Yes it's one of the missing patches that @BukrosSzabolcs listed on HBASE-27154.

@Apache9
Copy link
Contributor

Apache9 commented Aug 15, 2022

This is for aligning the code?

@Apache9 Yes it's one of the missing patches that @BukrosSzabolcs listed on HBASE-27154.

I mean the problem describe on HBASE-24163 does not exist on branch-2. But anyway, no harm to align the code .

@ndimiduk
Copy link
Member Author

I mean, maybe @BukrosSzabolcs or @busbey can confirm. I'd rather not backport changes that are not required.

saintstack
saintstack approved these changes Aug 15, 2022
View reviewed changes
Copy link
Contributor

@saintstack saintstack left a comment

Choose a reason for hiding this comment

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

LGTM

@Apache9
Copy link
Contributor

Apache9 commented Aug 16, 2022

I mean, maybe @BukrosSzabolcs or @busbey can confirm. I'd rather not backport changes that are not required.

The problem on master is we change to use slf4j style formatter but actually we do not use slf4j here, it is String.format...

On branch-2 we even do not use String.format, we just use '+' to concat the error message so no actually bug...

@ndimiduk
Copy link
Member Author

Yep, I see that. I wonder what patch brought in the bug that this one fixed on master, it apparently hasn't been back ported to branch-2 yet.

Anyway, I'll continue this backport so that the active release lines remain fairly similar.

@ndimiduk ndimiduk merged commit 1986a6e into apache:branch-2 Aug 17, 2022
@ndimiduk ndimiduk deleted the 24163-mob-compactor-string.format-branch-2 branch August 17, 2022 11:47
@busbey
Copy link
Contributor

busbey commented Aug 18, 2022

One of the distributed mob compaction changes brought in this bug on master

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

@saintstack saintstack saintstack approved these changes

@Apache9 Apache9 Apache9 approved these changes

@apurtell apurtell Awaiting requested review from apurtell

@huaxiangsun huaxiangsun Awaiting requested review from huaxiangsun

Assignees
No one assigned
Labels
backport This PR is a back port of some issue or issues already committed to master
Projects
None yet
Milestone
No milestone
5 participants
@ndimiduk @Apache-HBase @Apache9 @busbey @saintstack