[phoenix] branch master updated: PHOENIX-7003 Harden hbase region inconsistencies check in CQSI#getAllTableRegions method (#1686)

[vjasani]

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

vjasani pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new 6125f0244c PHOENIX-7003 Harden hbase region inconsistencies check in 
CQSI#getAllTableRegions method (#1686)
6125f0244c is described below

commit 6125f0244ce69b86235c19b39c6abaf3095f7eab
Author: Divneet18 <dik...@ucsd.edu>
AuthorDate: Wed Sep 27 11:35:07 2023 -0700

    PHOENIX-7003 Harden hbase region inconsistencies check in 
CQSI#getAllTableRegions method (#1686)
---
 .../phoenix/query/ConnectionQueryServicesImpl.java |  4 +++-
 .../query/ConnectionQueryServicesImplTest.java     | 26 ++++++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 3e7c5ed098..551be97e1c 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -699,7 +699,9 @@ public class ConnectionQueryServicesImpl extends 
DelegateQueryServices implement
     public byte[] getNextRegionStartKey(HRegionLocation regionLocation, byte[] 
currentKey) throws IOException {
         // in order to check the overlap/inconsistencies bad region info, we 
have to make sure
         // the current endKey always increasing(compare the previous endKey)
-        if (Bytes.compareTo(regionLocation.getRegion().getEndKey(), 
currentKey) <= 0
+        // note :- currentKey is the previous regions endKey
+        if ((Bytes.compareTo(regionLocation.getRegion().getStartKey(), 
currentKey) != 0
+                || Bytes.compareTo(regionLocation.getRegion().getEndKey(), 
currentKey) <= 0)
                 && !Bytes.equals(currentKey, HConstants.EMPTY_START_ROW)
                 && !Bytes.equals(regionLocation.getRegion().getEndKey(), 
HConstants.EMPTY_END_ROW)) {
             GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.increment();
diff --git 
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
 
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
index 5d60423adb..7726fe7026 100644
--- 
a/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
+++ 
b/phoenix-core/src/test/java/org/apache/phoenix/query/ConnectionQueryServicesImplTest.java
@@ -168,6 +168,7 @@ public class ConnectionQueryServicesImplTest {
                 org.mockito.Mockito.CALLS_REAL_METHODS);
         byte[] corruptedStartAndEndKey = "0x3000".getBytes();
         byte[] corruptedDecreasingKey = "0x2999".getBytes();
+        byte[] corruptedNewEndKey = "0x3001".getBytes();
         byte[] notCorruptedStartKey = "0x2999".getBytes();
         byte[] notCorruptedEndKey = "0x3000".getBytes();
         byte[] notCorruptedNewKey = "0x3001".getBytes();
@@ -190,9 +191,30 @@ public class ConnectionQueryServicesImplTest {
         testGetNextRegionStartKey(mockCqsi, mockRegionLocation, 
corruptedStartAndEndKey, true);
 
         // comparing the current regionInfo endKey is greater than the 
previous endKey
-        // [0x3000,0x3000) vs 0x3001
+        // [0x2999,0x3001) vs 0x3000
         
GlobalClientMetrics.GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.getMetric().reset();
-        when(mockHRegionInfo.getStartKey()).thenReturn(notCorruptedStartKey);
+        when(mockHRegionInfo.getStartKey()).thenReturn(corruptedDecreasingKey);
+        when(mockHRegionInfo.getEndKey()).thenReturn(corruptedNewEndKey);
+        testGetNextRegionStartKey(mockCqsi, mockRegionLocation, 
corruptedStartAndEndKey, true);
+
+        // comparing the current regionInfo startKey is greater than the 
previous endKey leading to a hole
+        // [0x3000,0x3001) vs 0x2999
+        
GlobalClientMetrics.GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.getMetric().reset();
+        
when(mockHRegionInfo.getStartKey()).thenReturn(corruptedStartAndEndKey);
+        when(mockHRegionInfo.getEndKey()).thenReturn(corruptedNewEndKey);
+        testGetNextRegionStartKey(mockCqsi, mockRegionLocation, 
corruptedDecreasingKey, true);
+
+        // comparing the current regionInfo startKey is less than the previous 
endKey leading to an overlap
+        // [0x2999,0x3001) vs 0x3000
+        
GlobalClientMetrics.GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.getMetric().reset();
+        when(mockHRegionInfo.getStartKey()).thenReturn(corruptedDecreasingKey);
+        when(mockHRegionInfo.getEndKey()).thenReturn(corruptedNewEndKey);
+        testGetNextRegionStartKey(mockCqsi, mockRegionLocation, 
corruptedStartAndEndKey, true);
+
+        // comparing the current regionInfo startKey is equal to the previous 
endKey
+        // [0x3000,0x3001) vs 0x3000
+        
GlobalClientMetrics.GLOBAL_HBASE_COUNTER_METADATA_INCONSISTENCY.getMetric().reset();
+        
when(mockHRegionInfo.getStartKey()).thenReturn(corruptedStartAndEndKey);
         when(mockHRegionInfo.getEndKey()).thenReturn(notCorruptedNewKey);
         testGetNextRegionStartKey(mockCqsi, mockRegionLocation, 
notCorruptedEndKey, false);