You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/05/28 06:31:46 UTC

[GitHub] [ozone] guihecheng opened a new pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

guihecheng opened a new pull request #2290:
URL: https://github.com/apache/ozone/pull/2290


   ## What changes were proposed in this pull request?
   
   Ensure disk checker also scans the ratis log disks periodically.
   To do so, I did some refactoring work, and then add ratis log disks to disk checker.
   Here is a checklist:
   **Done**:
   - Introduce a base class `StorageVolume` and let `HddsVolume` extends it.
     VolumeInfo and disk check stuff are extracted into `StorageVolume`
   - Introduce a sub class `MetadataVolume` extends `StorageVolume`,
     so it shares the VolumeInfo and disk check stuff.
   - Migrate ratisVolumeMap from `XceiverServerRatis` to `MutableVolumeSet`,
     so we mange all kinds of volumes together.
   - The config 'failed.volumes.tolerated' takes failed metadata volumes into consideration.
   - Add VolumeInfo related fields into `MetadataStorageReportProto`,
     so SCM could get space info from datanodes.
   
   **Not Done**:
   - Handle the case that `MetadataVolume` and `HddsVolume` are on the same physical disk,
      so for now failed volumes may be double counted.
   - Try to check a single failed `MetadataVolume` on log failure.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5268
   
   ## How was this patch tested?
   
   Exisiting unit-tests and integration-tests.
   To add more tests.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-853752890


   > The approach looks good. I would prefer to have metadata and data volume in two different sets altogether as well. Do we need to consider the case where metadata and data volumes share the same underlying physical disk. I such cases, the disk checker might run more frequently. Its ok to not address the problem here but something to take note of.
   
   Sure, I've updated with the separated VolumeSets approach, also with separated 'tolerated' configs, checklist updated above.
   In order to check whether the underlying physical disks are the same, we need to access the fsid of the local fs, it should be nice to have but not very urgent.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] bshashikant commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r648853731



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolumeChecker.java
##########
@@ -109,11 +120,6 @@ public HddsVolumeChecker(ConfigurationSource conf, Timer timer)
     this.timer = timer;
 
     DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class);
