HDDS-10232. Refactor some constructors in SCM to avoid too many param…

…eters (apache#6114)

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/BlockManagerImpl.java

@@ -97,12 +97,9 @@ public BlockManagerImpl(final ConfigurationSource conf,
 
     // SCM block deleting transaction log and deleting service.
     deletedBlockLog = new DeletedBlockLogImpl(conf,
+        scm,
         scm.getContainerManager(),
-        scm.getScmHAManager().getRatisServer(),
-        scm.getScmMetadataStore().getDeletedBlocksTXTable(),
         scm.getScmHAManager().getDBTransactionBuffer(),
-        scm.getScmContext(),
-        scm.getSequenceIdGen(),
         metrics);
 
 

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/DeletedBlockLogImpl.java

@@ -46,9 +46,9 @@
 import org.apache.hadoop.hdds.scm.container.replication.ContainerHealthResult;
 import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager;
 import org.apache.hadoop.hdds.scm.ha.SCMContext;
-import org.apache.hadoop.hdds.scm.ha.SCMRatisServer;
 import org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator;
 import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer;
+import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
 import org.apache.hadoop.hdds.server.events.EventHandler;
 import org.apache.hadoop.hdds.server.events.EventPublisher;
 import org.apache.hadoop.hdds.utils.db.Table;
@@ -95,14 +95,10 @@ public class DeletedBlockLogImpl
 
   private static final int LIST_ALL_FAILED_TRANSACTIONS = -1;
 
-  @SuppressWarnings("parameternumber")
   public DeletedBlockLogImpl(ConfigurationSource conf,
+      StorageContainerManager scm,
       ContainerManager containerManager,
-      SCMRatisServer ratisServer,
-      Table<Long, DeletedBlocksTransaction> deletedBlocksTXTable,
       DBTransactionBuffer dbTxBuffer,
-      SCMContext scmContext,
-      SequenceIdGenerator sequenceIdGen,
       ScmBlockDeletingServiceMetrics metrics) {
     maxRetry = conf.getInt(OZONE_SCM_BLOCK_DELETION_MAX_RETRY,
         OZONE_SCM_BLOCK_DELETION_MAX_RETRY_DEFAULT);
@@ -112,17 +108,17 @@ public DeletedBlockLogImpl(ConfigurationSource conf,
     this.deletedBlockLogStateManager = DeletedBlockLogStateManagerImpl
         .newBuilder()
         .setConfiguration(conf)
-        .setDeletedBlocksTable(deletedBlocksTXTable)
+        .setDeletedBlocksTable(scm.getScmMetadataStore().getDeletedBlocksTXTable())
         .setContainerManager(containerManager)
-        .setRatisServer(ratisServer)
+        .setRatisServer(scm.getScmHAManager().getRatisServer())
         .setSCMDBTransactionBuffer(dbTxBuffer)
         .build();
-    this.scmContext = scmContext;
-    this.sequenceIdGen = sequenceIdGen;
+    this.scmContext = scm.getScmContext();
+    this.sequenceIdGen = scm.getSequenceIdGen();
     this.metrics = metrics;
     this.transactionStatusManager =
         new SCMDeletedBlockTransactionStatusManager(deletedBlockLogStateManager,
-            containerManager, scmContext, metrics, scmCommandTimeoutMs);
+            containerManager, this.scmContext, metrics, scmCommandTimeoutMs);
   }
 
   @Override

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplica.java

@@ -39,33 +39,21 @@ public final class ContainerReplica implements Comparable<ContainerReplica> {
   private final UUID placeOfBirth;
   private final int replicaIndex;
 
-  private Long sequenceId;
+  private final Long sequenceId;
   private final long keyCount;
   private final long bytesUsed;
   private final boolean isEmpty;
 
-  @SuppressWarnings("parameternumber")
-  private ContainerReplica(
-      final ContainerID containerID,
-      final ContainerReplicaProto.State state,
-      final int replicaIndex,
-      final DatanodeDetails datanode,
-      final UUID originNodeId,
-      long keyNum,
-      long dataSize,
-      boolean isEmpty) {
-    this.containerID = containerID;
-    this.state = state;
-    this.datanodeDetails = datanode;
-    this.placeOfBirth = originNodeId;
-    this.keyCount = keyNum;
-    this.bytesUsed = dataSize;
-    this.replicaIndex = replicaIndex;
-    this.isEmpty = isEmpty;
-  }
-
-  private void setSequenceId(Long seqId) {
-    sequenceId = seqId;
+  private ContainerReplica(ContainerReplicaBuilder b) {
+    containerID = b.containerID;
+    state = b.state;
+    datanodeDetails = b.datanode;
+    placeOfBirth = Optional.ofNullable(b.placeOfBirth).orElse(datanodeDetails.getUuid());
+    keyCount = b.keyCount;
+    bytesUsed = b.bytesUsed;
+    replicaIndex = b.replicaIndex;
+    isEmpty = b.isEmpty;
+    sequenceId = b.sequenceId;
   }
 
   /**
@@ -299,12 +287,7 @@ public ContainerReplica build() {
           "Container state can't be null");
       Preconditions.checkNotNull(datanode,
           "DatanodeDetails can't be null");
-      ContainerReplica replica = new ContainerReplica(
-          containerID, state, replicaIndex, datanode,
-          Optional.ofNullable(placeOfBirth).orElse(datanode.getUuid()),
-          keyCount, bytesUsed, isEmpty);
-      Optional.ofNullable(sequenceId).ifPresent(replica::setSequenceId);
-      return replica;
+      return new ContainerReplica(this);
     }
   }
 

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/BackgroundSCMService.java

@@ -45,17 +45,14 @@ public final class BackgroundSCMService implements SCMService {
   private final Runnable periodicalTask;
   private volatile boolean runImmediately = false;
 
-  private BackgroundSCMService(
-      final Clock clock, final SCMContext scmContext,
-      final String serviceName, final long intervalInMillis,
-      final long waitTimeInMillis, final Runnable task) {
-    this.scmContext = scmContext;
-    this.clock = clock;
-    this.periodicalTask = task;
-    this.serviceName = serviceName;
-    this.log = LoggerFactory.getLogger(serviceName);
-    this.intervalInMillis = intervalInMillis;
-    this.waitTimeInMillis = waitTimeInMillis;
+  private BackgroundSCMService(Builder b) {
+    scmContext = b.scmContext;
+    clock = b.clock;
+    periodicalTask = b.periodicalTask;
+    serviceName = b.serviceName;
+    log = LoggerFactory.getLogger(serviceName);
+    intervalInMillis = b.intervalInMillis;
+    waitTimeInMillis = b.waitTimeInMillis;
     start();
   }
 
@@ -206,8 +203,7 @@ public BackgroundSCMService build() {
       Preconditions.assertNotNull(clock, "clock is null");
       Preconditions.assertNotNull(serviceName, "serviceName is null");
 
-      return new BackgroundSCMService(clock, scmContext, serviceName,
-          intervalInMillis, waitTimeInMillis, periodicalTask);
+      return new BackgroundSCMService(this);
     }
   }
 }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMContext.java

@@ -27,7 +27,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.Optional;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -78,17 +77,13 @@ public static SCMContext emptyContext() {
    */
   private volatile FinalizationCheckpoint finalizationCheckpoint;
 
-  private SCMContext(boolean isLeader, long term,
-      final SafeModeStatus safeModeStatus,
-      final FinalizationCheckpoint finalizationCheckpoint,
-      final OzoneStorageContainerManager scm, String threadNamePrefix) {
-    this.isLeader = isLeader;
-    this.term = term;
-    this.safeModeStatus = safeModeStatus;
-    this.finalizationCheckpoint = finalizationCheckpoint;
-    this.scm = scm;
-    this.isLeaderReady = false;
-    this.threadNamePrefix = threadNamePrefix;
+  private SCMContext(Builder b) {
+    isLeader = b.isLeader;
+    term = b.term;
+    safeModeStatus = new SafeModeStatus(b.isInSafeMode, b.isPreCheckComplete);
+    finalizationCheckpoint = b.finalizationCheckpoint;
+    scm = b.scm;
+    threadNamePrefix = b.threadNamePrefix;
   }
 
   /**
@@ -104,9 +99,9 @@ public void updateLeaderAndTerm(boolean leader, long newTerm) {
       isLeader = leader;
       // If it is not leader, set isLeaderReady to false.
       if (!isLeader) {
-        isLeaderReady = false;
         LOG.info("update <isLeaderReady> from <{}> to <{}>", isLeaderReady,
             false);
+        isLeaderReady = false;
       }
       term = newTerm;
     } finally {
@@ -285,7 +280,7 @@ public static class Builder {
     private boolean isInSafeMode = false;
     private boolean isPreCheckComplete = true;
     private OzoneStorageContainerManager scm = null;
-    private FinalizationCheckpoint finalizationCheckpoint;
+    private FinalizationCheckpoint finalizationCheckpoint = FinalizationCheckpoint.FINALIZATION_COMPLETE;
     private String threadNamePrefix = "";
 
     public Builder setLeader(boolean leader) {
@@ -335,13 +330,7 @@ public SCMContext build() {
      */
     @VisibleForTesting
     SCMContext buildMaybeInvalid() {
-      return new SCMContext(
-          isLeader,
-          term,
-          new SafeModeStatus(isInSafeMode, isPreCheckComplete),
-          Optional.ofNullable(finalizationCheckpoint).orElse(
-              FinalizationCheckpoint.FINALIZATION_COMPLETE),
-          scm, threadNamePrefix);
+      return new SCMContext(this);
     }
   }
 }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMNodeDetails.java

@@ -19,8 +19,6 @@
 
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.hdds.NodeDetails;
-import org.apache.ratis.protocol.RaftGroup;
-import org.apache.ratis.protocol.RaftPeerId;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -30,38 +28,26 @@
  * Construct SCM node details.
  */
 public final class SCMNodeDetails extends NodeDetails {
-  private InetSocketAddress blockProtocolServerAddress;
-  private String blockProtocolServerAddressKey;
-  private InetSocketAddress clientProtocolServerAddress;
-  private String clientProtocolServerAddressKey;
-  private InetSocketAddress datanodeProtocolServerAddress;
-  private String datanodeAddressKey;
-  private int grpcPort;
+  private final InetSocketAddress blockProtocolServerAddress;
+  private final String blockProtocolServerAddressKey;
+  private final InetSocketAddress clientProtocolServerAddress;
+  private final String clientProtocolServerAddressKey;
+  private final InetSocketAddress datanodeProtocolServerAddress;
+  private final String datanodeAddressKey;
+  private final int grpcPort;
+
   public static final Logger LOG =
       LoggerFactory.getLogger(SCMNodeDetails.class);
 
-  /**
-   * Constructs SCMNodeDetails object.
-   */
-  @SuppressWarnings("checkstyle:ParameterNumber")
-  private SCMNodeDetails(String serviceId, String nodeId,
-      InetSocketAddress rpcAddr, int ratisPort, int grpcPort,
-      String httpAddress, String httpsAddress,
-      InetSocketAddress blockProtocolServerAddress,
-      InetSocketAddress clientProtocolServerAddress,
-      InetSocketAddress datanodeProtocolServerAddress, RaftGroup group,
-      RaftPeerId selfPeerId, String datanodeAddressKey,
-      String blockProtocolServerAddressKey,
-      String clientProtocolServerAddressAddressKey) {
-    super(serviceId, nodeId, rpcAddr, ratisPort,
-        httpAddress, httpsAddress);
-    this.grpcPort = grpcPort;
-    this.blockProtocolServerAddress = blockProtocolServerAddress;
-    this.clientProtocolServerAddress = clientProtocolServerAddress;
-    this.datanodeProtocolServerAddress = datanodeProtocolServerAddress;
-    this.datanodeAddressKey = datanodeAddressKey;
-    this.blockProtocolServerAddressKey = blockProtocolServerAddressKey;
-    this.clientProtocolServerAddressKey = clientProtocolServerAddressAddressKey;
+  private SCMNodeDetails(Builder b) {
+    super(b.scmServiceId, b.scmNodeId, b.rpcAddress, b.ratisPort, b.httpAddr, b.httpsAddr);
+    grpcPort = b.grpcPort;
+    blockProtocolServerAddress = b.blockProtocolServerAddress;
+    clientProtocolServerAddress = b.clientProtocolServerAddress;
+    datanodeProtocolServerAddress = b.datanodeProtocolServerAddress;
+    datanodeAddressKey = b.datanodeAddressKey;
+    blockProtocolServerAddressKey = b.blockProtocolServerAddressKey;
+    clientProtocolServerAddressKey = b.clientProtocolServerAddressKey;
   }
 
   @Override
@@ -96,8 +82,6 @@ public static class Builder {
     private String clientProtocolServerAddressKey;
     private InetSocketAddress datanodeProtocolServerAddress;
     private String datanodeAddressKey;
-    private RaftGroup raftGroup;
-    private RaftPeerId selfPeerId;
 
     public Builder setDatanodeAddressKey(String addressKey) {
       this.datanodeAddressKey = addressKey;
@@ -129,16 +113,6 @@ public Builder setDatanodeProtocolServerAddress(InetSocketAddress address) {
       return this;
     }
 
-    public Builder setRaftGroup(RaftGroup group) {
-      this.raftGroup = group;
-      return this;
-    }
-
-    public Builder setSelfPeerId(RaftPeerId peerId) {
-      this.selfPeerId = peerId;
-      return this;
-    }
-
     public Builder setRpcAddress(InetSocketAddress rpcAddr) {
       this.rpcAddress = rpcAddr;
       return this;
@@ -175,11 +149,7 @@ public Builder setHttpsAddress(String httpsAddress) {
     }
 
     public SCMNodeDetails build() {
-      return new SCMNodeDetails(scmServiceId, scmNodeId, rpcAddress,
-          ratisPort, grpcPort, httpAddr, httpsAddr, blockProtocolServerAddress,
-          clientProtocolServerAddress, datanodeProtocolServerAddress,
-          raftGroup, selfPeerId, datanodeAddressKey,
-          blockProtocolServerAddressKey, clientProtocolServerAddressKey);
+      return new SCMNodeDetails(this);
     }
   }
 

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/security/SecretKeyManagerService.java

@@ -61,7 +61,6 @@ public class SecretKeyManagerService implements SCMService, Runnable {
 
   private final ScheduledExecutorService scheduler;
 
-  @SuppressWarnings("parameternumber")
   public SecretKeyManagerService(SCMContext scmContext,
                                  ConfigurationSource conf,
                                  SCMRatisServer ratisServer) {

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java

@@ -128,12 +128,9 @@ public void setup() throws Exception {
         new SCMHADBTransactionBufferStub(scm.getScmMetadataStore().getStore());
     metrics = mock(ScmBlockDeletingServiceMetrics.class);
     deletedBlockLog = new DeletedBlockLogImpl(conf,
+        scm,
         containerManager,
-        scm.getScmHAManager().getRatisServer(),
-        scm.getScmMetadataStore().getDeletedBlocksTXTable(),
         scmHADBTransactionBuffer,
-        scm.getScmContext(),
-        scm.getSequenceIdGen(),
         metrics);
     dnList = new ArrayList<>(3);
     setupContainerManager();
@@ -734,12 +731,9 @@ public void testPersistence() throws Exception {
     // transactions are stored persistently.
     deletedBlockLog.close();
     deletedBlockLog = new DeletedBlockLogImpl(conf,
+        scm,
         containerManager,
-        scm.getScmHAManager().getRatisServer(),
-        scm.getScmMetadataStore().getDeletedBlocksTXTable(),
         scmHADBTransactionBuffer,
-        scm.getScmContext(),
-        scm.getSequenceIdGen(),
         metrics);
     List<DeletedBlocksTransaction> blocks =
         getTransactions(10 * BLOCKS_PER_TXN * THREE);
@@ -753,12 +747,9 @@ public void testPersistence() throws Exception {
     // currentTxnID = 50
     deletedBlockLog.close();
     new DeletedBlockLogImpl(conf,
+        scm,
         containerManager,
-        scm.getScmHAManager().getRatisServer(),
-        scm.getScmMetadataStore().getDeletedBlocksTXTable(),
         scmHADBTransactionBuffer,
-        scm.getScmContext(),
-        scm.getSequenceIdGen(),
         metrics);
     blocks = getTransactions(40 * BLOCKS_PER_TXN * THREE);
     assertEquals(0, blocks.size());