diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java index 20f87fc1af0b..37c114fca0f6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionMover.java @@ -527,9 +527,6 @@ private boolean unloadRegions(boolean unloadFromRack, List isolateRegion @InterfaceAudience.Private Collection filterRSGroupServers(RSGroupInfo rsgroup, Collection onlineServers) { - if (rsgroup.getName().equals(RSGroupInfo.DEFAULT_GROUP)) { - return onlineServers; - } List serverLists = new ArrayList<>(rsgroup.getServers().size()); for (ServerName server : onlineServers) { Address address = Address.fromParts(server.getHostname(), server.getPort()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverFilterRSGroupServers.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverFilterRSGroupServers.java new file mode 100644 index 000000000000..66838282d64b --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverFilterRSGroupServers.java @@ -0,0 +1,204 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import org.apache.hadoop.hbase.HBaseTestingUtil; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.net.Address; +import org.apache.hadoop.hbase.rsgroup.RSGroupInfo; +import org.apache.hadoop.hbase.rsgroup.RSGroupUtil; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.MiscTests; +import org.apache.hadoop.hbase.util.RegionMover.RegionMoverBuilder; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link RegionMover#filterRSGroupServers}. RSGroups is enabled on the mini cluster + * and servers are moved between groups via the real {@code moveServersToRSGroup} admin call -- no + * test constructs an {@link RSGroupInfo} by hand. Every {@code RSGroupInfo} used here is read back + * from the master via {@code admin.getRSGroup(...)}, which computes "default" membership as "online + * servers minus servers claimed by other groups" (see RSGroupInfoManagerImpl#getDefaultServers), so + * its membership shrinks on its own once a move happens instead of being asserted into existence by + * the test. + */ +@Tag(MiscTests.TAG) +@Tag(MediumTests.TAG) +public class TestRegionMoverFilterRSGroupServers { + + private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil(); + + @BeforeAll + public static void setUpBeforeClass() throws Exception { + RSGroupUtil.enableRSGroup(TEST_UTIL.getConfiguration()); + TEST_UTIL.startMiniCluster(2); + } + + @AfterAll + public static void tearDownAfterClass() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + private RegionMover buildMover() throws Exception { + ServerName any = TEST_UTIL.getAdmin().getRegionServers().iterator().next(); + return new RegionMoverBuilder(any.getHostname() + ":" + any.getPort(), + TEST_UTIL.getConfiguration()).build(); + } + + private static Address addressOf(ServerName sn) { + return Address.fromParts(sn.getHostname(), sn.getPort()); + } + + /** + * Reproduces HBASE-30331: once a server is moved out of "default" into a custom group, the + * master-computed "default" RSGroupInfo only lists the server that remains. filterRSGroupServers + * must honor that real membership instead of short-circuiting on the group name and returning + * every online server. + */ + @Test + public void testDefaultGroupFiltersToActualMembers() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + List allServers = new ArrayList<>(admin.getRegionServers()); + assertEquals(2, allServers.size(), "Mini cluster should have started with 2 region servers"); + + ServerName movedOut = allServers.get(0); + ServerName inDefault = allServers.get(1); + String groupName = "test_default_filter"; + + admin.addRSGroup(groupName); + admin.moveServersToRSGroup(new HashSet<>(List.of(addressOf(movedOut))), groupName); + try { + // Master-computed membership, not something we constructed ourselves. + RSGroupInfo defaultGroup = admin.getRSGroup(RSGroupInfo.DEFAULT_GROUP); + assertEquals(1, defaultGroup.getServers().size(), + "Master-computed default group should shrink to 1 member after moving the other " + + "server out"); + assertTrue(defaultGroup.containsServer(addressOf(inDefault)), + "Server that was not moved out must remain in the default group's real membership"); + + try (RegionMover rm = buildMover()) { + Collection result = rm.filterRSGroupServers(defaultGroup, allServers); + + assertEquals(1, result.size(), + "filterRSGroupServers should return only the default group's actual members"); + assertTrue(result.contains(inDefault), + "Server that is an actual member of the default group must be returned as a " + + "destination"); + assertFalse(result.contains(movedOut), + "Server moved out of default must not be returned as a destination just because the " + + "group being filtered is named 'default'"); + } + } finally { + admin.moveServersToRSGroup(new HashSet<>(List.of(addressOf(movedOut))), + RSGroupInfo.DEFAULT_GROUP); + admin.removeRSGroup(groupName); + } + } + + /** + * A non-default group with one member must return only that member. Same real-cluster approach as + * above: the group and its membership come from actual {@code moveServersToRSGroup} calls, not a + * hand-built {@link RSGroupInfo}. + */ + @Test + public void testNonDefaultGroupFiltersToMembers() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + List allServers = new ArrayList<>(admin.getRegionServers()); + assertEquals(2, allServers.size(), "Mini cluster should have started with 2 region servers"); + + ServerName member = allServers.get(0); + ServerName other = allServers.get(1); + String groupName = "test_nondefault_filter"; + + admin.addRSGroup(groupName); + admin.moveServersToRSGroup(new HashSet<>(List.of(addressOf(member))), groupName); + try { + RSGroupInfo group = admin.getRSGroup(groupName); + assertEquals(1, group.getServers().size(), + "Master-computed group should have exactly the one server moved into it"); + assertTrue(group.containsServer(addressOf(member)), + "Server moved into the group must be part of its real membership"); + + try (RegionMover rm = buildMover()) { + Collection result = rm.filterRSGroupServers(group, allServers); + assertEquals(1, result.size(), + "filterRSGroupServers should return only the non-default group's actual members"); + assertTrue(result.contains(member), + "Server that is an actual member of the group must be returned as a destination"); + assertFalse(result.contains(other), + "Server that is not a member of the group must not be returned as a destination"); + } + } finally { + admin.moveServersToRSGroup(new HashSet<>(List.of(addressOf(member))), + RSGroupInfo.DEFAULT_GROUP); + admin.removeRSGroup(groupName); + } + } + + /** + * A group's real member must not be returned as a destination when it is absent from the + * {@code onlineServers} snapshot handed to the filter (e.g. the server is currently offline or + * was already excluded upstream). Uses a real group/member from {@code moveServersToRSGroup}, not + * a fabricated, never-existed host address. + */ + @Test + public void testGroupMemberAbsentFromOnlineServersReturnsEmpty() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + List allServers = new ArrayList<>(admin.getRegionServers()); + assertEquals(2, allServers.size(), "Mini cluster should have started with 2 region servers"); + + ServerName member = allServers.get(0); + ServerName other = allServers.get(1); + String groupName = "test_absent_filter"; + + admin.addRSGroup(groupName); + admin.moveServersToRSGroup(new HashSet<>(List.of(addressOf(member))), groupName); + try { + RSGroupInfo group = admin.getRSGroup(groupName); + assertEquals(1, group.getServers().size(), + "Master-computed group should have exactly the one server moved into it"); + assertTrue(group.containsServer(addressOf(member)), + "Server moved into the group must be part of its real membership"); + + try (RegionMover rm = buildMover()) { + // The group's only member is not part of the online-servers snapshot passed in. + Collection result = + rm.filterRSGroupServers(group, Collections.singletonList(other)); + assertTrue(result.isEmpty(), + "Group member absent from the online-servers snapshot must not be returned as a " + + "destination"); + } + } finally { + admin.moveServersToRSGroup(new HashSet<>(List.of(addressOf(member))), + RSGroupInfo.DEFAULT_GROUP); + admin.removeRSGroup(groupName); + } + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverWithRSGroupEnable.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverWithRSGroupEnable.java index 3d261da92789..4a8dad55f7b4 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverWithRSGroupEnable.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestRegionMoverWithRSGroupEnable.java @@ -18,6 +18,8 @@ package org.apache.hadoop.hbase.util; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.ArrayList; import java.util.Collection; @@ -32,6 +34,7 @@ import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.net.Address; +import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; import org.apache.hadoop.hbase.rsgroup.RSGroupInfo; import org.apache.hadoop.hbase.rsgroup.RSGroupUtil; @@ -39,6 +42,7 @@ import org.apache.hadoop.hbase.testclassification.MiscTests; import org.apache.hadoop.hbase.util.RegionMover.RegionMoverBuilder; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -69,7 +73,11 @@ public static void tearDownAfterClass() throws Exception { TEST_UTIL.shutdownMiniCluster(); } + private static final TableName TABLE_NAME = TableName.valueOf("testRegionMoverWithRSGroupEnable"); + private final List
rsservers = new ArrayList<>(2); + private final List defaultGroupServers = new ArrayList<>(); + private ServerName rsContainMeta; @BeforeEach public void setUp() throws Exception { @@ -79,7 +87,7 @@ public void setUp() throws Exception { admin.addRSGroup(TEST_RSGROUP); Collection allServers = admin.getRegionServers(); // Remove rs contains hbase:meta, otherwise test looks unstable and buggy in test env. - ServerName rsContainMeta = TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().stream() + rsContainMeta = TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().stream() .map(t -> t.getRegionServer()) .filter(rs -> rs.getRegions(TableName.META_TABLE_NAME).size() > 0).findFirst().get() .getServerName(); @@ -97,12 +105,18 @@ public void setUp() throws Exception { assertEquals(3, admin.getRSGroup(RSGroupInfo.DEFAULT_GROUP).getServers().size()); assertEquals(2, admin.getRSGroup(TEST_RSGROUP).getServers().size()); + // Track the servers left in the default group, used for isolation assertions. + for (ServerName server : allServers) { + if (!rsservers.contains(Address.fromParts(server.getHostname(), server.getPort()))) { + defaultGroupServers.add(server); + } + } + // Create a pre-split table in test rsgroup - TableName tableName = TableName.valueOf("testRegionMoverWithRSGroupEnable"); - if (admin.tableExists(tableName)) { - TEST_UTIL.deleteTable(tableName); + if (admin.tableExists(TABLE_NAME)) { + TEST_UTIL.deleteTable(TABLE_NAME); } - TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(tableName) + TableDescriptor tableDesc = TableDescriptorBuilder.newBuilder(TABLE_NAME) .setColumnFamily(ColumnFamilyDescriptorBuilder.of("f")).setRegionServerGroup(TEST_RSGROUP) .build(); String startKey = "a"; @@ -110,6 +124,23 @@ public void setUp() throws Exception { admin.createTable(tableDesc, Bytes.toBytes(startKey), Bytes.toBytes(endKey), 9); } + @AfterEach + public void tearDown() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + if (admin.tableExists(TABLE_NAME)) { + TEST_UTIL.deleteTable(TABLE_NAME); + } + if (!rsservers.isEmpty()) { + admin.moveServersToRSGroup(new HashSet<>(rsservers), RSGroupInfo.DEFAULT_GROUP); + } + if (admin.getRSGroup(TEST_RSGROUP) != null) { + admin.removeRSGroup(TEST_RSGROUP); + } + rsservers.clear(); + defaultGroupServers.clear(); + rsContainMeta = null; + } + @Test public void testUnloadRegions() throws Exception { Address decommission = rsservers.get(0); @@ -125,6 +156,82 @@ public void testUnloadRegions() throws Exception { .map(JVMClusterUtil.RegionServerThread::getRegionServer) .filter(rs -> rs.getServerName().getAddress().equals(online)).findFirst().get(); assertEquals(9, onlineRS.getNumberOfOnlineRegions()); + + // Isolation assertion: no default-group server must hold any region of the test table. + for (ServerName defaultSN : defaultGroupServers) { + HRegionServer defaultRS = TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().stream() + .map(JVMClusterUtil.RegionServerThread::getRegionServer) + .filter(rs -> rs.getServerName().equals(defaultSN)).findFirst().orElse(null); + if (defaultRS == null) { + continue; + } + List tableRegions = defaultRS.getRegions(TABLE_NAME); + assertTrue(tableRegions.isEmpty(), "Default-group server " + defaultSN + + " must not hold any regions of " + TABLE_NAME + " but had: " + tableRegions); + } + } + + /** + * Unloading a server that is in the default RSGroup must still succeed end-to-end when RSGroups + * are enabled. Destinations must be filtered to the default group: regions may spread across the + * other default-group servers, but must not land on any test-group server. + */ + @Test + public void testUnloadDefaultGroupServerWithRSGroupEnabled() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + // Avoid unloading the meta-carrying server here too, for the same stability reason setUp() + // avoids it when picking rsservers. + ServerName defaultSN = + defaultGroupServers.stream().filter(sn -> !sn.equals(rsContainMeta)).findFirst().get(); + Address decommission = defaultSN.getAddress(); + String filename = new Path(TEST_UTIL.getDataTestDir(), "testDefaultGroupUnload").toString(); + + // Create a table in the default group; the balancer will distribute its regions naturally + // across the default-group servers, so defaultSN will hold at least some. + TableName defaultTable = TableName.valueOf("testDefaultGroupTable"); + if (admin.tableExists(defaultTable)) { + TEST_UTIL.deleteTable(defaultTable); + } + try { + TableDescriptor td = TableDescriptorBuilder.newBuilder(defaultTable) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of("f")).build(); + admin.createTable(td, Bytes.toBytes("a"), Bytes.toBytes("z"), 6); + TEST_UTIL.waitTableAvailable(defaultTable); + + HRegionServer decommRS = TEST_UTIL.getMiniHBaseCluster().getRegionServerThreads().stream() + .map(JVMClusterUtil.RegionServerThread::getRegionServer) + .filter(rs -> rs.getServerName().equals(defaultSN)).findFirst().get(); + assertFalse(decommRS.getRegions(defaultTable).isEmpty(), + "Precondition: decommissioned server must actually host some regions of the default " + + "table, otherwise the post-unload check below is vacuous"); + + RegionMoverBuilder builder = + new RegionMoverBuilder(decommission.toString(), TEST_UTIL.getConfiguration()); + try (RegionMover rm = builder.filename(filename).ack(true).build()) { + LOG.info("Unloading default-group server {}", decommission.getHostname()); + rm.unload(); + } + + // After unload, the decommissioned server must hold no regions of the default table. + assertEquals(0, decommRS.getRegions(defaultTable).size(), + "Decommissioned default-group server must hold no regions after unload"); + + // Isolation assertion: no test-group server must hold any region of the default table. + for (JVMClusterUtil.RegionServerThread rst : TEST_UTIL.getMiniHBaseCluster() + .getRegionServerThreads()) { + HRegionServer rs = rst.getRegionServer(); + Address addr = rs.getServerName().getAddress(); + if (rsservers.contains(addr)) { + List found = rs.getRegions(defaultTable); + assertTrue(found.isEmpty(), "Test-group server " + addr + " must not hold any region of " + + defaultTable + " but had: " + found); + } + } + } finally { + if (admin.tableExists(defaultTable)) { + TEST_UTIL.deleteTable(defaultTable); + } + } } }