errose28 commented on code in PR #7266:
URL: https://github.com/apache/ozone/pull/7266#discussion_r1819742080


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java:
##########
@@ -382,6 +383,7 @@ public abstract static class Builder<T extends Builder<T>> {
     private boolean failedVolume = false;
     private String datanodeUuid;
     private String clusterID;
+    private long failureDate;

Review Comment:
   Lets use `failureTime`. I'm assuming this is being stored as millis since 
epoch, so it will have data and time information.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/VolumeFailureSubCommand.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.admin.scm;
+
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.scm.cli.ScmSubcommand;
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+import org.apache.hadoop.hdds.scm.datanode.VolumeFailureInfo;
+import org.apache.hadoop.hdds.server.JsonUtils;
+import org.apache.hadoop.ozone.utils.FormattingCLIUtils;
+import org.apache.hadoop.util.StringUtils;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+
+import java.io.IOException;
+import java.text.SimpleDateFormat;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+
+/**
+ * Handler of ozone admin scm volumesfailure command.
+ */
+@Command(
+    name = "volumesfailure",

Review Comment:
   For the CLI, we should probably use something like `ozone admin datanode 
volume list`. The `datanode` subcommand is already used to retrieve information 
about datanodes from SCM. Splitting the commands so that `volume` has its own 
subcommand gives us more options in the future.
   
   To distinguish failed and healthy volumes and filter out different nodes, we 
can either add some kind of filter flag, or leave it up to grep/jq to be 
applied to the output.



##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/StorageVolume.java:
##########
@@ -548,6 +571,11 @@ public ConfigurationSource getConf() {
 
   public void failVolume() {
     setState(VolumeState.FAILED);
+    // Ensure it is set only once,
+    // which is the time when the failure was first detected.
+    if (failureDate == 0L) {
+      setFailureDate(Time.now());

Review Comment:
   Let's use `Instant.now()` per HDDS-7911.



##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/scm/VolumeFailureSubCommand.java:
##########
@@ -0,0 +1,101 @@
+/*
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * 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.admin.scm;
+
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.scm.cli.ScmSubcommand;
+import org.apache.hadoop.hdds.scm.client.ScmClient;
+import org.apache.hadoop.hdds.scm.datanode.VolumeFailureInfo;
+import org.apache.hadoop.hdds.server.JsonUtils;
+import org.apache.hadoop.ozone.utils.FormattingCLIUtils;
+import org.apache.hadoop.util.StringUtils;
+import picocli.CommandLine.Command;
+import picocli.CommandLine.Option;
+
+import java.io.IOException;
+import java.text.SimpleDateFormat;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+
+/**
+ * Handler of ozone admin scm volumesfailure command.
+ */
+@Command(
+    name = "volumesfailure",

Review Comment:
   This also means we should make the RPC more generic to support pulling all 
volume information.



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