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 d3f8e36010a5..bcb295a2628b 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 @@ private RegionLocations locateRegionInMeta(TableName tableName, byte[] row, bool } // 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 @@ rpcControllerFactory, getMetaLookupPool(), connectionConfig.getMetaReadRpcTimeou 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/TestAdmin.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java index 6a28831d087a..5feca4549ce3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java @@ -39,8 +39,8 @@ import org.apache.hadoop.hbase.TableExistsException; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; -import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.hadoop.hbase.master.HMaster; +import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.hadoop.hbase.master.assignment.AssignmentManager; import org.apache.hadoop.hbase.master.assignment.RegionStateNode; import org.apache.hadoop.hbase.regionserver.storefiletracker.StoreFileTrackerFactory; 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 e79f0dc8e5d4..778f6c4c57d4 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 void testUserRegionLockThrowsException() throws IOException, InterruptedE // 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);