Tejaskriya commented on code in PR #8282:
URL: https://github.com/apache/ozone/pull/8282#discussion_r2065665124
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/logs/LogParser.java:
##########
@@ -15,24 +15,25 @@
* limitations under the License.
*/
-package org.apache.hadoop.ozone.debug.container;
+package org.apache.hadoop.ozone.debug.logs;
import org.apache.hadoop.hdds.cli.DebugSubcommand;
+import org.apache.hadoop.ozone.debug.logs.container.ContainerLogController;
import org.kohsuke.MetaInfServices;
import picocli.CommandLine;
/**
- * A controller for managing container log operations like parsing and listing
containers.
+ * Entry point for Ozone debug log parsing and analysis commands.
*/
@CommandLine.Command(
- name = "container",
+ name = "log",
subcommands = {
- ContainerLogParser.class
+ ContainerLogController.class
},
- description = "Parse, Store, Retrieve"
+ description = "This serves as a common entry point for all commands that
parse and analyze logs," +
+ "regardless of their source or type and require logs to be extracted
first."
Review Comment:
```suggestion
description = "Command to parse and analyze logs," +
"regardless of their source or type. It require logs to be extracted
first."
```
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/containerlog/parser/DBConsts.java:
##########
Review Comment:
It would be better to rename this to `SQLDBConstants` to avoid confusing it
with rocksDB constants.
Also, moving this to
`hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/utils` would make
more sense.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/logs/container/ContainerLogParser.java:
##########
@@ -15,34 +15,36 @@
* limitations under the License.
*/
-package org.apache.hadoop.ozone.debug.container;
+package org.apache.hadoop.ozone.debug.logs.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 org.apache.hadoop.ozone.containerlog.parser.ContainerLogFileParser;
+import org.apache.hadoop.ozone.containerlog.parser.DBConsts;
import picocli.CommandLine;
/**
* Parses container logs, processes them, and updates the database accordingly.
*/
@CommandLine.Command(
- name = "container_log_parse",
- description = "parse the container logs"
+ name = "parse",
+ description = "Parse container logs and extract key details such as
datanode ID, container ID, state, " +
+ "BCSID, timestamp, log level, index value, and messages (if any)."
)
public class ContainerLogParser implements Callable<Void> {
Review Comment:
Extending `AbstractSubcommand` and then replacing `System.out.println` or
`System.err.println` with out() and err() respectively will be cleaner
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/containerlog/parser/ContainerDatanodeDatabase.java:
##########
Review Comment:
If I understand correctly, all the classes under the `containerlog.parser`
are util methods used for the `debug.logs.container` classes. It doesn't make
sense to keep `containerlog.parser` in another package unrelated to
`debug.logs.container`. (As each package here seems to be corresponding to a
parent command).
I suggest moving it under the utils package in tools (like mentioned in a
below comment for `DBConsts`). Or we can also keep it under the
`debug.logs.container` by calling it a `utils` package instead of
`containerlog.parser`
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/logs/container/ListContainers.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.logs.container;
+
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.concurrent.Callable;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.containerlog.parser.ContainerDatanodeDatabase;
+import org.apache.hadoop.ozone.containerlog.parser.DBConsts;
+import org.apache.hadoop.ozone.shell.ListOptions;
+import picocli.CommandLine;
+
+
+/**
+ * List containers based on the parameter given.
+ */
+
[email protected](
+ name = "list",
+ description = "Finds containers from the database based on the option
provided."
+)
+public class ListContainers implements Callable<Void> {
Review Comment:
Similarly for this, extending the `AbstractSubcommand` will provide the
helper methods for printing any message to the console.
--
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]
