HDDS-11367. Improve ozone balancing status command output #7139

juncevich · 2024-08-30T13:52:50Z

What changes were proposed in this pull request?

Minor improvements to command output:

job task time consumption
iteration time consumption
unfinished iteration shouldn't be in iteration history section
show data units, even it less than gb show it in mb

What is the link to the Apache JIRA

HDDS-11367

How was this patch tested?

Tested by robot test

siddhantsangwan

@juncevich can you please elaborate on what these points mean?

job task time consumption
iteration time consumption

It will help reviewers understand what you're proposing.

juncevich · 2024-09-11T13:33:07Z

@juncevich can you please elaborate on what these points mean?

job task time consumption
iteration time consumption

It will help reviewers understand what you're proposing.

Hi, @siddhantsangwan. Sure, i can explain. Balancing job constists from some iterations. Job task time - the whole balancing time, iteration time - particular iteration time. Job time = iteration time + iteration time ...

...ds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStatusSubcommand.java

juncevich · 2024-09-23T19:31:09Z

Hi siddhantsangwan. Can you review my PR?

siddhantsangwan

@juncevich thanks for working on this. Other than my comments below, we definitely need more testing here. I'd like to at least see:

Unit tests for balancer that verify the iteration duration and other properties are correct.
Tests for the balancer status command that verify the code is correctly picking up the mocked values (mocked response from SCM).

We can also add verifications to the robot tests.

...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java

...ds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStatusSubcommand.java

siddhantsangwan · 2024-10-01T11:43:14Z

...ds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStatusSubcommand.java

            "Scheduled to move containers", containerMovesScheduled,
            "Already moved containers", containerMovesCompleted,
            "Failed to move containers", containerMovesFailed,
            "Failed to move containers by timeout", containerMovesTimeout,
            "Entered data to nodes", enteringDataNodeList,
            "Exited data from nodes", leavingDataNodeList);
  }
