errose28 commented on code in PR #6608: URL: https://github.com/apache/ozone/pull/6608#discussion_r1610796161
########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/common/FSOBaseCLI.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.common; + +import org.apache.hadoop.hdds.cli.SubcommandWithParent; +import org.kohsuke.MetaInfServices; +import picocli.CommandLine; + +import java.util.concurrent.Callable; + +/** + * Parser for scm.db file. + */ [email protected]( + name = "fso-tree", + description = "Identify a disconnected FSO tree, and optionally mark " + + "unreachable entries for deletion. OM should be " + + "stopped while this tool is run. Information will be logged at " + + "INFO and DEBUG levels." +) +@MetaInfServices(SubcommandWithParent.class) +public class FSOBaseCLI implements Callable<Void>, SubcommandWithParent { + + @CommandLine.Option(names = {"--db"}, + required = true, + description = "Path to OM RocksDB") + private String dbPath; + + @CommandLine.Option(names = {"--verbose"}, + description = "More verbose output. ") + private boolean verbose; + + + @Override + public Void call() throws Exception { + + try { + // TODO case insensitive enum options. Review Comment: We can add this to the change, it should be easy to support. I think this was copy/pasted into the other child classes too. ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/common/FSOBaseCLI.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.common; + +import org.apache.hadoop.hdds.cli.SubcommandWithParent; +import org.kohsuke.MetaInfServices; +import picocli.CommandLine; + +import java.util.concurrent.Callable; + +/** + * Parser for scm.db file. + */ [email protected]( + name = "fso-tree", + description = "Identify a disconnected FSO tree, and optionally mark " + + "unreachable entries for deletion. OM should be " + + "stopped while this tool is run. Information will be logged at " + + "INFO and DEBUG levels." +) +@MetaInfServices(SubcommandWithParent.class) +public class FSOBaseCLI implements Callable<Void>, SubcommandWithParent { + + @CommandLine.Option(names = {"--db"}, + required = true, + description = "Path to OM RocksDB") + private String dbPath; + + @CommandLine.Option(names = {"--verbose"}, + description = "More verbose output. ") + private boolean verbose; + + + @Override + public Void call() throws Exception { + + try { + // TODO case insensitive enum options. + FSOBaseTool + baseTool = new FSOBaseTool(dbPath, true); + baseTool.run(); + } catch (Exception ex) { + throw new IllegalArgumentException("FSO inspection failed: " + ex.getMessage()); + } + + if (verbose) { + System.out.println("FSO inspection finished. See client logs for results."); Review Comment: This needs to be updated as well since I think in a previous comment we decided to output everything to stdout and use `--verbose` to change the level of output. This is more user friendly than log4j for client output. ########## hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksDB.java: ########## @@ -87,6 +89,11 @@ public static ManagedRocksDB open( ); } + public static ManagedRocksDB open(final String path) throws RocksDBException { Review Comment: I think we actually want to use `RocksDatabase` here, which wraps `ManagedRocksDB`. Looks like it should already have all the operations we need implemented except drop column family, which would need to be added to `RocksDatabase`. ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/common/FSOBaseCLI.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.common; + +import org.apache.hadoop.hdds.cli.SubcommandWithParent; +import org.kohsuke.MetaInfServices; +import picocli.CommandLine; + +import java.util.concurrent.Callable; + +/** + * Parser for scm.db file. + */ Review Comment: This is an old comment that needs to be updated. Child classes have this as well. ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/common/FSOBaseCLI.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.common; + +import org.apache.hadoop.hdds.cli.SubcommandWithParent; +import org.kohsuke.MetaInfServices; +import picocli.CommandLine; + +import java.util.concurrent.Callable; + +/** + * Parser for scm.db file. + */ [email protected]( + name = "fso-tree", + description = "Identify a disconnected FSO tree, and optionally mark " + + "unreachable entries for deletion. OM should be " + + "stopped while this tool is run. Information will be logged at " + + "INFO and DEBUG levels." +) +@MetaInfServices(SubcommandWithParent.class) +public class FSOBaseCLI implements Callable<Void>, SubcommandWithParent { Review Comment: I'm not sure there's enough shared code to warrant a base class for debug and repair. The shared CLI flags like `--db` and `--verbose` can be used in both commands with PicoCLI mixins. -- 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]
