From 70cfe2525e9732350a343b27c985b589166eac75 Mon Sep 17 00:00:00 2001 From: Mohammad Arshad Date: Tue, 14 Jul 2020 21:24:18 +0530 Subject: [PATCH] HBASE-24721: rename_rsgroup overwriting the existing rsgroup Closes #2059 Signed-off-by: Reid Chan Signed-off-by: Viraj Jasani --- .../hbase/rsgroup/RSGroupInfoManagerImpl.java | 6 ++- .../hbase/rsgroup/TestRSGroupsAdmin1.java | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java index 1cb51cf38c9..3cad9391137 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java @@ -1227,9 +1227,13 @@ final class RSGroupInfoManagerImpl implements RSGroupInfoManager { throw new ConstraintException(RSGroupInfo.DEFAULT_GROUP + " can't be rename"); } checkGroupName(newName); - + //getRSGroupInfo validates old RSGroup existence. RSGroupInfo oldRSG = getRSGroupInfo(oldName); Map rsGroupMap = holder.groupName2Group; + if (rsGroupMap.containsKey(newName)) { + throw new ConstraintException("Group already exists: " + newName); + } + Map newGroupMap = Maps.newHashMap(rsGroupMap); newGroupMap.remove(oldRSG.getName()); RSGroupInfo newRSG = new RSGroupInfo(newName, oldRSG.getServers()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java index 2af3f9d6d85..bf7a2fd57ff 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin1.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.rsgroup; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -534,4 +535,55 @@ public class TestRSGroupsAdmin1 extends TestRSGroupsBase { assertEquals(newgroup.getName(), ADMIN.getRSGroup(tb1).getName()); assertEquals(normal.getName(), ADMIN.getRSGroup(tb2).getName()); } + + @Test + public void testRenameRSGroupConstraints() throws Exception { + // Add RSGroup, and assign 2 servers and a table to it. + String oldGroupName = "oldGroup"; + RSGroupInfo oldGroup = addGroup(oldGroupName, 2); + oldGroup = ADMIN.getRSGroup(oldGroup.getName()); + assertNotNull(oldGroup); + assertEquals(2, oldGroup.getServers().size()); + + //Add another RSGroup + String anotherRSGroupName = "anotherRSGroup"; + RSGroupInfo anotherGroup = addGroup(anotherRSGroupName, 1); + anotherGroup = ADMIN.getRSGroup(anotherGroup.getName()); + assertNotNull(anotherGroup); + assertEquals(1, anotherGroup.getServers().size()); + + + //Rename a non existing RSGroup + try { + ADMIN.renameRSGroup("nonExistingRSGroup", "newRSGroup1"); + fail("ConstraintException was expected."); + } catch (ConstraintException e){ + assertTrue(e.getMessage().contains("does not exist")); + } + + //Rename to existing group + try { + ADMIN.renameRSGroup(oldGroup.getName(), anotherRSGroupName); + fail("ConstraintException was expected."); + } catch (ConstraintException e){ + assertTrue(e.getMessage().contains("Group already exists")); + } + + //Rename default RSGroup + try { + ADMIN.renameRSGroup(RSGroupInfo.DEFAULT_GROUP, "newRSGroup2"); + fail("ConstraintException was expected."); + } catch (ConstraintException e){ + //Do nothing + } + + //Rename to default RSGroup + try { + ADMIN.renameRSGroup(oldGroup.getName(), RSGroupInfo.DEFAULT_GROUP); + fail("ConstraintException was expected."); + } catch (ConstraintException e){ + assertTrue(e.getMessage().contains("Group already exists")); + } + + } }