HBASE-27823 NPE in ClaimReplicationQueuesProcedure when running TestA…

…ssignmentManager.testAssignSocketTimeout (apache#5216) Also done some cleanup around the MockMasterServices related classes and tests Signed-off-by: Liangjun He <[email protected]> (cherry picked from commit 89e80da)
vinayakphegde · May 4, 2023 · 0b8fa5a · 0b8fa5a
1 parent af78169
commit 0b8fa5a
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 43 deletions.
diff --git a/...e-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoreMasterCoprocessor.java b/...e-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
@@ -20,12 +20,13 @@
 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;
@@ -52,16 +53,18 @@
 import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants;
 import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv;
 import org.apache.hadoop.hbase.master.procedure.RSProcedureDispatcher;
+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.zookeeper.KeeperException;
-import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
@@ -93,14 +96,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<ServerName, SortedSet<byte[]>> regionsToRegionServers) throws IOException {
+  public MockMasterServices(Configuration conf) throws IOException, ReplicationException {
     super(conf);
     Superusers.initialize(conf);
     this.fileSystemManager = new MasterFileSystem(conf);
@@ -114,22 +117,22 @@ public MockMasterServices(Configuration conf,
     this.assignmentManager = new AssignmentManager(this, new MockRegionStateStore(this));
     this.balancer = LoadBalancerFactory.getLoadBalancer(conf);
     this.serverManager = new ServerManager(this);
-    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<MultiResponse>() {
+      when(ri.multi(any(), any())).thenAnswer(new Answer<MultiResponse>() {
         @Override
         public MultiResponse answer(InvocationOnMock invocation) throws Throwable {
           return buildMultiResponse(invocation.getArgument(1));
@@ -142,11 +145,15 @@ public MultiResponse answer(InvocationOnMock invocation) throws Throwable {
     // ClusterConnection return the HRI. Have the HRI return a few mocked up responses
     // to make our test work.
     this.connection = HConnectionTestingUtility.getMockedConnectionAndDecorate(getConfiguration(),
-      Mockito.mock(AdminProtos.AdminService.BlockingInterface.class), ri, MOCK_MASTER_SERVERNAME,
+      mock(AdminProtos.AdminService.BlockingInterface.class), ri, MOCK_MASTER_SERVERNAME,
       RegionInfoBuilder.FIRST_META_REGIONINFO);
     // 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)
@@ -356,4 +363,9 @@ private static MultiResponse buildMultiResponse(MultiRequest req) {
   public SplitWALManager getSplitWALManager() {
     return splitWALManager;
   }
+
+  @Override
+  public ReplicationPeerManager getReplicationPeerManager() {
+    return rpm;
+  }
 }
diff --git a/...server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java b/...server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java
@@ -231,7 +231,7 @@ public void testAssignMetaAndCrashBeforeResponse() throws Exception {
     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/...er/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerBase.java b/...er/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManagerBase.java
@@ -66,7 +66,6 @@
 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;
@@ -95,8 +94,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;
@@ -156,7 +153,7 @@ public void setUp() throws Exception {
     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
@@ -21,7 +21,7 @@
 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.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.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.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 static void beforeClass() throws Exception {
   }
 
   @Before
-  public void setup() throws IOException, KeeperException {
+  public void setup() throws Exception {
     setRootDirAndCleanIt(HTU, this.name.getMethodName());
-    NavigableMap<ServerName, SortedSet<byte[]>> regionsToRegionServers =
-      new ConcurrentSkipListMap<ServerName, SortedSet<byte[]>>();
-    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/...ver/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerRemoteProcedure.java b/...ver/src/test/java/org/apache/hadoop/hbase/master/procedure/TestServerRemoteProcedure.java
@@ -21,11 +21,8 @@
 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.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<ServerName, SortedSet<byte[]>> 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);