Skip to content

HDDS-12653. CLI command to list containers based on Health State - #8415

Merged
adoroszlai merged 7 commits into
apache:masterfrom
sreejasahithi:HDDS-12653
May 12, 2025
Merged

HDDS-12653. CLI command to list containers based on Health State#8415
adoroszlai merged 7 commits into
apache:masterfrom
sreejasahithi:HDDS-12653

Conversation

@sreejasahithi

@sreejasahithi sreejasahithi commented May 8, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This ozone debug CLI command lists all the containers which are either UNDER-REPLICATED , OVER-REPLICATED , UNHEALTHY or QUASI_CLOSED stuck.

the command provides the Container ID and count of replicas.

The command options used are:

List containers by health type:
ozone debug log container --db=<path to db> list --health=<type>

List containers by health type with a specified row limit:
ozone debug log container --db=<path to db> list --health=<type> --length=<limit>

List all containers by health type, overriding the row limit:
ozone debug log container --db=<path to db> list --health=<type> --all

Sample output for UNHEALTHY :

Container ID = 6002 - Count = 1
Container ID = 6201 - Count = 1
Container ID = 136662 - Count = 3
Container ID = 136837 - Count = 3
Container ID = 199954 - Count = 3
Container ID = 237747 - Count = 3
Container ID = 2579099 - Count = 1
Container ID = 2626888 - Count = 1
Container ID = 2627711 - Count = 1
Container ID = 2627751 - Count = 2

Sample output for UNDER-REPLICATED:

Container ID = 361 - Count = 2
Container ID = 362 - Count = 2
Container ID = 363 - Count = 2
Container ID = 364 - Count = 2
Container ID = 368 - Count = 2
Container ID = 372 - Count = 2
Container ID = 376 - Count = 2
Container ID = 380 - Count = 2
Container ID = 386 - Count = 2
Container ID = 409 - Count = 2

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-12653

How was this patch tested?

Below is the workflow link that passed successfully:
https://github.com/sreejasahithi/ozone/actions/runs/14909363332

@sreejasahithi
sreejasahithi marked this pull request as ready for review May 8, 2025 16:19
@adoroszlai adoroszlai added the tools Tools that helps with debugging label May 8, 2025
aryangupta1998
aryangupta1998 reviewed May 8, 2025
View reviewed changes

@aryangupta1998 aryangupta1998 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @sreejasahithi, do you feel we should include enum for container states in SQLDBConstants?
public enum ContainerState { OPEN, CLOSED, DELETED, UNHEALTHY, QUASI_CLOSED }

adoroszlai
adoroszlai reviewed May 9, 2025
View reviewed changes
Comment thread ...tools/src/main/java/org/apache/hadoop/ozone/debug/logs/container/ListContainersByHealth.java Outdated
Comment on lines +38 to +41
@CommandLine.Option(names = {"--issue"},
required = true,
description = "Health state of the container.")
private ReplicationManagerReport.HealthState healthState;

@adoroszlai adoroszlai May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue is an unusual name for "health state"

Comment thread ...tools/src/main/java/org/apache/hadoop/ozone/debug/logs/container/ListContainersByHealth.java Outdated
Comment on lines +43 to +44
@CommandLine.Mixin
private ListOptions listOptions;

@adoroszlai adoroszlai May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ListOptions has --start and --prefix options that are not supported (used) by this command. #8413 splits ListOptions, providing another mixin for commands that only want to use --length/--all. It also changes ListContainers to use the new ListLimitOptions.

Depending on which PR gets merged first, the other one will need to be updated.

@sreejasahithi sreejasahithi May 9, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay , thanks for letting me know about this.

Comment thread ...tools/src/main/java/org/apache/hadoop/ozone/debug/logs/container/ListContainersByHealth.java Outdated
Comment on lines +32 to +36
name = "list-health",
description = "Lists containers based on health state like UNDER_REPLICATED, " +
"OVER_REPLICATED , UNHEALTHY, QUASI_CLOSED stuck."
)
public class ListContainersByHealth extends AbstractSubcommand implements Callable<Void> {

@adoroszlai adoroszlai May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the existing ListContainers class should be updated to support filtering by HealthState, too, instead of introducing a new command.

ozone debug log container --db=<path> list --lifecycle ...
ozone debug log container --db=<path> list --health ...

adoroszlai
adoroszlai reviewed May 9, 2025
View reviewed changes
Comment thread ...tools/src/main/java/org/apache/hadoop/ozone/debug/logs/container/ListContainersByHealth.java Outdated
Comment on lines +52 to +55
Path dbPath = parent.resolveDbPath();
if (dbPath == null) {
return null;
}

@adoroszlai adoroszlai May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is duplicated in subcommands. ContainerLogController should throw IllegalArgumentException if it cannot resolve DB path from any sources (option or property).

adoroszlai
adoroszlai reviewed May 9, 2025
View reviewed changes
Comment thread .../main/java/org/apache/hadoop/ozone/debug/logs/container/utils/ContainerDatanodeDatabase.java Outdated

while (rs.next()) {
if (limitProvided && count >= limit) {
out.println("Note: There might be more containers. Use -all option to list all entries.");

@adoroszlai adoroszlai May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no -all option, it's --all. Also, information like this should be printed to stderr.

@sreejasahithi sreejasahithi May 9, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message just tells the user that there might be more containers so if they want to check they can use --all option. Hence it is not exactly and error.

@adoroszlai adoroszlai May 9, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for using stderr is to avoid mixing actual content and informational messages. This is especially important when using JSON or other machine-readable output format.

adoroszlai
adoroszlai reviewed May 12, 2025
View reviewed changes

@adoroszlai adoroszlai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @sreejasahithi for updating the patch, LGTM.

aryangupta1998
aryangupta1998 approved these changes May 12, 2025
View reviewed changes

@aryangupta1998 aryangupta1998 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @sreejasahithi, LGTM!

@adoroszlai
adoroszlai merged commit d92f0ed into apache:master May 12, 2025
@adoroszlai

adoroszlai commented May 12, 2025

Copy link
Copy Markdown
Contributor

Thanks @sreejasahithi for the patch, @aryangupta1998 for the review.

@swamirishi

swamirishi commented May 12, 2025

Copy link
Copy Markdown
Contributor

@adoroszlai CI on master is broken. This patch seems to be the root cause. Should we revert this patch?

@adoroszlai

adoroszlai commented May 13, 2025

Copy link
Copy Markdown
Contributor

@swamirishi no, this is not the root cause, it's just a coincidence. The same failures are observed even on feature branch HDDS-10685, which does not have this commit.

https://github.com/apache/ozone/actions/runs/14978935639

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adoroszlai adoroszlai adoroszlai left review comments
@aryangupta1998 aryangupta1998 aryangupta1998 approved these changes

Assignees

No one assigned

Labels

tools Tools that helps with debugging

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sreejasahithi @adoroszlai @swamirishi @aryangupta1998