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 <heliangjun@apache.org>
(cherry picked from commit 89e80da57f29b501ffe9ca852bca8dcd3462ef86)

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 {
+  public void before() throws Exception {
-    String methodName = this.name.getMethodName();
+    this.ms = new MockMasterServices(HTU.getConfiguration());
-    this.ms = new MockMasterServices(HTU.getConfiguration(), null);
     this.mch = new MasterCoprocessorHost(this.ms, HTU.getConfiguration());
     this.mch.preMasterInitialization();
   }

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,
+  public MockMasterServices(Configuration conf) throws IOException, ReplicationException {
-    NavigableMap<ServerName, SortedSet<byte[]>> regionsToRegionServers) throws IOException {
     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);
+    this.tableStateManager = mock(TableStateManager.class);
-    Mockito.when(this.tableStateManager.getTableState(Mockito.any())).thenReturn(new TableState(
+    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<MultiResponse>() {
+      when(ri.multi(any(), any())).thenAnswer(new Answer<MultiResponse>() {
         @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;
+  }
 }

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();

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;

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<ServerName, SortedSet<byte[]>> regionsToRegionServers =
+    this.masterServices = new MockMasterServices(HTU.getConfiguration());
-      new ConcurrentSkipListMap<ServerName, SortedSet<byte[]>>();
-    this.masterServices = new MockMasterServices(HTU.getConfiguration(), regionsToRegionServers);
     this.masterServices.start(10, null);
     this.janitor = new CatalogJanitor(masterServices);
   }

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
+  private HBaseTestingUtility util;
-  public final ExpectedException exception = ExpectedException.none();
+  private MockRSProcedureDispatcher rsDispatcher;
-  protected HBaseTestingUtility util;
+  private MockMasterServices master;
-  protected MockRSProcedureDispatcher rsDispatcher;
+  private AssignmentManager am;
-  protected MockMasterServices master;
-  protected AssignmentManager am;
-  protected NavigableMap<ServerName, SortedSet<byte[]>> regionsToRegionServers =
-    new ConcurrentSkipListMap<>();
   // 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);