Skip to content

HBASE-26616 Refactor code related to ZooKeeper authentication - #3973

Merged
ndimiduk merged 2 commits into
apache:masterfrom
ndimiduk:26616-refactor-zk-authentication
Jan 7, 2022
Merged

HBASE-26616 Refactor code related to ZooKeeper authentication#3973
ndimiduk merged 2 commits into
apache:masterfrom
ndimiduk:26616-refactor-zk-authentication

Conversation

@ndimiduk

Copy link
Copy Markdown
Member

This refactor reduces the size and scope of the ZKUtil class. The core of this refactor is
moving the login* methods from ZKUtil into their own class, ZKAuthentication. The class
JaasConfiguration is also moved along with them.

@Apache-HBase

This comment has been minimized.

@Apache9Apache9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So is this related to how we want to implement tracing?

@ndimiduk

Copy link
Copy Markdown
MemberAuthor

So is this related to how we want to implement tracing?

No sir. I have taken some time aside this week to learn a tool called Structure101, hoping that I can use it to find parts of our code that are unnecessarily complex, and untangle them. These two ZooKeeper -related refactors (and others, presuming i spend more time on it tomorrow) are not related to tracing, but to general improvement of the health and well-being of our source code.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
@ndimiduk
ndimidukforce-pushed the 26616-refactor-zk-authentication branch from af857d1 to ea97e1eCompareJanuary 5, 2022 19:11
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 28sDocker 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 16sMaven dependency ordering for branch
+1 💚mvninstall4m 19smaster passed
+1 💚compile3m 55smaster passed
+1 💚checkstyle1m 34smaster passed
+1 💚spotbugs2m 53smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 15sMaven dependency ordering for patch
+1 💚mvninstall3m 50sthe patch passed
+1 💚compile3m 43sthe patch passed
+1 💚javac0m 30shbase-zookeeper generated 0 new + 93 unchanged - 1 fixed = 93 total (was 94)
-0 ⚠️javac3m 13shbase-server generated 4 new + 189 unchanged - 4 fixed = 193 total (was 193)
+1 💚checkstyle0m 14shbase-zookeeper: The patch generated 0 new + 1 unchanged - 1 fixed = 1 total (was 2)
+1 💚checkstyle1m 3sThe patch passed checkstyle in hbase-server
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚hadoopcheck19m 32sPatch does not cause any errors with Hadoop 3.1.2 3.2.2 3.3.1.
+1 💚spotbugs3m 0sthe patch passed
_ Other Tests _
+1 💚asflicense0m 28sThe patch does not generate ASF License warnings.
54m 0s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#3973
Optional Testsdupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile
unameLinux f60c19882f86 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / baeb51f
Default JavaAdoptOpenJDK-1.8.0_282-b08
javachttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/artifact/yetus-general-check/output/diff-compile-javac-hbase-server.txt
Max. process+thread count96 (vs. ulimit of 30000)
modulesC: hbase-zookeeper hbase-server U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/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

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 15sMaven dependency ordering for branch
+1 💚mvninstall3m 51smaster passed
+1 💚compile1m 22smaster passed
+1 💚shadedjars8m 21sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 55smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 18sMaven dependency ordering for patch
+1 💚mvninstall3m 52sthe patch passed
+1 💚compile1m 26sthe patch passed
+1 💚javac1m 26sthe patch passed
+1 💚shadedjars8m 17spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 55sthe patch passed
_ Other Tests _
+1 💚unit0m 45shbase-zookeeper in the patch passed.
+1 💚unit151m 21shbase-server in the patch passed.
184m 22s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile
GITHUB PR#3973
Optional Testsjavac javadoc unit shadedjars compile
unameLinux b8ff3cc62ed0 4.15.0-161-generic #169-Ubuntu SMP Fri Oct 15 13:41:54 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / baeb51f
Default JavaAdoptOpenJDK-1.8.0_282-b08
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/testReport/
Max. process+thread count3892 (vs. ulimit of 30000)
modulesC: hbase-zookeeper hbase-server U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/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

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec1m 2sDocker 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 13sMaven dependency ordering for branch
+1 💚mvninstall5m 4smaster passed
+1 💚compile1m 42smaster passed
+1 💚shadedjars9m 9sbranch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 59smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 15sMaven dependency ordering for patch
+1 💚mvninstall5m 11sthe patch passed
+1 💚compile1m 43sthe patch passed
+1 💚javac1m 43sthe patch passed
+1 💚shadedjars9m 12spatch has no errors when building our shaded downstream artifacts.
+1 💚javadoc0m 59sthe patch passed
_ Other Tests _
+1 💚unit0m 46shbase-zookeeper in the patch passed.
-1 ❌unit206m 11shbase-server in the patch failed.
244m 22s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#3973
Optional Testsjavac javadoc unit shadedjars compile
unameLinux 702225de31e8 4.15.0-162-generic #170-Ubuntu SMP Mon Oct 18 11:38:05 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionmaster / baeb51f
Default JavaAdoptOpenJDK-11.0.10+9
unithttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt
Test Resultshttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/testReport/
Max. process+thread count2559 (vs. ulimit of 30000)
modulesC: hbase-zookeeper hbase-server U: .
Console outputhttps://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3973/3/console
versionsgit=2.17.1 maven=3.6.3
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@ndimiduk

Copy link
Copy Markdown
MemberAuthor

ErrorProne parsing is still broken -- those javac failures are in files I did not touch.

TestDrainReplicationQueuesForStandBy is flaky for me on master. It's failing at a rate of roughly 2 in 10 as of ce2bbaf30a.

@ndimiduk
ndimiduk merged commit 5e263da into apache:masterJan 7, 2022
@ndimiduk
ndimiduk deleted the 26616-refactor-zk-authentication branch January 7, 2022 01:04
@Apache9

Copy link
Copy Markdown
Contributor

The error prone output is not very stable, so typically warnings should be fine, but if it fails the compilation then we still need to take care of it.

ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Jan 7, 2022
…#3973)
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
ndimiduk added a commit that referenced this pull request Jan 10, 2022
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Jan 10, 2022
…#3973)
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
ndimiduk added a commit to ndimiduk/hbase that referenced this pull request Jan 10, 2022
…#3973)
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
ndimiduk added a commit that referenced this pull request Jan 11, 2022
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
ndimiduk added a commit that referenced this pull request Jan 11, 2022
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
vinayakphegde pushed a commit to vinayakphegde/hbase that referenced this pull request Apr 4, 2024
…#3973)
This refactor reduces the size and scope of the `ZKUtil` class. The core of this refactor is
moving the `login*` methods from `ZKUtil` into their own class, `ZKAuthentication`. The class
`JaasConfiguration` is also moved along with them.
Signed-off-by: Andrew Purtell <apurtell@apache.org>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit 87e0554)
Change-Id: I611fc4a3bcbcd31049449eb352550e6c5364b86c
Sign up for freeto 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.

4 participants

@ndimiduk@Apache-HBase@Apache9@apurtell