HDDS-15889. Show failed MoveResult breakdown in ozone admin containerbalancer status -v - #10938
HDDS-15889. Show failed MoveResult breakdown in ozone admin containerbalancer status -v#10938sreejasahithi wants to merge 1 commit into
Conversation
…balancer status -v
|
@ashishkumar50 could you please review this PR. |
ashishkumar50
left a comment
There was a problem hiding this comment.
@sreejasahithi Thanks for working on this, Please update the patch wrt count instead of showing containerids. Here are the initial comments.
| optional int64 count = 2; | ||
| } | ||
|
|
||
| message ContainerMoveFailureDetailProto { |
There was a problem hiding this comment.
I think Failure breakdown and Failed move details should be merged. Also instead of showing containerids, we should show count of failures for source and target, otherwise it will be too verbose. Issue generally happens at DN level(like slow/unhealthy etc), so count is enough to know which dn is problematic.
| } | ||
|
|
||
| private String formatFailureBreakdown(List<ContainerMoveFailureSummaryProto> summaries) { | ||
| List<ContainerMoveFailureSummaryProto> sortedSummaries = summaries.stream() |
There was a problem hiding this comment.
The proto already sorts failure summaries, then the CLI sorts them again which is not required. Remove from server side sort.
| */ | ||
| public final class ContainerMoveFailureTracker { | ||
| public static final int DEFAULT_MAX_FAILURE_DETAILS = 100; | ||
| public static final int DEFAULT_MAX_FAILURE_DETAILS_PER_REASON = 10; |
There was a problem hiding this comment.
Please update as per dn count suggestion and remove these limits.
|
|
||
| private final int maxFailureDetails; | ||
| private final int maxFailureDetailsPerReason; | ||
| private final Map<String, Long> failuresByReason = new HashMap<>(); |
There was a problem hiding this comment.
Use enum FailureReason instead of plain string.
| failureDetails.add(new ContainerMoveFailureDetail( | ||
| containerId.getId(), | ||
| source.getUuidString(), | ||
| target.getUuidString(), |
There was a problem hiding this comment.
Source or target can be null.
What changes were proposed in this pull request?
Improves observability of Container Balancer move failures in
ozone admin containerbalancer status -v.Previously, verbose status only showed aggregate counts (“Failed to move containers”, “Failed to move containers by timeout”) with no breakdown of why moves failed, so users had to dig through SCM logs.
This change tracks per-iteration move failures on the SCM side and surfaces them in the CLI:
What is the link to the Apache JIRA
HDDS-15889
How was this patch tested?
Added test cases.
Tested in docker ozone-balancer cluster:
Green CI : https://github.com/sreejasahithi/ozone/actions/runs/30628368322