Merge pull request #258 from oceanbase/fix_region_metric_slow

Fix region metric slow and region name does not match with region locator

Author: WeiXinChan

src/main/java/com/alipay/oceanbase/hbase/util/OHRegionLocatorExecutor.java

@@ -188,10 +188,13 @@ private OHRegionLocator createRangePartitionLocator(
                             i
                     );
                     int boundIndex = i / replicaDict.size();
+                    long tabletId = Integer.toUnsignedLong((Integer) partition.get(1));
                     final HRegionInfo regionInfo = new HRegionInfo(
                             TableName.valueOf(tableName),
                             startKeys[boundIndex],
-                            endKeys[boundIndex]
+                            endKeys[boundIndex],
+                            false,
+                            tabletId
                     );
                     HRegionLocation location = new HRegionLocation(regionInfo, serverName, i);
                     Boolean role = (int) partition.get(4) == 1;
@@ -230,10 +233,13 @@ private OHRegionLocator createHashPartitionLocator(
                             (int) hostInfo.get(1),
                             i
                     );
+                    long tabletId = Integer.toUnsignedLong((Integer) partition.get(1));
                     final HRegionInfo regionInfo = new HRegionInfo(
                             TableName.valueOf(tableName),
                             startKeys[0],
-                            endKeys[0]
+                            endKeys[0],
+                            false,
+                            tabletId
                     );
                     HRegionLocation location = new HRegionLocation(regionInfo, serverName, i);
                     Boolean role = (int) partition.get(4) == 1;

src/main/java/com/alipay/oceanbase/hbase/util/OHRegionMetricsExecutor.java

@@ -17,6 +17,7 @@
 
 package com.alipay.oceanbase.hbase.util;
 
+import com.alipay.oceanbase.rpc.exception.ObTableUnexpectedException;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -25,8 +26,8 @@
 import com.alipay.oceanbase.rpc.meta.ObTableMetaRequest;
 import com.alipay.oceanbase.rpc.meta.ObTableMetaResponse;
 import com.alipay.oceanbase.rpc.meta.ObTableRpcMetaType;
-import org.apache.hadoop.hbase.RegionMetrics;
-import org.apache.hadoop.hbase.Size;
+import org.apache.hadoop.hbase.*;
+import org.apache.hadoop.hbase.client.Table;
 
 import java.io.IOException;
 import java.util.*;
