Make SCM constructor private and have SCMRatisServer constructed in s…

…tatic method.
apache · May 13, 2020 · ba67c76 · ba67c76
1 parent a777392
commit ba67c76
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 38 deletions.
diff --git a/...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/...s/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
@@ -220,7 +220,7 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
    *
    * @param conf configuration
    */
-  public StorageContainerManager(OzoneConfiguration conf)
+  private StorageContainerManager(OzoneConfiguration conf)
       throws IOException, AuthenticationException {
     // default empty configurator means default managers will be used.
     this(conf, new SCMConfigurator());
@@ -236,7 +236,7 @@ public StorageContainerManager(OzoneConfiguration conf)
    * @param conf - Configuration
    * @param configurator - configurator
    */
-  public StorageContainerManager(OzoneConfiguration conf,
+  private StorageContainerManager(OzoneConfiguration conf,
                                  SCMConfigurator configurator)
       throws IOException, AuthenticationException  {
     super(HddsVersionInfo.HDDS_VERSION_INFO);
@@ -269,15 +269,9 @@ public StorageContainerManager(OzoneConfiguration conf,
       loginAsSCMUser(conf);
     }
 
-    // Initialize SCM Ratis Server
-    if (SCMHAUtils.isSCMHAEnabled(conf)) {
-      this.scmRatisSnapshotInfo = new SCMRatisSnapshotInfo(
-          scmStorageConfig.getCurrentDir());
-      this.scmRatisSnapshotDir = SCMHAUtils.createSCMRatisDir(conf);
-      initializeRatisServer();
-    } else {
-      scmRatisServer = null;
-    }
+    this.scmRatisSnapshotInfo = new SCMRatisSnapshotInfo(
+        scmStorageConfig.getCurrentDir());
+    this.scmRatisSnapshotDir = SCMHAUtils.createSCMRatisDir(conf);
 
     // Creates the SCM DBs or opens them if it exists.
     // A valid pointer to the store is required by all the other services below.
@@ -388,6 +382,42 @@ public StorageContainerManager(OzoneConfiguration conf,
     registerMetricsSource(this);
   }
 
+  /**
+   * Create an SCM instance based on the supplied configuration.
+   *
+   * @param conf        HDDS configuration
+   * @return SCM instance
+   * @throws IOException, AuthenticationException
+   */
+  public static StorageContainerManager createSCM(
+      OzoneConfiguration conf, SCMConfigurator configurator)
+      throws IOException, AuthenticationException {
+    StorageContainerManager scm = new StorageContainerManager(
+          conf, configurator);
+    if (SCMHAUtils.isSCMHAEnabled(conf) && scm.getScmRatisServer() == null) {
+      SCMRatisServer scmRatisServer = initializeRatisServer(conf, scm);
+      scm.setScmRatisServer(scmRatisServer);
+    }
+    return scm;
+  }
+
+  /**
+   * Create an SCM instance based on the supplied configuration.
+   *
+   * @param conf        HDDS configuration
+   * @return SCM instance
+   * @throws IOException, AuthenticationException
+   */
+  public static StorageContainerManager createSCM(OzoneConfiguration conf)
+      throws IOException, AuthenticationException {
+    StorageContainerManager scm = new StorageContainerManager(conf);
+    if (SCMHAUtils.isSCMHAEnabled(conf) && scm.getScmRatisServer() == null) {
+      SCMRatisServer scmRatisServer = initializeRatisServer(conf, scm);
+      scm.setScmRatisServer(scmRatisServer);
+    }
+    return scm;
+  }
+
   /**
    * This function initializes the following managers. If the configurator
    * specifies a value, we will use it, else we will use the default value.
@@ -634,18 +664,6 @@ public static RPC.Server startRpcServer(
     return rpcServer;
   }
 
-  /**
-   * Create an SCM instance based on the supplied configuration.
-   *
-   * @param conf        HDDS configuration
-   * @return SCM instance
-   * @throws IOException, AuthenticationException
-   */
-  public static StorageContainerManager createSCM(OzoneConfiguration conf)
-      throws IOException, AuthenticationException {
-    return new StorageContainerManager(conf);
-  }
-
   /**
    * Routine to set up the Version info for StorageContainerManager.
    *
@@ -1138,27 +1156,31 @@ public NetworkTopology getClusterMap() {
     return this.clusterMap;
   }
 
-  private void initializeRatisServer() throws IOException {
-    if (scmRatisServer == null) {
-      SCMNodeDetails scmNodeDetails = SCMNodeDetails
-          .initStandAlone(configuration);
-      //TODO enable Ratis group
-      scmRatisServer = SCMRatisServer.newSCMRatisServer(configuration
-              .getObject(SCMRatisServer.SCMRatisServerConfiguration.class),
-          this, scmNodeDetails, Collections.EMPTY_LIST,
-          SCMRatisServer.getSCMRatisDirectory(configuration));
-      if (scmRatisServer != null) {
-        LOG.info("SCM Ratis server initialized at port {}",
-            scmRatisServer.getServerPort());
-      }
+  private static SCMRatisServer initializeRatisServer(
+      OzoneConfiguration conf, StorageContainerManager scm) throws IOException {
+    SCMNodeDetails scmNodeDetails = SCMNodeDetails
+        .initStandAlone(conf);
+    //TODO enable Ratis group
+    SCMRatisServer scmRatisServer = SCMRatisServer.newSCMRatisServer(
+        conf.getObject(SCMRatisServer.SCMRatisServerConfiguration.class),
+        scm, scmNodeDetails, Collections.EMPTY_LIST,
+        SCMRatisServer.getSCMRatisDirectory(conf));
+    if (scmRatisServer != null) {
+      LOG.info("SCM Ratis server initialized at port {}",
+          scmRatisServer.getServerPort());
     }
+    return scmRatisServer;
   }
 
   @VisibleForTesting
   public SCMRatisServer getScmRatisServer() {
     return scmRatisServer;
   }
 
+  public void setScmRatisServer(SCMRatisServer scmRatisServer) {
+    this.scmRatisServer = scmRatisServer;
+  }
+
   @VisibleForTesting
   public SCMRatisSnapshotInfo getSnapshotInfo() {
     return scmRatisSnapshotInfo;

diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestUtils.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/TestUtils.java
@@ -497,7 +497,7 @@ public static StorageContainerManager getScm(OzoneConfiguration conf,
       // writes the version file properties
       scmStore.initialize();
     }
-    return new StorageContainerManager(conf, configurator);
+    return StorageContainerManager.createSCM(conf, configurator);
   }
 
   public static ContainerInfo getContainer(

diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genesis/GenesisUtil.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genesis/GenesisUtil.java
@@ -133,7 +133,7 @@ static StorageContainerManager getScm(OzoneConfiguration conf,
       // writes the version file properties
       scmStore.initialize();
     }
-    return new StorageContainerManager(conf, configurator);
+    return StorageContainerManager.createSCM(conf, configurator);
   }
 
   static void configureSCM(OzoneConfiguration conf, int numHandlers) {