-    /**
-     * Maximum number of volume failures that can be tolerated without
-     * declaring a fatal error.
-     */
-    int maxVolumeFailuresTolerated = dnConf.getFailedVolumesTolerated();
 
     minDiskCheckGapMs = conf.getTimeDuration(
         DFSConfigKeysLegacy.DFS_DATANODE_DISK_CHECK_MIN_GAP_KEY,

Review comment:
       I think, its better to avoid any legacy configs in the code. can be done later as well.

##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DatanodeLayout.java
##########
@@ -96,11 +97,14 @@ public static void main(String[] args) {
     OzoneContainer.buildContainerSet(volumeSet, containerSet, conf);
     volumeSet.shutdown();
 
+    List<HddsVolume> failedVolumes = StorageVolumeUtil.getHddsVolumesList(
+        volumeSet.getFailedVolumesList());
+
     if (verify) {
-      for (HddsVolume vol : volumeSet.getFailedVolumesList()) {
+      for (HddsVolume vol : failedVolumes) {
         System.out.println("Failed Volume:" + vol.getHddsRootDir());
       }
     }
-    return volumeSet.getFailedVolumesList();
+    return failedVolumes;

Review comment:
       I think, its better to log if there is any failed metadata volume here as well.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r648878447



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DatanodeLayout.java
##########
@@ -96,11 +97,14 @@ public static void main(String[] args) {
     OzoneContainer.buildContainerSet(volumeSet, containerSet, conf);
     volumeSet.shutdown();
 
+    List<HddsVolume> failedVolumes = StorageVolumeUtil.getHddsVolumesList(
+        volumeSet.getFailedVolumesList());
+
     if (verify) {
-      for (HddsVolume vol : volumeSet.getFailedVolumesList()) {
+      for (HddsVolume vol : failedVolumes) {
         System.out.println("Failed Volume:" + vol.getHddsRootDir());
       }
     }
-    return volumeSet.getFailedVolumesList();
+    return failedVolumes;

Review comment:
       OK.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r649037091



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
##########
@@ -43,8 +43,10 @@
       "hdds.datanode.container.delete.threads.max";
   static final String PERIODIC_DISK_CHECK_INTERVAL_MINUTES_KEY =
       "hdds.datanode.periodic.disk.check.interval.minutes";
-  public static final String FAILED_VOLUMES_TOLERATED_KEY =
-      "hdds.datanode.failed.volumes.tolerated";
+  public static final String FAILED_HDDS_VOLUMES_TOLERATED_KEY =
+      "hdds.datanode.failed.hdds.volumes.tolerated";

Review comment:
       OK.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.container.common.volume;
+
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.fs.SpaceUsageCheckFactory;
+import org.apache.hadoop.hdfs.server.datanode.StorageLocation;
+import org.apache.hadoop.hdfs.server.datanode.checker.Checkable;
+import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult;
+import org.apache.hadoop.util.DiskChecker;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.util.Objects;
+
+/**
+ * StorageVolume represents a generic Volume in datanode, could be
+ * 1. HddsVolume for container storage.
+ * 2. MetadataVolume for metadata(ratis) storage.
+ */
+public abstract class StorageVolume
+    implements Checkable<Boolean, VolumeCheckResult> {
+
+  /**
+   * Type for StorageVolume.
+   */
+  public enum VolumeType {
+    HDDS_VOLUME,

Review comment:
       OK.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-853752890


   > The approach looks good. I would prefer to have metadata and data volume in two different sets altogether as well. Do we need to consider the case where metadata and data volumes share the same underlying physical disk. I such cases, the disk checker might run more frequently. Its ok to not address the problem here but something to take note of.
   
   Sure, I've updated with the separated VolumeSets approach, also with separated 'tolerated' configs, checklist updated above.
   In order to check whether the underlying physical disks are the same, we need to access the fsid of the local fs, it should be nice to have but not very urgent.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r648882888



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolumeChecker.java
##########
@@ -109,11 +120,6 @@ public HddsVolumeChecker(ConfigurationSource conf, Timer timer)
     this.timer = timer;
 
     DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class);
-    /**
-     * Maximum number of volume failures that can be tolerated without
-     * declaring a fatal error.
-     */
-    int maxVolumeFailuresTolerated = dnConf.getFailedVolumesTolerated();
 
     minDiskCheckGapMs = conf.getTimeDuration(
         DFSConfigKeysLegacy.DFS_DATANODE_DISK_CHECK_MIN_GAP_KEY,

Review comment:
       Ah yes, there are many legacy configs left, 2-3 are disk checker related, will fix it here.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] ChenSammi commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r649017987



##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java
##########
@@ -138,7 +143,7 @@ public void testAddVolume() {
   public void testFailVolume() throws Exception {
 
     //Fail a volume
-    volumeSet.failVolume(volume1);
+    volumeSet.failVolume(volume1 + "/hdds");

Review comment:
       Why need add the "/hdds" now? 




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] ChenSammi commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r649003789



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.container.common.volume;
+
+import org.apache.hadoop.fs.StorageType;
+import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.fs.SpaceUsageCheckFactory;
+import org.apache.hadoop.hdfs.server.datanode.StorageLocation;
+import org.apache.hadoop.hdfs.server.datanode.checker.Checkable;
+import org.apache.hadoop.hdfs.server.datanode.checker.VolumeCheckResult;
+import org.apache.hadoop.util.DiskChecker;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.util.Objects;
+
+/**
+ * StorageVolume represents a generic Volume in datanode, could be
+ * 1. HddsVolume for container storage.
+ * 2. MetadataVolume for metadata(ratis) storage.
+ */
+public abstract class StorageVolume
+    implements Checkable<Boolean, VolumeCheckResult> {
+
+  /**
+   * Type for StorageVolume.
+   */
+  public enum VolumeType {
+    HDDS_VOLUME,

Review comment:
       Suggest use DATA_VOLUME instead.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-850179822


   Hi @bshashikant @ChenSammi , here is the first version of this work, please take a look.
   I just present it to check whether I was on a proper way to do it, if there are other ideas to do this, we could discuss, thanks~


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] bshashikant commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-857609969


   @ChenSammi , can you please have a look?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] bshashikant commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-853618531


   The approach looks good. I would prefer to have metadata and data volume in two different sets altogether as well. Do we need to consider the case where metadata and data volumes share the same underlying physical disk. I such cases, the disk checker might run more frequently. Its ok to not address the problem here but something to take note of. 


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] ChenSammi merged pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
ChenSammi merged pull request #2290:
URL: https://github.com/apache/ozone/pull/2290


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-852690836


   > Thanks @guihecheng for working on this. Two general comments,
   > 
   > 1. The config "hdds.datanode.failed.volumes.tolerated" takes failed metadata volumes into consideration.  I would suggest use seperate properties to control the metadata volumes and data volumes.
   
   I'm fine with this separation, for example we could have 'hdds.datanode.failed.data.volumes.tolerated' and 'hdds.datanode.failed.meta.volumes.tolerated' and check them each.
   
   > 2. Would it be more clear that using different MutableVolumeSet to hold the data volume set and meta volume set?
   
   Yes, I think they could be in separate VolumeSets, then I think we could extract the periodic disk checker into HddsVolumeChecker and let these 2 VolumeSets register there volumes for check, then we retain a single checker for different kinds of volumes. Then it should be easy to add the potential rocksdb volumes mentioned below, in the future.
   
   > Please also do consider if the current abstration is enough if rocksdbs are combined into one instance and saved on a different disk than current data disk in future.
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] ChenSammi commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-852668780


   Thanks @guihecheng for working on this.   Two general comments, 
   1.  The config "hdds.datanode.failed.volumes.tolerated" takes failed metadata volumes into consideration.  I would suggest use seperate properties to control the metadata volumes and data volumes. 
   2. Would it be more clear that using different MutableVolumeSet to hold the data volume set and meta volume set? 
   
   Please also do consider if the current abstration is enough if rocksdbs are combined into one instance and saved on a different disk than current data disk in future. 


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] ChenSammi commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r648997884



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
##########
@@ -43,8 +43,10 @@
       "hdds.datanode.container.delete.threads.max";
   static final String PERIODIC_DISK_CHECK_INTERVAL_MINUTES_KEY =
       "hdds.datanode.periodic.disk.check.interval.minutes";
-  public static final String FAILED_VOLUMES_TOLERATED_KEY =
-      "hdds.datanode.failed.volumes.tolerated";
+  public static final String FAILED_HDDS_VOLUMES_TOLERATED_KEY =
+      "hdds.datanode.failed.hdds.volumes.tolerated";

Review comment:
       I would suggest use "hdds.datanode.failed.data.volumes.tolerated" instead.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] bshashikant commented on pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#issuecomment-853618531


   The approach looks good. I would prefer to have metadata and data volume in two different sets altogether as well. Do we need to consider the case where metadata and data volumes share the same underlying physical disk. I such cases, the disk checker might run more frequently. Its ok to not address the problem here but something to take note of. 


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r649042048



##########
File path: hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/volume/TestVolumeSet.java
##########
@@ -138,7 +143,7 @@ public void testAddVolume() {
   public void testFailVolume() throws Exception {
 
     //Fail a volume
-    volumeSet.failVolume(volume1);
+    volumeSet.failVolume(volume1 + "/hdds");

Review comment:
       Because the original test is using HddsVolume, originally, failVolume will use a function `getHddsRoot()` to append the "/hdds" for volume1, but now since the volume is StorageVolume, so I do it by hand.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org

[GitHub] [ozone] guihecheng commented on a change in pull request #2290: HDDS-5268. Ensure disk checker also scans the ratis log disks periodically

Posted by GitBox <gi...@apache.org>.
guihecheng commented on a change in pull request #2290:
URL: https://github.com/apache/ozone/pull/2290#discussion_r649094012



##########
File path: hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/DatanodeLayout.java
##########
@@ -96,11 +97,14 @@ public static void main(String[] args) {
     OzoneContainer.buildContainerSet(volumeSet, containerSet, conf);
     volumeSet.shutdown();
 
+    List<HddsVolume> failedVolumes = StorageVolumeUtil.getHddsVolumesList(
+        volumeSet.getFailedVolumesList());
+
     if (verify) {
-      for (HddsVolume vol : volumeSet.getFailedVolumesList()) {
+      for (HddsVolume vol : failedVolumes) {
         System.out.println("Failed Volume:" + vol.getHddsRootDir());
       }
     }
-    return volumeSet.getFailedVolumesList();
+    return failedVolumes;

Review comment:
       Oh, here I think it is dedicated to HddsVolumes since this `dnlayout` command tries to verify the on-disk layout of all data paths or the single data path. So we don't have to check metadata volume here.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org