HBASE-23561 Look up of Region in Master by encoded region name is O(n)

[mwkang]

This is my first PR using github.
If something is wrong. please let me know.

[Apache-HBase]

💔 -1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	2m 4s	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.
-0 ⚠️	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
		_ master Compile Tests _
+1 💚	mvninstall	5m 37s	master passed
+1 💚	compile	0m 57s	master passed
+1 💚	checkstyle	1m 7s	master passed
+1 💚	shadedjars	4m 40s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 37s	master passed
+0 🆗	spotbugs	4m 15s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 14s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	4m 59s	the patch passed
+1 💚	compile	0m 59s	the patch passed
+1 💚	javac	0m 59s	the patch passed
+1 💚	checkstyle	1m 8s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedjars	4m 42s	patch has no errors when building our shaded downstream artifacts.
+1 💚	hadoopcheck	16m 10s	Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2.
+1 💚	javadoc	0m 38s	the patch passed
+1 💚	findbugs	4m 14s	the patch passed
		_ Other Tests _
-1 ❌	unit	65m 8s	hbase-server in the patch failed.
+1 💚	asflicense	0m 35s	The patch does not generate ASF License warnings.
		124m 29s

Subsystem	Report/Notes
Docker	Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/1/artifact/out/Dockerfile
GITHUB PR	#1193
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux 4718f2be10ea 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/Base-PreCommit-GitHub-PR_PR-1193/out/precommit/personality/provided.sh
git revision	master / fe0d49b
Default Java	1.8.0_181
unit	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/1/artifact/out/patch-unit-hbase-server.txt
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/1/testReport/
Max. process+thread count	9750 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/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.

[wchevreuil]

LGTM.

The test failure reported is not an actual UT failure, somehow the jenkins process didn't terminate properly. Triggered a rebuild, just in case, planning to merge this later today if we get a green.

[Apache-HBase]

🎊 +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.
-0 ⚠️	test4tests	0m 0s	The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
		_ master Compile Tests _
+1 💚	mvninstall	5m 35s	master passed
+1 💚	compile	0m 57s	master passed
+1 💚	checkstyle	1m 9s	master passed
+1 💚	shadedjars	4m 41s	branch has no errors when building our shaded downstream artifacts.
+1 💚	javadoc	0m 39s	master passed
+0 🆗	spotbugs	4m 18s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 15s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	4m 58s	the patch passed
+1 💚	compile	0m 57s	the patch passed
+1 💚	javac	0m 57s	the patch passed
+1 💚	checkstyle	1m 4s	the patch passed
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	shadedjars	4m 43s	patch has no errors when building our shaded downstream artifacts.
+1 💚	hadoopcheck	16m 2s	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 26s	the patch passed
		_ Other Tests _
+1 💚	unit	64m 58s	hbase-server in the patch passed.
+1 💚	asflicense	0m 38s	The patch does not generate ASF License warnings.
		122m 46s

Subsystem	Report/Notes
Docker	Client=19.03.6 Server=19.03.6 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/2/artifact/out/Dockerfile
GITHUB PR	#1193
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
uname	Linux 15416d7d2cb0 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/Base-PreCommit-GitHub-PR_PR-1193/out/precommit/personality/provided.sh
git revision	master / fe0d49b
Default Java	1.8.0_181
Test Results	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/2/testReport/
Max. process+thread count	9839 (vs. ulimit of 10000)
modules	C: hbase-server U: hbase-server
Console output	https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1193/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.

[virajjasani]

+1

[wchevreuil]

Got a green, let me merge this PR. Thanks for the contribution, @mwkang !

[saintstack]

Thank you for working on this @mwkang It looks good. Only question is if the methods where we modify the two maps are sure to only have a single thread accessing them at at a time. I think they should be good because synchronize or locks are taken at a higher level by accessors IIRC. Might be worth a check.

[mwkang]

@saintstack I think it looks okay. However, I am not sure only a single thread accessing them. And as you have already mentioned, If I want to synchronize two maps, I should use synchronize or locks. It was my mistake. I didn't think about it then. Should do I use synchronized or locks?

[mwkang]

I think the current code may have different values for the two maps.
What do you think about changing it like below?

// ...

private final Object regionsMapLock = new Object();

// ...

RegionStateNode createRegionStateNode(RegionInfo regionInfo) {
  synchronized (regionsMapLock) {
    RegionStateNode node = regionsMap.computeIfAbsent(regionInfo.getRegionName(), key -> new RegionStateNod(regionInfo, regionInTransition));
    encodedRegionsMap.putIfAbsent(node.getRegionInfo().getEncodedName(), node);
    return node;
  }
}

Also there is a remove logic, it seems safe to use a synchronized.

public void deleteRegion(final RegionInfo regionInfo) {
  synchronized (regionsMapLock) {
    RegionStateNode removeNode = regionsMap.remove(regionInfo.getRegionName());
    encodedRegionsMap.remove(regionInfo.getEncodedName(), removeNode);
  }

  // ...
}