You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by st...@apache.org on 2020/07/06 14:14:07 UTC

[phoenix] branch 4.x updated: PHOENIX-5779 SplitSystemCatalogIT tests fail with Multiple Regions error (addendum - different fix)

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

stoty pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/4.x by this push:
     new f27af68  PHOENIX-5779 SplitSystemCatalogIT tests fail with Multiple Regions error (addendum - different fix)
f27af68 is described below

commit f27af68329bb5ee89cbc8e7ce744dab1b1fefee2
Author: Istvan Toth <st...@apache.org>
AuthorDate: Thu Jul 2 16:31:37 2020 +0200

    PHOENIX-5779 SplitSystemCatalogIT tests fail with Multiple Regions error (addendum - different fix)
    
    Co-authored-by: Richard Antal <an...@gmail.com>
---
 .../phoenix/end2end/SplitSystemCatalogIT.java      |  2 +
 .../it/java/org/apache/phoenix/end2end/ViewIT.java |  2 +
 .../org/apache/phoenix/end2end/ViewMetadataIT.java |  2 +
 .../java/org/apache/phoenix/query/BaseTest.java    | 71 ++++++++--------------
 4 files changed, 30 insertions(+), 47 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SplitSystemCatalogIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SplitSystemCatalogIT.java
index 673c632..8c8f391 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SplitSystemCatalogIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SplitSystemCatalogIT.java
@@ -57,6 +57,8 @@ public class SplitSystemCatalogIT extends BaseTest {
         setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator()));
         // Split SYSTEM.CATALOG once after the mini-cluster is started
         if (splitSystemCatalog) {
+            // splitSystemCatalog is incompatible with the balancer chore
+            getUtility().getHBaseCluster().getMaster().balanceSwitch(false);
             splitSystemCatalog();
         }
     }
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
index c962740..e896513 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
@@ -140,6 +140,8 @@ public class ViewIT extends SplitSystemCatalogIT {
         setUpTestDriver(new ReadOnlyProps(serverProps.entrySet().iterator()), new ReadOnlyProps(props.entrySet().iterator()));
         // Split SYSTEM.CATALOG once after the mini-cluster is started
         if (splitSystemCatalog) {
+            // splitSystemCatalog is incompatible with the balancer chore
+            getUtility().getHBaseCluster().getMaster().balanceSwitch(false);
             splitSystemCatalog();
         }
     }
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewMetadataIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewMetadataIT.java
index eccfed6..a6beba9 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewMetadataIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewMetadataIT.java
@@ -100,6 +100,8 @@ public class ViewMetadataIT extends SplitSystemCatalogIT {
                 new ReadOnlyProps(clientProps.entrySet().iterator()));
         // Split SYSTEM.CATALOG once after the mini-cluster is started
         if (splitSystemCatalog) {
+            // splitSystemCatalog is incompatible with the balancer chore
+            getUtility().getHBaseCluster().getMaster().balanceSwitch(false);
             splitSystemCatalog();
         }
 
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
index 7e1ba12..a1e794a 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java
@@ -92,9 +92,11 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Types;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -102,7 +104,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
-import java.util.HashSet;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -1838,31 +1839,6 @@ public abstract class BaseTest {
         return false;
     }
 
