HBASE-23275: Track active master's address in ActiveMasterManager by bharathv · Pull Request #812 · apache/hbase

bharathv · 2019-11-10T00:12:11Z

Currently we just track whether an active master exists.
It helps to also track the address of the active master in
all the masters to help serve the client RPC requests to
know which master is active.

bharathv · 2019-11-10T00:14:37Z

@wchevreuil @apurtell This is to track the active master server name in all the masters. With this change (and the other pull request in flight), I think clients can go to any master to fetch all the information they need.

Apache-HBase · 2019-11-10T03:54:30Z

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 34s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	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.
+1 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ master Compile Tests _
+1 💚	mvninstall	5m 24s	master passed
+1 💚	compile	0m 55s	master passed
+1 💚	checkstyle	1m 20s	master passed
+1 💚	shadedjars	4m 34s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 37s	master passed
+0 🆗	spotbugs	4m 8s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 5s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	5m 6s	the patch passed
+1 💚	compile	0m 57s	the patch passed
+1 💚	javac	0m 57s	the patch passed
-1 ❌	checkstyle	1m 18s	hbase-server: The patch generated 1 new + 113 unchanged - 0 fixed = 114 total (was 113)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedjars	4m 36s	patch has no errors when building our shaded downstream artifacts.
+1 💚	hadoopcheck	15m 42s	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1 💚	javadoc	0m 36s	the patch passed
+1 💚	findbugs	4m 10s	the patch passed
		_ Other Tests _
+1 💚	unit	160m 38s	hbase-server in the patch passed.
+1 💚	asflicense	0m 34s	The patch does not generate ASF License warnings.
		217m 34s

Subsystem	Report/Notes
Docker	Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/1/artifact/out/Dockerfile
GITHUB PR	#812
JIRA Issue	HBASE-23275
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux a7800f4d7041 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-812/out/precommit/personality/provided.sh
git revision	master / `08aae42`
Default Java	1.8.0_181
checkstyle	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/1/artifact/out/diff-checkstyle-hbase-server.txt
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/1/testReport/
Max. process+thread count	5183 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/1/console
versions	git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

virajjasani

Looks good mostly

Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active.

bharathv · 2019-11-11T23:21:15Z

Fixed a bunch of check style nits and typos. No functional changes in the new version.

ndimiduk · 2019-11-12T00:43:36Z

 }
 }