@@ -46,11 +47,10 @@ public ObTableRpcMetaType getMetaType() throws IOException {
     /*
     * {
         tableName: "tablegroup_name",
-        regionList:{
-                "regions": [200051, 200052, 200053, 200191, 200192, 200193, ...],
-                "memTableSize":[123, 321, 321, 123, 321, 321, ...],
-                "ssTableSize":[5122, 4111, 5661, 5122, 4111, 5661, ...]
-        }
+        "regions": [200051, 200052, 200053, 200191, 200192, 200193, ...],
+        "memTableSize":[123, 321, 321, 123, 321, 321, ...],
+        "ssTableSize":[5122, 4111, 5661, 5122, 4111, 5661, ...],
+        "boundary":["rowkey1", "rowkey2", "rowkey3", ..., "rowkey100", "rowkey101", "rowkey102", ...]
       }
     * */
     @Override
@@ -61,21 +61,56 @@ public List<RegionMetrics> parse(ObTableMetaResponse response) throws IOExceptio
         JsonNode tableGroupNameNode = Optional.<JsonNode>ofNullable(jsonMap.get("tableName"))
                 .orElseThrow(() -> new IOException("tableName is null"));
         String tableGroupName = tableGroupNameNode.asText();
-        JsonNode regionListNode = Optional.<JsonNode>ofNullable(jsonMap.get("regionList"))
-                .orElseThrow(() -> new IOException("regionList is null"));
-        List<Integer> regions = Optional.<List<Integer>>ofNullable(objectMapper.convertValue(regionListNode.get("regions"), new TypeReference<List<Integer>>() {}))
+        List<Integer> regions = Optional.<List<Integer>>ofNullable(objectMapper.convertValue(jsonMap.get("regions"), new TypeReference<List<Integer>>() {}))
                 .orElseThrow(() -> new IOException("regions is null"));
-        List<Integer> memTableSizeList = Optional.<List<Integer>>ofNullable(objectMapper.convertValue(regionListNode.get("memTableSize"), new TypeReference<List<Integer>>() {}))
+        List<Long> memTableSizeList = Optional.<List<Long>>ofNullable(objectMapper.convertValue(jsonMap.get("memTableSize"), new TypeReference<List<Long>>() {}))
                 .orElseThrow(() -> new IOException("memTableSize is null"));
-        List<Integer> ssTableSizeList = Optional.<List<Integer>>ofNullable(objectMapper.convertValue(regionListNode.get("ssTableSize"), new TypeReference<List<Integer>>() {}))
+        List<Long> ssTableSizeList = Optional.<List<Long>>ofNullable(objectMapper.convertValue(jsonMap.get("ssTableSize"), new TypeReference<List<Long>>() {}))
                 .orElseThrow(() -> new IOException("ssTableSize is null"));
+        List<String> boundaryList = Optional.<List<String>>ofNullable(objectMapper.convertValue(jsonMap.get("boundary"), new TypeReference<List<String>>() {}))
+                .orElseThrow(() -> new IOException("boundary is null"));
+        boolean isHashLikePartition = boundaryList.stream().allMatch(String::isEmpty);
+        boolean isRangeLikePartition = boundaryList.stream().noneMatch(String::isEmpty);
+        if (!isRangeLikePartition && !isHashLikePartition) {
+            // there are empty string and non-empty string in boundary, which is illegal for ADAPTIVE tablegroup
+            throw new ObTableUnexpectedException("tablegroup {" + tableGroupName + "} has tables with different partition types");
+        }
+        byte[][] startKeys = new byte[regions.size()][];
+        byte[][] endKeys = new byte[regions.size()][];
+        if (isHashLikePartition) {
+            startKeys[0] = HConstants.EMPTY_BYTE_ARRAY;
+            endKeys[0] = HConstants.EMPTY_BYTE_ARRAY;
+        } else {
+            final List<byte[]> startKeysList = new ArrayList<>();
+            final List<byte[]> endKeysList = new ArrayList<>();
+            for (int i = 0; i < boundaryList.size(); ++i) {
+                if (i == 0) {
+                    startKeysList.add(HConstants.EMPTY_BYTE_ARRAY);
+                    endKeysList.add(boundaryList.get(i).getBytes());
+                } else if (i == boundaryList.size() - 1) {
+                    startKeysList.add(boundaryList.get(i - 1).getBytes());
+                    endKeysList.add(HConstants.EMPTY_BYTE_ARRAY);
+                } else {
+                    startKeysList.add(boundaryList.get(i - 1).getBytes());
+                    endKeysList.add(boundaryList.get(i).getBytes());
+                }
+            }
+            startKeys = startKeysList.toArray(new byte[0][]);
+        }
         List<RegionMetrics> metricsList = new ArrayList<>();
-        if (regions.isEmpty() || regions.size() != memTableSizeList.size() || memTableSizeList.size() != ssTableSizeList.size()) {
+        if (regions.isEmpty() || regions.size() != memTableSizeList.size()
+                || memTableSizeList.size() != ssTableSizeList.size()
+                || ssTableSizeList.size() != startKeys.length) {
             throw new IOException("size length has to be the same");
         }
         for (int i = 0; i < regions.size(); ++i) {
-            String name_str = Integer.toString(regions.get(i));
-            byte[] name = name_str.getBytes();
+            byte[] startKey = isHashLikePartition ? startKeys[0] : startKeys[i];
+            byte[] name = HRegionInfo.createRegionName(
+                    TableName.valueOf(tableGroupName),
+                    startKey,
+                    regions.get(i),
+                    HRegionInfo.DEFAULT_REPLICA_ID, true
+            );
             Size storeFileSize = new Size(((double) ssTableSizeList.get(i)) / (1024 * 1024) , Size.Unit.MEGABYTE); // The unit in original HBase is MEGABYTE, for us it is BYTE
             Size memStoreSize = new Size(((double) memTableSizeList.get(i)) / (1024 * 1024), Size.Unit.MEGABYTE); // The unit in original HBase is MEGABYTE, for us it is BYTE
             OHRegionMetrics ohRegionMetrics = new OHRegionMetrics(tableGroupName, name, storeFileSize, memStoreSize);

src/test/java/com/alipay/oceanbase/hbase/OHTableAdminInterfaceTest.java

@@ -515,72 +515,72 @@ public void testAdminGetRegionMetrics() throws Exception {
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = test_get_region_metrics PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
+                    ") TABLEGROUP = test_get_region_metrics PARTITION BY KEY(`K`) PARTITIONS 10;\n" +
                     "\n" +
                     "CREATE TABLE IF NOT EXISTS `test_get_region_metrics$cf2` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = test_get_region_metrics PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
+                    ") TABLEGROUP = test_get_region_metrics PARTITION BY KEY(`K`) PARTITIONS 10;\n" +
                     "\n" +
                     "CREATE TABLE IF NOT EXISTS `test_get_region_metrics$cf3` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = test_get_region_metrics PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
+                    ") TABLEGROUP = test_get_region_metrics PARTITION BY KEY(`K`) PARTITIONS 10;\n" +
                     "\n" +
-                    "CREATE DATABASE IF NOT EXISTS `get_region`;\n" +
-                    "use `get_region`;\n" +
-                    "CREATE TABLEGROUP IF NOT EXISTS `get_region:test_multi_cf` SHARDING = 'ADAPTIVE';\n" +
-                    "CREATE TABLE IF NOT EXISTS `get_region:test_multi_cf$cf1` (\n" +
+                    "CREATE TABLEGROUP IF NOT EXISTS test_no_part SHARDING = 'ADAPTIVE';\n" +
+                    "CREATE TABLE IF NOT EXISTS `test_no_part$cf1` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
-                    "   PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = `get_region:test_multi_cf` PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
-                    "CREATE TABLE IF NOT EXISTS `get_region:test_multi_cf$cf2` (\n" +
+                    "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
+                    ") TABLEGROUP = test_no_part;\n" +
+                    "CREATE TABLE IF NOT EXISTS `test_no_part$cf2` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = `get_region:test_multi_cf` PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
-                    "CREATE TABLE IF NOT EXISTS `get_region:test_multi_cf$cf3` (\n" +
+                    ") TABLEGROUP = test_no_part;\n" +
+                    "CREATE TABLE IF NOT EXISTS `test_no_part$cf3` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = `get_region:test_multi_cf` PARTITION BY KEY(`K`) PARTITIONS 3;" +
-                    "USE `test`;" +
-                    "CREATE TABLEGROUP IF NOT EXISTS test_no_part SHARDING = 'ADAPTIVE';" +
-                    "CREATE TABLE IF NOT EXISTS `test_no_part$cf1` (\n" +
+                    ") TABLEGROUP = test_no_part;\n" +
+                    "CREATE DATABASE IF NOT EXISTS `get_region`;\n" +
+                    "use `get_region`;\n" +
+                    "CREATE TABLEGROUP IF NOT EXISTS `get_region:test_multi_cf` SHARDING = 'ADAPTIVE';\n" +
+                    "CREATE TABLE IF NOT EXISTS `get_region:test_multi_cf$cf1` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "   PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = `test_no_part`;\n" +
-                    "CREATE TABLE IF NOT EXISTS `test_no_part$cf2` (\n" +
+                    ") TABLEGROUP = `get_region:test_multi_cf` PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
+                    "CREATE TABLE IF NOT EXISTS `get_region:test_multi_cf$cf2` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = `test_no_part`;\n" +
-                    "CREATE TABLE IF NOT EXISTS `test_no_part$cf3` (\n" +
+                    ") TABLEGROUP = `get_region:test_multi_cf` PARTITION BY KEY(`K`) PARTITIONS 3;\n" +
+                    "CREATE TABLE IF NOT EXISTS `get_region:test_multi_cf$cf3` (\n" +
                     "    `K` varbinary(1024) NOT NULL,\n" +
                     "    `Q` varbinary(256) NOT NULL,\n" +
                     "    `T` bigint(20) NOT NULL,\n" +
                     "    `V` varbinary(1024) DEFAULT NULL,\n" +
                     "    PRIMARY KEY (`K`, `Q`, `T`)\n" +
-                    ") TABLEGROUP = `test_no_part`;");
+                    ") TABLEGROUP = `get_region:test_multi_cf` PARTITION BY KEY(`K`) PARTITIONS 3;");
             st.close();
+            conn.close();
             String tablegroup1 = "test_get_region_metrics";
             String tablegroup2 = "get_region:test_multi_cf";
             Configuration conf = ObHTableTestUtil.newConfiguration();
@@ -594,11 +594,11 @@ public void testAdminGetRegionMetrics() throws Exception {
             Assert.assertTrue(thrown.getCause() instanceof ObTableException);
             Assert.assertEquals(ResultCodes.OB_KV_HBASE_TABLE_NOT_EXISTS.errorCode, ((ObTableException) thrown.getCause()).getErrorCode());
 
-            // test use serverName without tableName to get region metrics
-            assertThrows(FeatureNotSupportedException.class,
-                    () -> {
-                        admin.getRegionMetrics(ServerName.valueOf("localhost,1,1"));
-                    });
+                // test use serverName without tableName to get region metrics
+                assertThrows(FeatureNotSupportedException.class,
+                        () -> {
+                            admin.getRegionMetrics(ServerName.valueOf("localhost,1,1"));
+                        });
 
             // test single-thread getRegionMetrics after writing
             batchInsert(10000, tablegroup1);
@@ -607,7 +607,7 @@ public void testAdminGetRegionMetrics() throws Exception {
             List<RegionMetrics> metrics = admin.getRegionMetrics(ServerName.valueOf("localhost,1,1"), TableName.valueOf(tablegroup1));
             long cost = System.currentTimeMillis() - start;
             System.out.println("get region metrics time usage: " + cost + "ms, tablegroup: " + tablegroup1);
-            assertEquals(9, metrics.size());
+            assertEquals(10, metrics.size());
 
             // test getRegionMetrics concurrently reading while writing
             ExecutorService executorService = Executors.newFixedThreadPool(10);
@@ -625,7 +625,7 @@ public void testAdminGetRegionMetrics() throws Exception {
                                 regionMetrics = admin.getRegionMetrics(null, TableName.valueOf(tablegroup1));
                                 long thrCost = System.currentTimeMillis() - thrStart;
                                 System.out.println("task: " + taskId + ", get region metrics time usage: " + thrCost + "ms, tablegroup: " + tablegroup1);
-                                if (regionMetrics.size() != 9) {
+                                if (regionMetrics.size() != 10) {
                                     throw new ObTableGetException(
                                             "the number of region metrics does not match the number of tablets, the number of region metrics: " + regionMetrics.size());
                                 }
@@ -634,7 +634,7 @@ public void testAdminGetRegionMetrics() throws Exception {
                                 regionMetrics = admin.getRegionMetrics(null, TableName.valueOf(tablegroup2));
                                 long thrCost = System.currentTimeMillis() - thrStart;
                                 System.out.println("task: " + taskId + ", get region metrics time usage: " + thrCost + "ms, tablegroup: " + tablegroup2);
-                                if (regionMetrics.size() != 9) {
+                                if (regionMetrics.size() != 3) {
                                     throw new ObTableGetException(
                                             "the number of region metrics does not match the number of tablets, the number of region metrics: " + regionMetrics.size());
                                 }
@@ -1624,12 +1624,7 @@ public void testHbaseDDLException() throws Exception {
         }
 
         // 11. check table exists from an uncreated namespace
-        try {
-            admin.tableExists(TableName.valueOf("n101:t1"));
-            fail();
-        } catch (Exception e) {
-            Assert.assertEquals(e.getClass(), NamespaceNotFoundException.class);
-        }
+        Assert.assertFalse(admin.tableExists(TableName.valueOf("n101:t1")));
 
     }