From 558913ea1ad8a2c0ae5f153406c547e775079e0e Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Thu, 4 May 2023 20:59:07 +0800 Subject: [PATCH] HBASE-27823 NPE in ClaimReplicationQueuesProcedure when running TestAssignmentManager.testAssignSocketTimeout (#5216) Also done some cleanup around the MockMasterServices related classes and tests Signed-off-by: Liangjun He (cherry picked from commit 89e80da57f29b501ffe9ca852bca8dcd3462ef86) --- .../TestCoreMasterCoprocessor.java | 5 ++- .../master/assignment/MockMasterServices.java | 32 +++++++++++++------ .../assignment/TestAssignmentManager.java | 2 +- .../assignment/TestAssignmentManagerBase.java | 5 +-- .../master/janitor/TestCatalogJanitor.java | 13 ++------ .../procedure/TestServerRemoteProcedure.java | 20 ++++-------- 6 files changed, 35 insertions(+), 42 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoreMasterCoprocessor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoreMasterCoprocessor.java index e134285c330..051875c5942 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoreMasterCoprocessor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoreMasterCoprocessor.java @@ -54,9 +54,8 @@ public class TestCoreMasterCoprocessor { private MasterCoprocessorHost mch; @Before - public void before() throws IOException { - String methodName = this.name.getMethodName(); - this.ms = new MockMasterServices(HTU.getConfiguration(), null); + public void before() throws Exception { + this.ms = new MockMasterServices(HTU.getConfiguration()); this.mch = new MasterCoprocessorHost(this.ms, HTU.getConfiguration()); this.mch.preMasterInitialization(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java index 72d28ea634a..91fc75510e9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java @@ -20,12 +20,13 @@ package org.apache.hadoop.hbase.master.assignment; import static org.apache.hadoop.hbase.HConstants.DEFAULT_HBASE_SPLIT_COORDINATED_BY_ZK; import static org.apache.hadoop.hbase.HConstants.HBASE_SPLIT_WAL_COORDINATED_BY_ZK; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.IOException; +import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.NavigableMap; -import java.util.SortedSet; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.CoordinatedStateManager; @@ -55,17 +56,19 @@ import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.master.procedure.RSProcedureDispatcher; import org.apache.hadoop.hbase.master.region.MasterRegion; import org.apache.hadoop.hbase.master.region.MasterRegionFactory; +import org.apache.hadoop.hbase.master.replication.ReplicationPeerManager; import org.apache.hadoop.hbase.procedure2.ProcedureEvent; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; import org.apache.hadoop.hbase.procedure2.store.NoopProcedureStore; import org.apache.hadoop.hbase.procedure2.store.ProcedureStore; import org.apache.hadoop.hbase.procedure2.store.ProcedureStore.ProcedureStoreListener; +import org.apache.hadoop.hbase.replication.ReplicationException; +import org.apache.hadoop.hbase.replication.ReplicationQueueStorage; import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.util.CommonFSUtils; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.zookeeper.KeeperException; -import org.mockito.Mockito; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; @@ -98,14 +101,14 @@ public class MockMasterServices extends MockNoopMasterServices { private final ClusterConnection connection; private final LoadBalancer balancer; private final ServerManager serverManager; + private final ReplicationPeerManager rpm; private final ProcedureEvent initialized = new ProcedureEvent<>("master initialized"); public static final String DEFAULT_COLUMN_FAMILY_NAME = "cf"; public static final ServerName MOCK_MASTER_SERVERNAME = ServerName.valueOf("mockmaster.example.org", 1234, -1L); - public MockMasterServices(Configuration conf, - NavigableMap> regionsToRegionServers) throws IOException { + public MockMasterServices(Configuration conf) throws IOException, ReplicationException { super(conf); Superusers.initialize(conf); this.fileSystemManager = new MasterFileSystem(conf); @@ -120,22 +123,22 @@ public class MockMasterServices extends MockNoopMasterServices { new AssignmentManager(this, masterRegion, new MockRegionStateStore(this, masterRegion)); this.balancer = LoadBalancerFactory.getLoadBalancer(conf); this.serverManager = new ServerManager(this, new DummyRegionServerList()); - this.tableStateManager = Mockito.mock(TableStateManager.class); - Mockito.when(this.tableStateManager.getTableState(Mockito.any())).thenReturn(new TableState( + this.tableStateManager = mock(TableStateManager.class); + when(this.tableStateManager.getTableState(any())).thenReturn(new TableState( TableName.valueOf("AnyTableNameSetInMockMasterServcies"), TableState.State.ENABLED)); // Mock up a Client Interface ClientProtos.ClientService.BlockingInterface ri = - Mockito.mock(ClientProtos.ClientService.BlockingInterface.class); + mock(ClientProtos.ClientService.BlockingInterface.class); MutateResponse.Builder builder = MutateResponse.newBuilder(); builder.setProcessed(true); try { - Mockito.when(ri.mutate(any(), any())).thenReturn(builder.build()); + when(ri.mutate(any(), any())).thenReturn(builder.build()); } catch (ServiceException se) { throw ProtobufUtil.handleRemoteException(se); } try { - Mockito.when(ri.multi(any(), any())).thenAnswer(new Answer() { + when(ri.multi(any(), any())).thenAnswer(new Answer() { @Override public MultiResponse answer(InvocationOnMock invocation) throws Throwable { return buildMultiResponse(invocation.getArgument(1)); @@ -153,6 +156,10 @@ public class MockMasterServices extends MockNoopMasterServices { // Set hbase.rootdir into test dir. Path rootdir = CommonFSUtils.getRootDir(getConfiguration()); CommonFSUtils.setRootDir(getConfiguration(), rootdir); + this.rpm = mock(ReplicationPeerManager.class); + ReplicationQueueStorage rqs = mock(ReplicationQueueStorage.class); + when(rqs.getAllQueues(any())).thenReturn(Collections.emptyList()); + when(rpm.getQueueStorage()).thenReturn(rqs); } public void start(final int numServes, final RSProcedureDispatcher remoteDispatcher) @@ -364,4 +371,9 @@ public class MockMasterServices extends MockNoopMasterServices { public SplitWALManager getSplitWALManager() { return splitWALManager; } + + @Override + public ReplicationPeerManager getReplicationPeerManager() { + return rpm; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java index 025aebb3e08..a0434788e78 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java @@ -233,7 +233,7 @@ public class TestAssignmentManager extends TestAssignmentManagerBase { util = new HBaseTestingUtility(); this.executor = Executors.newSingleThreadScheduledExecutor(); setupConfiguration(util.getConfiguration()); - master = new MockMasterServices(util.getConfiguration(), this.regionsToRegionServers); + master = new MockMasterServices(util.getConfiguration()); rsDispatcher = new MockRSProcedureDispatcher(master); master.start(NSERVERS, rsDispatcher); am = master.getAssignmentManager(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerBase.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerBase.java index 5618549f323..8ed065d20ad 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerBase.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerBase.java @@ -67,7 +67,6 @@ import org.apache.hadoop.ipc.RemoteException; import org.junit.After; import org.junit.Before; import org.junit.Rule; -import org.junit.rules.ExpectedException; import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -96,8 +95,6 @@ public abstract class TestAssignmentManagerBase { @Rule public TestName name = new TestName(); - @Rule - public final ExpectedException exception = ExpectedException.none(); protected static final int PROC_NTHREADS = 64; protected static final int NREGIONS = 1 * 1000; @@ -157,7 +154,7 @@ public abstract class TestAssignmentManagerBase { this.executor = Executors.newSingleThreadScheduledExecutor(new ThreadFactoryBuilder() .setUncaughtExceptionHandler((t, e) -> LOG.warn("Uncaught: ", e)).build()); setupConfiguration(util.getConfiguration()); - master = new MockMasterServices(util.getConfiguration(), this.regionsToRegionServers); + master = new MockMasterServices(util.getConfiguration()); rsDispatcher = new MockRSProcedureDispatcher(master); master.start(NSERVERS, rsDispatcher); newRsAdded = 0; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java index 185c67feb76..c2e24d1f569 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/janitor/TestCatalogJanitor.java @@ -21,7 +21,7 @@ import static org.apache.hadoop.hbase.util.HFileArchiveTestingUtil.assertArchive import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.any; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -32,12 +32,9 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.NavigableMap; import java.util.Objects; import java.util.SortedMap; -import java.util.SortedSet; import java.util.TreeMap; -import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; @@ -49,7 +46,6 @@ import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.MetaMockingUtil; -import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; @@ -70,7 +66,6 @@ import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.CommonFSUtils; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.HFileArchiveUtil; -import org.apache.zookeeper.KeeperException; import org.junit.After; import org.junit.Before; import org.junit.BeforeClass; @@ -106,11 +101,9 @@ public class TestCatalogJanitor { } @Before - public void setup() throws IOException, KeeperException { + public void setup() throws Exception { setRootDirAndCleanIt(HTU, this.name.getMethodName()); - NavigableMap> regionsToRegionServers = - new ConcurrentSkipListMap>(); - this.masterServices = new MockMasterServices(HTU.getConfiguration(), regionsToRegionServers); + this.masterServices = new MockMasterServices(HTU.getConfiguration()); this.masterServices.start(10, null); this.janitor = new CatalogJanitor(masterServices); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerRemoteProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerRemoteProcedure.java index ce1a58da812..eac8510778d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerRemoteProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerRemoteProcedure.java @@ -21,11 +21,8 @@ import static org.apache.hadoop.hbase.master.procedure.ServerProcedureInterface. import static org.junit.Assert.fail; import java.io.IOException; -import java.util.NavigableMap; import java.util.Optional; import java.util.Set; -import java.util.SortedSet; -import java.util.concurrent.ConcurrentSkipListMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -57,7 +54,6 @@ import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.junit.rules.ExpectedException; import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -74,23 +70,19 @@ public class TestServerRemoteProcedure { HBaseClassTestRule.forClass(TestServerRemoteProcedure.class); @Rule public TestName name = new TestName(); - @Rule - public final ExpectedException exception = ExpectedException.none(); - protected HBaseTestingUtility util; - protected MockRSProcedureDispatcher rsDispatcher; - protected MockMasterServices master; - protected AssignmentManager am; - protected NavigableMap> regionsToRegionServers = - new ConcurrentSkipListMap<>(); + private HBaseTestingUtility util; + private MockRSProcedureDispatcher rsDispatcher; + private MockMasterServices master; + private AssignmentManager am; // Simple executor to run some simple tasks. - protected ScheduledExecutorService executor; + private ScheduledExecutorService executor; @Before public void setUp() throws Exception { util = new HBaseTestingUtility(); this.executor = Executors.newSingleThreadScheduledExecutor(new ThreadFactoryBuilder() .setUncaughtExceptionHandler((t, e) -> LOG.warn("Uncaught: ", e)).build()); - master = new MockMasterServices(util.getConfiguration(), this.regionsToRegionServers); + master = new MockMasterServices(util.getConfiguration()); rsDispatcher = new MockRSProcedureDispatcher(master); rsDispatcher.setMockRsExecutor(new NoopRSExecutor()); master.start(2, rsDispatcher);