adoroszlai commented on code in PR #8282: URL: https://github.com/apache/ozone/pull/8282#discussion_r2046298905
########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ListContainers.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.debug.container; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase; +import picocli.CommandLine; + + +/** + * List containers based on the parameter given. + */ + [email protected]( + name = "list-containers", + description = "Finds containers from the database based on the option provided." +) +public class ListContainers implements Callable<Void> { + /** + * Container states. + * Used to filter container records during database query. + */ + public enum ContainerState { + OPEN, CLOSED, QUASI_CLOSED, CLOSING, DELETED, UNHEALTHY + } + + @CommandLine.Option(names = {"--state", "-s"}, Review Comment: Please don't add `-s`, since it would conflict with short option of `--start` (from `ListOptions). ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ListContainers.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.debug.container; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase; +import picocli.CommandLine; + + +/** + * List containers based on the parameter given. + */ + [email protected]( + name = "list-containers", + description = "Finds containers from the database based on the option provided." +) +public class ListContainers implements Callable<Void> { + /** + * Container states. + * Used to filter container records during database query. + */ + public enum ContainerState { + OPEN, CLOSED, QUASI_CLOSED, CLOSING, DELETED, UNHEALTHY + } + + @CommandLine.Option(names = {"--state", "-s"}, + description = "state of the container") + private ContainerState state; + + @CommandLine.Option(names = {"--limit", "-l"}, + description = "number of rows to display in the output , by default it would be 100 rows") + private Integer limit; Review Comment: Can we use `ListOptions` mixin for consistency with other list commands? https://github.com/apache/ozone/blob/55f6924a1f09152bcc2997fc93d6fe2245390018/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/ListKeyHandler.java#L41-L42 ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/containerlog/parser/ContainerDatanodeDatabase.java: ########## @@ -233,5 +240,102 @@ private void dropTable(String tableName, Statement stmt) throws SQLException { stmt.executeUpdate(dropTableSQL); } + private void createContainerLogIndex(Statement stmt) throws SQLException { + String createIndexSQL = queries.get("CREATE_INDEX_LATEST_STATE"); + stmt.execute(createIndexSQL); + } + + /** + * Lists containers filtered by the specified state and writes their details to a file or console. + * <p> + * The output includes timestamp, datanode ID, container ID, BCSID, error message, and index value, + * written in a human-readable table format to a file or console. + * - If no file path is provided and no limit is specified, results are printed + * to the console with a default limit of 100 rows. + * - If no file path is provided but a limit is specified, results are printed + * to the console with the specified limit. + * - If a file path is provided but no limit is specified, all matching rows are written + * to the specified file. + * - If both a file path and a limit are provided, only the specified number of rows are written + * to the file. + * + * @param state the container state to filter by (e.g., "OPEN", "CLOSED", "QUASI_CLOSED") + * @param path the file path to write the output to. if {@code null}, output is printed to the console. + * @param limit the maximum number of rows to return. + * + */ + + public void listContainersByState(String state, String path, Integer limit) throws SQLException { + int count = 0; + + boolean writeToFile = path != null; + boolean limitProvided = limit != null; + + String baseQuery = queries.get("SELECT_LATEST_CONTAINER_LOGS_BY_STATE"); + String finalQuery = (!writeToFile && !limitProvided) ? baseQuery + " LIMIT " + DEFAULT_LIMIT + : (limitProvided ? baseQuery + " LIMIT ?" : baseQuery); + + Path outputPath = null; + if (writeToFile) { + outputPath = Paths.get(path); + if (Files.exists(outputPath)) { + System.out.println("Warning: Output file already exists and will be overwritten: " + + outputPath.toAbsolutePath()); + } + } + + try (Connection connection = getConnection(); + Statement stmt = connection.createStatement()) { + + createContainerLogIndex(stmt); + + try (PreparedStatement pstmt = connection.prepareStatement(finalQuery)) { + pstmt.setString(1, state); + if (limitProvided) { + pstmt.setInt(2, limit); + } + + try (ResultSet rs = pstmt.executeQuery(); + BufferedWriter writer = writeToFile + ? Files.newBufferedWriter(outputPath, StandardCharsets.UTF_8) + : new BufferedWriter(new OutputStreamWriter(System.out, StandardCharsets.UTF_8))) { + + writer.write(String.format("%-25s | %-35s | %-15s | %-15s | %-40s | %-12s%n", + "Timestamp", "Datanode ID", "Container ID", "BCSID", "Message", "Index Value")); + writer.write("-------------------------------------------------------------------------------------" + + "---------------------------------------------------------------------------------------\n"); + + while (rs.next()) { + String timestamp = rs.getString("timestamp"); + String datanodeId = rs.getString("datanode_id"); + long containerId = rs.getLong("container_id"); + long latestBcsid = rs.getLong("latest_bcsid"); + String errorMessage = rs.getString("error_message"); + int indexValue = rs.getInt("index_value"); + count++; + + writer.write(String.format("%-25s | %-35s | %-15d | %-15d | %-40s | %-12d%n", + timestamp, datanodeId, containerId, latestBcsid, errorMessage, indexValue)); + } + + if (count == 0) { + writer.write("No containers found for state: " + state + "\n"); + } else { + writer.write("total: " + count + "\n"); + writer.flush(); + if (writeToFile) { + System.out.println("Results written to file: " + outputPath.toAbsolutePath()); + } + } + } + } + } catch (SQLException e) { + LOG.error("Error while retrieving containers with state: {}", state, e); Review Comment: Write errors to stderr, not log. ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ListContainers.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.debug.container; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase; +import picocli.CommandLine; + + +/** + * List containers based on the parameter given. + */ + [email protected]( + name = "list-containers", + description = "Finds containers from the database based on the option provided." +) +public class ListContainers implements Callable<Void> { + /** + * Container states. + * Used to filter container records during database query. + */ + public enum ContainerState { + OPEN, CLOSED, QUASI_CLOSED, CLOSING, DELETED, UNHEALTHY + } + + @CommandLine.Option(names = {"--state", "-s"}, + description = "state of the container") + private ContainerState state; + + @CommandLine.Option(names = {"--limit", "-l"}, + description = "number of rows to display in the output , by default it would be 100 rows") + private Integer limit; + + @CommandLine.Option(names = {"--file-path", "-p"}, + description = "path to the output file") + private String path; + + @CommandLine.ParentCommand + private ContainerLogController parent; + + @Override + public Void call() throws Exception { + if (state != null) { + if (path != null) { + Path outputPath = Paths.get(path); + Path parentDir = outputPath.getParent(); + if (parentDir != null && !Files.exists(parentDir)) { + System.out.println("The directory does not exist: " + parentDir.toAbsolutePath()); + return null; + } + } + ContainerDatanodeDatabase cdd = new ContainerDatanodeDatabase(); + try { + cdd.listContainersByState(state.name(), path, limit); + } catch (SQLException e) { + System.out.println("Error while retrieving containers with state: " + state + e.getMessage()); + } catch (Exception e) { + System.out.println("Unexpected error while processing state: " + state + e.getMessage()); Review Comment: - Error/warning messages should go to stderr (`System.err`). - Add space between `state` and message. - Do we really need different messages for `SQLException` and `Exception`? ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/containerlog/parser/ContainerDatanodeDatabase.java: ########## @@ -233,5 +240,102 @@ private void dropTable(String tableName, Statement stmt) throws SQLException { stmt.executeUpdate(dropTableSQL); } + private void createContainerLogIndex(Statement stmt) throws SQLException { + String createIndexSQL = queries.get("CREATE_INDEX_LATEST_STATE"); + stmt.execute(createIndexSQL); + } + + /** + * Lists containers filtered by the specified state and writes their details to a file or console. + * <p> + * The output includes timestamp, datanode ID, container ID, BCSID, error message, and index value, + * written in a human-readable table format to a file or console. + * - If no file path is provided and no limit is specified, results are printed + * to the console with a default limit of 100 rows. + * - If no file path is provided but a limit is specified, results are printed + * to the console with the specified limit. + * - If a file path is provided but no limit is specified, all matching rows are written + * to the specified file. + * - If both a file path and a limit are provided, only the specified number of rows are written + * to the file. + * + * @param state the container state to filter by (e.g., "OPEN", "CLOSED", "QUASI_CLOSED") + * @param path the file path to write the output to. if {@code null}, output is printed to the console. + * @param limit the maximum number of rows to return. + * + */ + + public void listContainersByState(String state, String path, Integer limit) throws SQLException { Review Comment: Simplify this method by accepting a `PrintWriter` instead of a `path`. ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ListContainers.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.debug.container; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase; +import picocli.CommandLine; + + +/** + * List containers based on the parameter given. + */ + [email protected]( + name = "list-containers", + description = "Finds containers from the database based on the option provided." +) +public class ListContainers implements Callable<Void> { + /** + * Container states. + * Used to filter container records during database query. + */ + public enum ContainerState { + OPEN, CLOSED, QUASI_CLOSED, CLOSING, DELETED, UNHEALTHY + } + + @CommandLine.Option(names = {"--state", "-s"}, + description = "state of the container") + private ContainerState state; + + @CommandLine.Option(names = {"--limit", "-l"}, + description = "number of rows to display in the output , by default it would be 100 rows") + private Integer limit; + + @CommandLine.Option(names = {"--file-path", "-p"}, + description = "path to the output file") + private String path; Review Comment: How about printing to stdout and let user redirect it to file if needed? This would reduce options and simplify error handling. ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ListContainers.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.debug.container; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase; +import picocli.CommandLine; + + +/** + * List containers based on the parameter given. + */ + [email protected]( + name = "list-containers", Review Comment: `ozone debug container list-containers` seems redundant, how about `ozone debug container list`? ```suggestion name = "list", ``` ########## hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ListContainers.java: ########## @@ -0,0 +1,87 @@ +/* + * 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.debug.container; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.sql.SQLException; +import java.util.concurrent.Callable; +import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase; +import picocli.CommandLine; + + +/** + * List containers based on the parameter given. + */ + [email protected]( + name = "list-containers", + description = "Finds containers from the database based on the option provided." +) +public class ListContainers implements Callable<Void> { + /** + * Container states. + * Used to filter container records during database query. + */ + public enum ContainerState { + OPEN, CLOSED, QUASI_CLOSED, CLOSING, DELETED, UNHEALTHY + } Review Comment: Do we need a new enum? `ozone admin container list` uses `HddsProtos.LifeCycleState`. -- 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]
