guihecheng commented on code in PR #3292:
URL: https://github.com/apache/ozone/pull/3292#discussion_r849224867


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/HddsVolumeUtil.java:
##########
@@ -234,4 +257,154 @@ public static boolean checkVolume(HddsVolume hddsVolume, 
String scmId,
       return true;
     }
   }
+
+  /**
+   * Randomly pick a DbVolume for HddsVolume and init db instance.
+   * Use the HddsVolume directly if no DbVolume found.
+   * @param hddsVolume
+   * @param dbVolumeSet
+   * @param clusterID
+   * @param conf
+   */
+  public static void formatDbStoreForHddsVolume(HddsVolume hddsVolume,
+      MutableVolumeSet dbVolumeSet, String clusterID,
+      ConfigurationSource conf) throws IOException {
+    DbVolume chosenDbVolume = null;
+    File dbParentDir;
+
+    if (dbVolumeSet == null || dbVolumeSet.getVolumesList().isEmpty()) {
+      // No extra db volumes specified, just create db under the HddsVolume.
+      dbParentDir = new File(hddsVolume.getStorageDir(), clusterID);
+    } else {
+      // Randomly choose a DbVolume for simplicity.
+      List<DbVolume> dbVolumeList = StorageVolumeUtil.getDbVolumesList(
+          dbVolumeSet.getVolumesList());
+      chosenDbVolume = dbVolumeList.get(
+          ThreadLocalRandom.current().nextInt(dbVolumeList.size()));
+      dbParentDir = new File(chosenDbVolume.getStorageDir(), clusterID);
+    }
+
+    // Init subdir with the storageID of HddsVolume.
+    File storageIdDir = new File(dbParentDir, hddsVolume.getStorageID());
+    if (!storageIdDir.mkdirs() && !storageIdDir.exists()) {
+      throw new IOException("Can't make subdir under "
+          + dbParentDir.getAbsolutePath() + " for volume "
+          + hddsVolume.getStorageID());
+    }
+
+    // Init the db instance for HddsVolume under the subdir above.
+    String containerDBPath = new File(storageIdDir, CONTAINER_DB_NAME)
+        .getAbsolutePath();
+    try {
+      initPerDiskDBStore(containerDBPath, conf);
+    } catch (IOException e) {
+      throw new IOException("Can't init db instance under path "
+          + containerDBPath + " for volume " + hddsVolume.getStorageID());
+    }
+
+    // Set the dbVolume and dbParentDir of the HddsVolume for db path lookup.
+    hddsVolume.setDbVolume(chosenDbVolume);
+    hddsVolume.setDbParentDir(storageIdDir);
+  }
+
+  /**
+   * Load already formatted db instances for all HddsVolumes.
+   * @param hddsVolumeSet
+   * @param dbVolumeSet
+   * @param conf
+   * @param logger
+   */
+  public static void loadAllHddsVolumeDbStore(MutableVolumeSet hddsVolumeSet,
+      MutableVolumeSet dbVolumeSet, ConfigurationSource conf, Logger logger) {
+    // Scan subdirs under the db volumes and build a one-to-one map
+    // between each HddsVolume -> DbVolume.
+    mapDbVolumesToDataVolumesIfNeeded(hddsVolumeSet, dbVolumeSet);
+
+    List<HddsVolume> hddsVolumeList = StorageVolumeUtil.getHddsVolumesList(
+        hddsVolumeSet.getVolumesList());
+
+    for (HddsVolume volume : hddsVolumeList) {
+      String clusterID = volume.getClusterID();
+
+      // DN startup for the first time, not registered yet,
+      // so the DbVolume is not formatted.
+      if (clusterID == null) {
+        continue;
+      }
+
+      File clusterIdDir = new File(volume.getDbVolume() == null ?
+          volume.getStorageDir() : volume.getDbVolume().getStorageDir(),
+          clusterID);
+
+      if (!clusterIdDir.exists()) {
+        if (logger != null) {
+          logger.error("HddsVolume {} db instance not formatted, " +
+                  "clusterID {} directory not found",
+              volume.getStorageDir().getAbsolutePath(),
+              clusterIdDir.getAbsolutePath());
+        }
+        continue;

Review Comment:
   1. For the failed volume tolerated value, read this thread 
https://github.com/apache/ozone/pull/2243 which you've participant in, there's 
a discussion about the default 0 or -1. You could send a patch to fix it 
overall with the above reasons later after you have decided about the best 
default value, but not in this thread.
   2. For the "throw or continue" problem, first please provide an answer to my 
question first, what is the desired behavior(with no extra ssds) in your mind 
and the reason, then we try to reach an agreement.
   3. For the new question you raised, suppose there are 2 DbVolumes and one of 
them is down, the db instances on that one is unreadable and all related 
HddsVolumes are unreadable. This scenario equals to that half of the 
HddsVolumes are down. This is not a very extreme case, even the whole DN is 
down, data integrity and security is not fatally harmed. But there won't be any 
new db instances created on the remaining dbVolume as you said since db 
instances are only created when we try to format an HddsVolume on the DN 
registration, please recheck.
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to