+
+  private String getPrettyIterationStatusInfo(long duration) {


There are probably existing utility methods to do this as well. See if you can find any? It's better to use them since they will be well tested.

I didn't find utility methods. I will extract this method to separate util class and write test for it.

...r-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java

...src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerStatusInfo.java

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java

.../org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java

...ds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStatusSubcommand.java

hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/util/DurationUtil.java

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java

...src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerStatusInfo.java

...r-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java

.../org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTaskIterationStatusInfo.java

# Conflicts: # hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot

siddhantsangwan · 2024-11-05T12:37:46Z

@juncevich please let me know when you've addressed comments and this is ready for another review.

juncevich · 2024-11-05T13:13:17Z

please let me know when you've addressed comments and this is ready for another review.

@siddhantsangwan I've not understood about comments, can you explain about? PR is ready for another review.

juncevich · 2024-11-11T07:09:09Z

@siddhantsangwan, can you take another look at this PR?

...test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancerStatusInfo.java

hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/util/TestDurationUtil.java

myskov · 2024-11-20T06:48:00Z

...cm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerMetrics.java

@@ -40,6 +40,9 @@ public final class ContainerBalancerMetrics {
      " in the latest iteration.")
  private MutableCounterLong dataSizeMovedGBInLatestIteration;


Do we need this metric now? Or is it for backward compatibility?

For backward compatibility.

myskov

few minor comments

...s/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestContainerBalancerSubCommand.java

...r-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java

myskov · 2024-11-22T11:38:32Z

it seems issues @siddhantsangwan mentioned have been addressed. @siddhantsangwan could you take look again, please?

…tatisticsRequestInPeriodBetweenIterations

siddhantsangwan · 2024-11-22T12:53:09Z

@juncevich if you've addressed all comments and fixed tests, I can start the CI workflow.

juncevich · 2024-11-22T12:56:03Z

@juncevich if you've addressed all comments and fixed tests, I can start the CI workflow.

Great, thanks! Just fixed the flaky test. Everything should be OK.

...r-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java

# Conflicts: # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java

juncevich · 2024-11-25T21:52:11Z

@siddhantsangwan I've fixed your review notices. Could you look through the changed code? I have to fix the robot test and PR will be ready.

juncevich · 2024-11-26T21:08:34Z

@siddhantsangwan robot test fixed. PR is ready for another review round.

HDDS-11367. Improve ozone balancing status command output

774fbbc

juncevich force-pushed the HDDS-11367 branch from f6f34c0 to 9f928d1 Compare September 1, 2024 18:29

ivandika3 requested a review from siddhantsangwan September 3, 2024 02:41

HDDS-11367. Improve ozone balancing status command output

00ad471

juncevich force-pushed the HDDS-11367 branch from 9f928d1 to 00ad471 Compare September 3, 2024 19:00

kerneltime requested review from sodonnel and kerneltime September 9, 2024 16:50

siddhantsangwan reviewed Sep 11, 2024

View reviewed changes

afilpp reviewed Sep 19, 2024

View reviewed changes

...ds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerBalancerStatusSubcommand.java Outdated Show resolved Hide resolved

HDDS-11367. Add space between Gb and Mb in getPrettySize

bac5794

juncevich requested review from siddhantsangwan and afilpp September 19, 2024 07:51

afilpp approved these changes Sep 19, 2024

View reviewed changes

siddhantsangwan requested changes Oct 1, 2024

View reviewed changes

juncevich added 4 commits October 3, 2024 11:01

HDDS-11367. Fixe review notices + add unit tests

9dbd39c

Merge branch 'master' into HDDS-11367

4f6af05

HDDS-11367. Small fixes + improve robot test

570025f

HDDS-11367. Add licences to files

0c04a2a

ivanzlenko suggested changes Oct 8, 2024

View reviewed changes

...src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerStatusInfo.java Outdated Show resolved Hide resolved

juncevich added 2 commits October 8, 2024 10:47

HDDS-11367. Fix review notice

f1f11ec

HDDS-11367. Improve balancer robot test

68766f9

juncevich requested review from ivanzlenko and siddhantsangwan October 8, 2024 08:11

ivanzlenko suggested changes Oct 10, 2024

View reviewed changes

HDDS-11367. Fix review notices

4138ad7

juncevich requested a review from ivanzlenko October 10, 2024 16:18

ivanzlenko suggested changes Oct 15, 2024

View reviewed changes

HDDS-11367. Fix review notices

4d916a5

juncevich added 8 commits November 1, 2024 16:09

HDDS-11367. Fix tests

c6562fc

HDDS-11367. Fix tests

361ba74

HDDS-11367. Fix tests

78caa97

Merge branch 'master' into HDDS-11367

6a68f45

# Conflicts: # hadoop-ozone/dist/src/main/smoketest/balancer/testBalancer.robot

HDDS-11367. Fix tests

bae2a6b

HDDS-11367. Fix tests

bea5d60

HDDS-11367. Fix tests

9ae7527

HDDS-11367. Add tests

9e8213b

juncevich requested a review from siddhantsangwan November 5, 2024 13:14

myskov reviewed Nov 20, 2024

View reviewed changes

HDDS-11367. Fix review comments.

786fc4f

myskov reviewed Nov 20, 2024

View reviewed changes

HDDS-11367. Fix review comments.

e77ea14

HDDS-11367. Fix flaky TestContainerBalancerStatusInfo.testGetCurrentS…

6ccd658

…tatisticsRequestInPeriodBetweenIterations

siddhantsangwan reviewed Nov 22, 2024

View reviewed changes

...r-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java Outdated Show resolved Hide resolved

...r-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java Show resolved Hide resolved

juncevich added 2 commits November 25, 2024 10:31

Merge branch 'master' into HDDS-11367

6acad2b

# Conflicts: # hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancerTask.java

HDDS-11367. Remove unstarted balancing iteration

15a8558

juncevich force-pushed the HDDS-11367 branch from 5d1932f to 567b94f Compare November 25, 2024 20:26

juncevich force-pushed the HDDS-11367 branch 2 times, most recently from dec8e06 to 4a40429 Compare November 26, 2024 15:14

HDDS-11367. Fix review notice. Refactor saving iteration statistic.

2e60bf3

juncevich force-pushed the HDDS-11367 branch from 4a40429 to 2e60bf3 Compare November 26, 2024 18:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-11367. Improve ozone balancing status command output #7139

HDDS-11367. Improve ozone balancing status command output #7139

juncevich commented Aug 30, 2024

siddhantsangwan left a comment

juncevich commented Sep 11, 2024

juncevich commented Sep 23, 2024

siddhantsangwan left a comment

siddhantsangwan Oct 1, 2024

juncevich Oct 2, 2024

siddhantsangwan commented Nov 5, 2024

juncevich commented Nov 5, 2024

juncevich commented Nov 11, 2024

myskov Nov 20, 2024

juncevich Nov 20, 2024

myskov left a comment

myskov commented Nov 22, 2024

siddhantsangwan commented Nov 22, 2024

juncevich commented Nov 22, 2024

juncevich commented Nov 25, 2024

juncevich commented Nov 26, 2024

		@@ -40,6 +40,9 @@ public final class ContainerBalancerMetrics {
		" in the latest iteration.")
		private MutableCounterLong dataSizeMovedGBInLatestIteration;

HDDS-11367. Improve ozone balancing status command output #7139

Are you sure you want to change the base?

HDDS-11367. Improve ozone balancing status command output #7139

Conversation

juncevich commented Aug 30, 2024

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

siddhantsangwan left a comment

Choose a reason for hiding this comment

juncevich commented Sep 11, 2024

juncevich commented Sep 23, 2024

siddhantsangwan left a comment

Choose a reason for hiding this comment

siddhantsangwan Oct 1, 2024

Choose a reason for hiding this comment

juncevich Oct 2, 2024

Choose a reason for hiding this comment

siddhantsangwan commented Nov 5, 2024

juncevich commented Nov 5, 2024

juncevich commented Nov 11, 2024

myskov Nov 20, 2024

Choose a reason for hiding this comment

juncevich Nov 20, 2024

Choose a reason for hiding this comment

myskov left a comment

Choose a reason for hiding this comment

myskov commented Nov 22, 2024

siddhantsangwan commented Nov 22, 2024

juncevich commented Nov 22, 2024

juncevich commented Nov 25, 2024

juncevich commented Nov 26, 2024