[hbase] branch branch-2 updated: HBASE-27897 ConnectionImplementation#locateRegionInMeta should pause and retry when taking user region lock failed (#5258)

[ha...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

haxiaolin pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new b6aef4499fb HBASE-27897 ConnectionImplementation#locateRegionInMeta should pause and retry when taking user region lock failed (#5258)
b6aef4499fb is described below

commit b6aef4499fb3829a053c54156b697bd4cbe04c71
Author: Xiaolin Ha <ha...@apache.org>
AuthorDate: Wed Jun 7 16:31:39 2023 +0800

    HBASE-27897 ConnectionImplementation#locateRegionInMeta should pause and retry when taking user region lock failed (#5258)
    
    Signed-off-by: Wellington Chevreuil <wc...@apache.org>
---
 .../hadoop/hbase/client/ConnectionImplementation.java   | 17 +++++++++++------
 .../org/apache/hadoop/hbase/client/TestMetaCache.java   | 14 +++++++-------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
index d3f8e36010a..bcb295a2628 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java
@@ -1008,9 +1008,12 @@ public class ConnectionImplementation implements ClusterConnection, Closeable {
       }
       // Query the meta region
       long pauseBase = connectionConfig.getPauseMillis();
-      takeUserRegionLock();
-      final long lockStartTime = EnvironmentEdgeManager.currentTime();
+      long lockStartTime = 0;
+      boolean lockedUserRegion = false;
       try {
+        takeUserRegionLock();
+        lockStartTime = EnvironmentEdgeManager.currentTime();
+        lockedUserRegion = true;
         // We don't need to check if useCache is enabled or not. Even if useCache is false
         // we already cleared the cache for this row before acquiring userRegion lock so if this
         // row is present in cache that means some other thread has populated it while we were
@@ -1119,10 +1122,12 @@ public class ConnectionImplementation implements ClusterConnection, Closeable {
             ConnectionUtils.getPauseTime(pauseBase, tries), TimeUnit.MILLISECONDS);
         }
       } finally {
-        userRegionLock.unlock();
-        // update duration of the lock being held
-        if (metrics != null) {
-          metrics.updateUserRegionLockHeld(EnvironmentEdgeManager.currentTime() - lockStartTime);
+        if (lockedUserRegion) {
+          userRegionLock.unlock();
+          // update duration of the lock being held
+          if (metrics != null) {
+            metrics.updateUserRegionLockHeld(EnvironmentEdgeManager.currentTime() - lockStartTime);
+          }
         }
       }
       try {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java
index e79f0dc8e5d..778f6c4c57d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaCache.java
@@ -592,21 +592,21 @@ public class TestMetaCache {
       // obtain the client metrics
       MetricsConnection metrics = conn.getConnectionMetrics();
       long queueCount = metrics.getUserRegionLockQueue().getCount();
-      assertEquals("Queue of userRegionLock should be updated twice. queueCount: " + queueCount,
-        queueCount, 2);
+      assertEquals("Queue of userRegionLock should be updated twice. queueCount: " + queueCount, 2,
+        queueCount);
 
       long timeoutCount = metrics.getUserRegionLockTimeout().getCount();
-      assertEquals("Timeout of userRegionLock should happen once. timeoutCount: " + timeoutCount,
-        timeoutCount, 1);
+      assertEquals("Timeout of userRegionLock should happen once. timeoutCount: " + timeoutCount, 1,
+        timeoutCount);
 
       long waitingTimerCount = metrics.getUserRegionLockWaitingTimer().getCount();
       assertEquals("userRegionLock should be grabbed successfully once. waitingTimerCount: "
-        + waitingTimerCount, waitingTimerCount, 1);
+        + waitingTimerCount, 1, waitingTimerCount);
 
       long heldTimerCount = metrics.getUserRegionLockHeldTimer().getCount();
       assertEquals(
-        "userRegionLock should be held successfully once. heldTimerCount: " + heldTimerCount,
-        heldTimerCount, 1);
+        "userRegionLock should be held successfully once. heldTimerCount: " + heldTimerCount, 1,
+        heldTimerCount);
       double heldTime = metrics.getUserRegionLockHeldTimer().getSnapshot().getMax();
       assertTrue("Max held time should be greater than 2 seconds. heldTime: " + heldTime,
         heldTime >= 2E9);