-    protected static HashMap<ServerName, List<HRegionInfo>> getRegionMap(Admin admin,
-            HBaseTestingUtility util, TableName fullTableName, List<byte[]> splitPoints)
-            throws IOException {
-        MiniHBaseCluster cluster = util.getHBaseCluster();
-        HMaster master = cluster.getMaster();
-        AssignmentManager am = master.getAssignmentManager();
-        List<HRegionInfo> tableRegions =
-                admin.getTableRegions(fullTableName);
-        HashMap<ServerName, List<HRegionInfo>> serverToRegionsMap =
-                Maps.newHashMapWithExpectedSize(NUM_SLAVES_BASE);
-
-        for (HRegionInfo hRegionInfo : tableRegions) {
-            // filter on regions we are interested in
-            if (regionContainsMetadataRows(hRegionInfo, splitPoints)) {
-                ServerName serverName = am.getRegionStates().getRegionServerOfRegion(hRegionInfo);
-                if (!serverToRegionsMap.containsKey(serverName)) {
-                    serverToRegionsMap.put(serverName, new ArrayList<HRegionInfo>());
-                }
-                serverToRegionsMap.get(serverName).add(hRegionInfo);
-            }
-        }
-        return serverToRegionsMap;
-
-    }
-
     protected static void splitTable(TableName fullTableName, List<byte[]> splitPoints) throws Exception {
         HBaseAdmin admin =
                 driver.getConnectionQueryServices(getUrl(), TestUtil.TEST_PROPERTIES).getAdmin();
@@ -1873,42 +1849,43 @@ public abstract class BaseTest {
         HBaseTestingUtility util = getUtility();
         MiniHBaseCluster cluster = util.getHBaseCluster();
         HMaster master = cluster.getMaster();
+        //We don't want BalancerChore to undo our hard work
+        assertFalse("Balancer must be off", master.isBalancerOn());
         AssignmentManager am = master.getAssignmentManager();
         // No need to split on the first splitPoint since the end key of region boundaries are exclusive
         for (int i=1; i<splitPoints.size(); ++i) {
             splitRegion(fullTableName, splitPoints.get(i));
         }
-        HashSet<ServerName> allRegionServers = new HashSet<>(NUM_SLAVES_BASE);
+        HashMap<ServerName, List<HRegionInfo>> serverToRegionsList = Maps.newHashMapWithExpectedSize(NUM_SLAVES_BASE);
+        Deque<ServerName> availableRegionServers = new ArrayDeque<ServerName>(NUM_SLAVES_BASE);
         for (int i=0; i<NUM_SLAVES_BASE; ++i) {
-            allRegionServers.add(util.getHBaseCluster().getRegionServer(i).getServerName());
+            availableRegionServers.push(util.getHBaseCluster().getRegionServer(i).getServerName());
         }
-        HashSet<ServerName> availableRegionServers;
-        HashMap<ServerName, List<HRegionInfo>> serverToRegionsMap = getRegionMap(admin,
-                util, fullTableName, splitPoints);
-
-        for (Entry<ServerName, List<HRegionInfo>> entry : serverToRegionsMap.entrySet()) {
+        List<HRegionInfo> tableRegions =
+                admin.getTableRegions(fullTableName);
+        for (HRegionInfo hRegionInfo : tableRegions) {
+            // filter on regions we are interested in
+            if (regionContainsMetadataRows(hRegionInfo, splitPoints)) {
+                ServerName serverName = am.getRegionStates().getRegionServerOfRegion(hRegionInfo);
+                if (!serverToRegionsList.containsKey(serverName)) {
+                    serverToRegionsList.put(serverName, new ArrayList<HRegionInfo>());
+                }
+                serverToRegionsList.get(serverName).add(hRegionInfo);
+                availableRegionServers.remove(serverName);
+            }
+        }
+        assertTrue("No region servers available to move regions on to ", !availableRegionServers.isEmpty());
+        for (Entry<ServerName, List<HRegionInfo>> entry : serverToRegionsList.entrySet()) {
             List<HRegionInfo> regions = entry.getValue();
             if (regions.size()>1) {
                 for (int i=1; i< regions.size(); ++i) {
-                    availableRegionServers = new HashSet<>(NUM_SLAVES_BASE);
-                    for (ServerName serverName: allRegionServers) {
-                        availableRegionServers.add(serverName);
-                    }
-                    //PHOENIX-5779 refresh the server to region map
-                    //The regions may have been moved either by us, or by an HBase chore
-                    HashMap<ServerName, List<HRegionInfo>> currentServerToRegionsMap =
-                            getRegionMap(admin, util, fullTableName, splitPoints);
-                    availableRegionServers.removeAll(currentServerToRegionsMap.keySet());
-                    assertTrue("No region servers available to move regions on to ",
-                            !availableRegionServers.isEmpty());
-                    ServerName serverName = availableRegionServers.iterator().next();
-                    moveRegion(regions.get(i), entry.getKey(), serverName);
+                    moveRegion(regions.get(i), entry.getKey(), availableRegionServers.pop());
                 }
             }
         }
 
         // verify each region is on its own region server
-        List<HRegionInfo> tableRegions =
+        tableRegions =
                 admin.getTableRegions(fullTableName);
         Set<ServerName> serverNames = Sets.newHashSet();
         for (HRegionInfo hRegionInfo : tableRegions) {