+ /*


Should be a javadoc comment.

ndimiduk · 2019-11-12T00:45:37Z


+ // Active master's server name. Invalidated anytime active master changes (based on ZK
+ // notifications) and lazily fetched on-demand.
+ // ServerName is immutable, so we don't need heavy synchronization around it.


Actually, I think this field can replace the AtomicBoolean clusterHasActiveMaster. Seems your new information is a super-set of the existing information.

I didn't do this because I implemented it as a lazy-fetch (don't fetch it unless some client asks for it). This way we also don't touch the battle-tested synchronization around 'clusterHasActiveMaster' and hence lesser risk of introducing any new bugs. Let me know if you feel strongly, I'll refactor the whole thing.

No, this is a fair point. It would be nice to consolidate these concerns, but if we don't have the test coverage and you don't have the capacity for a manual test, better leave it as is.

Apache-HBase · 2019-11-12T03:23:14Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 34s	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.
+1 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ master Compile Tests _
+1 💚	mvninstall	5m 46s	master passed
+1 💚	compile	1m 1s	master passed
+1 💚	checkstyle	1m 25s	master passed
+1 💚	shadedjars	4m 58s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 37s	master passed
+0 🆗	spotbugs	4m 26s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 25s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	5m 11s	the patch passed
+1 💚	compile	0m 57s	the patch passed
+1 💚	javac	0m 57s	the patch passed
+1 💚	checkstyle	1m 20s	hbase-server: The patch generated 0 new + 107 unchanged - 6 fixed = 107 total (was 113)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedjars	4m 44s	patch has no errors when building our shaded downstream artifacts.
+1 💚	hadoopcheck	16m 29s	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1 💚	javadoc	0m 37s	the patch passed
+1 💚	findbugs	4m 25s	the patch passed
		_ Other Tests _
+1 💚	unit	173m 15s	hbase-server in the patch passed.
+1 💚	asflicense	0m 33s	The patch does not generate ASF License warnings.
		233m 0s

Subsystem	Report/Notes
Docker	Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/2/artifact/out/Dockerfile
GITHUB PR	#812
JIRA Issue	HBASE-23275
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux f88e32a40c1b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-812/out/precommit/personality/provided.sh
git revision	master / `9494c12`
Default Java	1.8.0_181
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/2/testReport/
Max. process+thread count	4767 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/2/console
versions	git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

- Use volatile instead of AtomicReference - Switch to Optional for better Nullability semantics.

Apache-HBase · 2019-11-12T20:49:14Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 33s	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.
+1 💚	test4tests	0m 0s	The patch appears to include 1 new or modified test files.
		_ master Compile Tests _
+1 💚	mvninstall	5m 21s	master passed
+1 💚	compile	0m 56s	master passed
+1 💚	checkstyle	1m 20s	master passed
+1 💚	shadedjars	4m 41s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 38s	master passed
+0 🆗	spotbugs	4m 10s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 8s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	4m 57s	the patch passed
+1 💚	compile	0m 57s	the patch passed
+1 💚	javac	0m 57s	the patch passed
+1 💚	checkstyle	1m 19s	hbase-server: The patch generated 0 new + 107 unchanged - 6 fixed = 107 total (was 113)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedjars	4m 38s	patch has no errors when building our shaded downstream artifacts.
+1 💚	hadoopcheck	15m 44s	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1 💚	javadoc	0m 36s	the patch passed
+1 💚	findbugs	4m 16s	the patch passed
		_ Other Tests _
+1 💚	unit	157m 14s	hbase-server in the patch passed.
+1 💚	asflicense	0m 32s	The patch does not generate ASF License warnings.
		213m 57s

Subsystem	Report/Notes
Docker	Client=19.03.4 Server=19.03.4 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/3/artifact/out/Dockerfile
GITHUB PR	#812
JIRA Issue	HBASE-23275
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux d05f442681c6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux
Build tool	maven
Personality	/home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-812/out/precommit/personality/provided.sh
git revision	master / `9494c12`
Default Java	1.8.0_181
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/3/testReport/
Max. process+thread count	4885 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-812/3/console
versions	git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11
Powered by	Apache Yetus 0.11.1 https://yetus.apache.org

This message was automatically generated.

bharathv

@ndimiduk Just checking to see if you had a chance to take a look at my last patch set. Thanks

apurtell

lgtm, fwiw

ndimiduk

👍

ndimiduk · 2019-11-18T21:52:57Z


+ // Active master's server name. Invalidated anytime active master changes (based on ZK
+ // notifications) and lazily fetched on-demand.
+ // ServerName is immutable, so we don't need heavy synchronization around it.


No, this is a fair point. It would be nice to consolidate these concerns, but if we don't have the test coverage and you don't have the capacity for a manual test, better leave it as is.

* HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

…ache#812) * HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

* HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

…ache#812) Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit efebb84)

…1098) * HBASE-23257: Track clusterID in stand by masters (#798) This patch implements a simple cache that all the masters can lookup to serve cluster ID to clients. Active HMaster is still responsible for creating it but all the masters will read it from fs to serve clients. RPCs exposing it will come in a separate patch as a part of HBASE-18095. Signed-off-by: Andrew Purtell <apurtell@apache.org> Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: Guangxu Cheng <guangxucheng@gmail.com> (cherry picked from commit c2e01f2) * HBASE-23275: Track active master's address in ActiveMasterManager (#812) Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit efebb84) * HBASE-23281: Track meta region locations in masters (#830) * HBASE-23281: Track meta region changes on masters This patch adds a simple cache that tracks the meta region replica locations. It keeps an eye on the region movements so that the cached locations are not stale. This information is used for servicing client RPCs for connections that use master based registry (HBASE-18095). The RPC end points will be added in a separate patch. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> (cherry picked from commit 8571d38) * HBASE-23304: RPCs needed for client meta information lookup (#904) * HBASE-23304: RPCs needed for client meta information lookup This patch implements the RPCs needed for the meta information lookup during connection init. New tests added to cover the RPC code paths. HBASE-23305 builds on this to implement the client side logic. Fixed a bunch of checkstyle nits around the places the patch touches. Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit 4f8fbba)

* HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

…ache#812) * HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

* HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

…ache#812) * HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

* HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

…ache#812) Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit efebb84)

Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit efebb84)

…ache#812) * HBASE-23275: Track active master's address in ActiveMasterManager Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org>

…ache#812) Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit efebb84) (cherry picked from commit 7429491)

Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active. Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Andrew Purtell <apurtell@apache.org> (cherry picked from commit efebb84) (cherry picked from commit 7429491)

virajjasani reviewed Nov 11, 2019
View reviewed changes

Comment threadhbase-server/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java Outdated

virajjasani reviewed Nov 11, 2019
View reviewed changes

Comment threadhbase-server/src/main/java/org/apache/hadoop/hbase/master/ActiveMasterManager.java

virajjasani reviewed Nov 11, 2019
View reviewed changes

HBASE-23275: Track active master's address in ActiveMasterManager
865fe77
Currently we just track whether an active master exists. It helps to also track the address of the active master in all the masters to help serve the client RPC requests to know which master is active.

bharathv force-pushed the HBASE-23275 branch from f61b1e3 to 865fe77Compare November 11, 2019 23:20

ndimiduk reviewed Nov 12, 2019
View reviewed changes

Address Nick's comments.
84a82c5
- Use volatile instead of AtomicReference - Switch to Optional for better Nullability semantics.

bharathv mentioned this pull request Nov 14, 2019
HBASE-18095: Zookeeper-less client connection implementation #781
Closed

bharathv commented Nov 14, 2019
View reviewed changes

apurtell approved these changes Nov 16, 2019
View reviewed changes

ndimiduk approved these changes Nov 18, 2019
View reviewed changes

ndimiduk changed the base branch from master to HBASE-18095/client-locate-meta-no-zookeeperNovember 20, 2019 19:37

ndimiduk merged commit cc611c9 into apache:HBASE-18095/client-locate-meta-no-zookeeperNov 20, 2019

bharathv deleted the HBASE-23275 branch November 20, 2019 21:06

bharathv mentioned this pull request Jan 2, 2020
HBASE-23304: RPCs needed for client meta information lookup #904
Merged

  }
  }
+ /*

Uh oh!

Conversation

bharathv commented Nov 10, 2019

Uh oh!

bharathv commented Nov 10, 2019

Uh oh!

Apache-HBase commented Nov 10, 2019

Uh oh!

Uh oh!

Uh oh!

virajjasani left a comment

Choose a reason for hiding this comment

Uh oh!

bharathv commented Nov 11, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ndimidukNov 12, 2019

Choose a reason for hiding this comment

Uh oh!

bharathvNov 12, 2019

Choose a reason for hiding this comment

Uh oh!

ndimidukNov 12, 2019

Choose a reason for hiding this comment

Uh oh!

bharathvNov 12, 2019

Choose a reason for hiding this comment

Uh oh!

ndimidukNov 18, 2019

Choose a reason for hiding this comment

Uh oh!

Apache-HBase commented Nov 12, 2019

Uh oh!

Apache-HBase commented Nov 12, 2019

Uh oh!

bharathv left a comment

Choose a reason for hiding this comment

Uh oh!

apurtell left a comment

Choose a reason for hiding this comment

Uh oh!

ndimiduk left a comment

Choose a reason for hiding this comment

Uh oh!

ndimidukNov 18, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants