Update printer.go for used/not-used ETCD storage percentage #17871

Scotchman0 · 2024-04-24T16:29:36Z

based on OMC's output for etcdctl endpoint health reporting, add output that includes how much storage is used/not in use and a percentage for better visualization of data within the database. Help with analysis for defrag rates and visual output.

https://github.com/gmeghnag/omc/blob/e97cc10586cefbf0c626ef43b61597730f2b82cf/cmd/etcd/etcdctl.go#L35-L50\

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

k8s-ci-robot · 2024-04-24T16:29:46Z

Hi @Scotchman0. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Scotchman0 · 2024-04-24T16:32:00Z

Example output:

$ etcdctl endpoint status

+-----------------------------+------------------+---------+----------------+----------+-----------+------------+-----------+------------+--------------------+--------+
|          ENDPOINT           |        ID        | VERSION | DB SIZE/IN USE | NOT USED | IS LEADER | IS LEARNER | RAFT TERM | RAFT INDEX | RAFT APPLIED INDEX | ERRORS |
+-----------------------------+------------------+---------+----------------+----------+-----------+------------+-----------+------------+--------------------+--------+
| https://xxx.xxx.xxx:2379    | 1fc63d8d0eb5fa7b | 3.5.9   | 6.7 GB/5.6 GB  | 18%      | false     | false      |      5853 | 4073918326 |         4073918326 |        |
| https://xxx.xxx.xxx:2379    | 3bd2d35c028a6a2f | 3.5.9   | 6.8 GB/5.6 GB  | 19%      | true      | false      |      5853 | 4073918327 |         4073918327 |        |
| https://xxx.xxx.xxx:2379    | 41cbbc084d60007d | 3.5.9   | 6.8 GB/5.6 GB  | 18%      | false     | false      |      5853 | 4073918330 |         4073918330 |        |
+-----------------------------+------------------+---------+----------------+----------+-----------+------------+-----------+------------+--------------------+--------+

ivanvc · 2024-04-24T20:49:06Z

Hi @Scotchman0, thanks for your contribution. While we await a maintainer to review this suggestion, would you mind signing your commit? So the developer certificate of origin (DCO) check passes, i.e:

git rebase HEAD~1 --signoff
git push --force

Ref: https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#commit-your-change

based on OMC's output for etcdctl endpoint health reporting, add output that includes how much storage is used/not in use and a percentage for better visualization of data within the database. Help with analysis for defrag rates and visual output. Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 · 2024-04-25T22:24:50Z

Latest update:

+-----------------------------+------------------+---------+----------------+----------+-----------+------------+-----------+------------+--------------------+--------+
|          ENDPOINT           |        ID        | VERSION | DB SIZE | IN USE | NOT USED | QUOTA | IS LEADER | IS LEARNER | RAFT TERM | RAFT INDEX | RAFT APPLIED INDEX | ERRORS |
+-----------------------------+------------------+---------+----------------+----------+-----------+------------+-----------+------------+--------------------+--------+
| https://xxx.xxx.xxx:2379    | 1fc63d8d0eb5fa7b | 3.5.9   | 6.7 GB | 5.6 GB  | 18%      | ??    | false     | false      |      5853 | 4073918326 |         4073918326 |        |
| https://xxx.xxx.xxx:2379    | 3bd2d35c028a6a2f | 3.5.9   | 6.8 GB | 5.6 GB  | 19%      | ??    | true      | false      |      5853 | 4073918327 |         4073918327 |        |
| https://xxx.xxx.xxx:2379    | 41cbbc084d60007d | 3.5.9   | 6.8 GB | 5.6 GB  | 18%      | ??    | false     | false      |      5853 | 4073918330 |         4073918330 |        |
+-----------------------------+------------------+---------+----------------+----------+-----------+------------+-----------+------------+--------------------+---------

…dded possible/pending Quota field Signed-off-by: Will Russell <will.russell.01@gmail.com>

siyuanfoundation · 2024-05-02T21:08:02Z

etcdctl/ctlv3/command/printer.go

@@ -211,7 +211,7 @@ func makeEndpointHealthTable(healthList []epHealth) (hdr []string, rows [][]stri
 }

 func makeEndpointStatusTable(statusList []epStatus) (hdr []string, rows [][]string) {
-	hdr = []string{"endpoint", "ID", "version", "storage version", "db size", "db size in use", "is leader", "is learner", "raft term",
+	hdr = []string{"endpoint", "ID", "version", "storage version", "db size", "in use", "not used", "quota", "is leader", "is learner", "raft term",


nit: "not in use"

changed - agreed that's clearer

siyuanfoundation · 2024-05-02T21:08:30Z

/ok-to-test

siyuanfoundation · 2024-05-02T21:10:18Z

etcdctl/ctlv3/command/printer.go

@@ -221,6 +221,8 @@ func makeEndpointStatusTable(statusList []epStatus) (hdr []string, rows [][]stri
 			status.Resp.StorageVersion,
 			humanize.Bytes(uint64(status.Resp.DbSize)),
 			humanize.Bytes(uint64(status.Resp.DbSizeInUse)),
+			fmt.Sprint(100-(status.Resp.DbSizeInUse*100/status.Resp.DBSize)) + "%",


it is better to keep this number format consistent with in use one. They can be both byte size, or percentage.

ack - would the following format handling also succeed here? - pushed to check it but I can also revisit how I'm calculating the percentage with more of a robust option if it doesn't like this:

humanize.Bytes(uint64(100-(status.Resp.DBsizeInUse*100/status.Resp.DBSize))) + "%",

looks like no. 🤔 I'll explore some alternatives

…d changed percent check to humanize out instead of sprint Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 · 2024-05-02T21:58:34Z

etcdctl/ctlv3/command/printer.go

@@ -221,6 +221,8 @@ func makeEndpointStatusTable(statusList []epStatus) (hdr []string, rows [][]stri
 			status.Resp.StorageVersion,
 			humanize.Bytes(uint64(status.Resp.DbSize)),
 			humanize.Bytes(uint64(status.Resp.DbSizeInUse)),
+			fmt.Sprint(100-(status.Resp.DbSizeInUse*100/status.Resp.DBSize)) + "%",
+			humanize.Bytes(uint64(status.Resp.DbSizeQuota)),


removed the quota reference for the time being to see about clearing the checks but will want to add it in before merge pending the other PR that will be setting this value - will revisit.

can you run the build and test locally first?

Scotchman0 · 2024-05-02T21:59:31Z

/retest-required

(oops wrong button)

…tion is problematic Signed-off-by: Will Russell <will.russell.01@gmail.com>

Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 · 2024-05-03T15:41:19Z

validated local build; removed additional percent symbol; confirmed output works the way I want it to (though I don't have a build running the version with the quota to check, I'll confirm that separately) latest should be where it needs to be pending another review.

sh-4.4# ./etcdctl endpoint status -w table
+----------------------------+------------------+---------+-----------------+---------+--------+------------+-------+-----------+------------+-----------+------------+--------------------+--------+
|          ENDPOINT          |        ID        | VERSION | STORAGE VERSION | DB SIZE | IN USE | NOT IN USE | QUOTA | IS LEADER | IS LEARNER | RAFT TERM | RAFT INDEX | RAFT APPLIED INDEX | ERRORS |
+----------------------------+------------------+---------+-----------------+---------+--------+------------+-------+-----------+------------+-----------+------------+--------------------+--------+
| https://xxx.xxx.xxx:2379 | 4e7c8905a7df1da4 |  3.5.12 |                   |  113 MB |  73 MB |        36% |   0 B |      true |      false |         9 |    7637412 |            7637412 |        |
| https://xxx.xxx.xxx:2379 | b58db7287ba07b76 |  3.5.12 |                   |  112 MB |  73 MB |        35% |   0 B |     false |      false |         9 |    7637412 |            7637412 |        |
| https://xxx.xxx.xxx:2379 | acfb38ff541a7d54 |  3.5.12 |                   |  115 MB |  73 MB |        37% |   0 B |     false |      false |         9 |    7637412 |            7637412 |        |
+----------------------------+------------------+---------+-----------------+---------+--------+------------+-------+-----------+------------+-----------+------------+--------------------+--------+

siyuanfoundation · 2024-05-07T20:46:25Z

Is it important to have NOT IN USE in percentage? If it is not critical, it is better to keep it in MB. Otherwise, can you change the name to PERCENTAGE NOT IN USE?

Scotchman0 · 2024-05-07T22:05:41Z

Is it important to have NOT IN USE in percentage? If it is not critical, it is better to keep it in MB. Otherwise, can you change the name to PERCENTAGE NOT IN USE?

I'd be inclined to change the name; The goal is to add an easy marker for identification of unused disk space which might help to illustrate why the cluster defrags often or whether or not a defrag is expected / could benefit performance. Mostly this is for easy visibility into this logic: https://github.com/openshift/cluster-etcd-operator/blob/cbfb856ec8892687a303989b84e01c8f34c1967e/pkg/operator/defragcontroller/defragcontroller.go#L181-L224
And where the max value is set: https://github.com/openshift/cluster-etcd-operator/blob/cbfb856ec8892687a303989b84e01c8f34c1967e/pkg/operator/defragcontroller/defragcontroller.go#L31C2-L31C45 (45%).

To be consistent, I think percentage handling is preferable. I've changed the name to PERCENTAGE NOT IN USE as requested and pushed a new build.

Thanks!

Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 · 2024-05-07T22:12:52Z

oof - I accidentally broke stuff while forcing DCO 🤦 1s I'll revert.

**fixed - not sure how I pulled 64 commits in but latest should be correct 😅

k8s-ci-robot added the needs-ok-to-test label Apr 24, 2024

Scotchman0 mentioned this pull request Apr 24, 2024

Add better verbosity to the etcdctl endpoint status output that includes percentage of DB used (to illustrate Defrag rate) #17872

Open

Scotchman0 force-pushed the patch-1 branch from 55dfc8c to 0b86e13 Compare April 24, 2024 21:34

modified the output to split each data field into separate column + a…

740da09

…dded possible/pending Quota field Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 force-pushed the patch-1 branch from bb9b3c5 to 740da09 Compare April 25, 2024 22:45

tjungblu mentioned this pull request Apr 30, 2024

add quota to endpoint status response #17877

Merged

siyuanfoundation reviewed May 2, 2024

View reviewed changes

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels May 2, 2024

siyuanfoundation reviewed May 2, 2024

View reviewed changes

removed quota insert for time being, modified verbiage to not used an…

6d4d7fb

…d changed percent check to humanize out instead of sprint Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 commented May 2, 2024

View reviewed changes

Scotchman0 closed this May 2, 2024

Scotchman0 reopened this May 2, 2024

Scotchman0 and others added 5 commits May 2, 2024 18:19

testing with fmt.sprintf handling of float instead, humanize substitu…

59fde43

…tion is problematic Signed-off-by: Will Russell <will.russell.01@gmail.com>

removed 'quota' header until that is available

29b9e25

Signed-off-by: Will Russell <will.russell.01@gmail.com>

Merge branch 'etcd-io:main' into patch-1

a8e6eac

added Quota block, now that PR 17877 is merged into main

0e57bf2

Signed-off-by: Will Russell <will.russell.01@gmail.com>

removed superfluous additional percent symbol

2975a6b

Signed-off-by: Will Russell <will.russell.01@gmail.com>

modified label from not in use to percentage not in use

917078f

Signed-off-by: Will Russell <will.russell.01@gmail.com>

Scotchman0 force-pushed the patch-1 branch from 8f6875f to 34a31bb Compare May 7, 2024 22:11

Scotchman0 force-pushed the patch-1 branch from 34a31bb to 917078f Compare May 7, 2024 22:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update printer.go for used/not-used ETCD storage percentage #17871

Update printer.go for used/not-used ETCD storage percentage #17871

Scotchman0 commented Apr 24, 2024

k8s-ci-robot commented Apr 24, 2024

Scotchman0 commented Apr 24, 2024 •

edited by serathius

ivanvc commented Apr 24, 2024

Scotchman0 commented Apr 25, 2024

siyuanfoundation May 2, 2024

Scotchman0 May 2, 2024

siyuanfoundation commented May 2, 2024

siyuanfoundation May 2, 2024

Scotchman0 May 2, 2024

Scotchman0 May 2, 2024

Scotchman0 May 2, 2024 •

edited

siyuanfoundation May 2, 2024

Scotchman0 commented May 2, 2024 •

edited

Scotchman0 commented May 3, 2024

siyuanfoundation commented May 7, 2024

Scotchman0 commented May 7, 2024

Scotchman0 commented May 7, 2024 •

edited

Update printer.go for used/not-used ETCD storage percentage #17871

Are you sure you want to change the base?

Update printer.go for used/not-used ETCD storage percentage #17871

Conversation

Scotchman0 commented Apr 24, 2024

k8s-ci-robot commented Apr 24, 2024

Scotchman0 commented Apr 24, 2024 • edited by serathius

ivanvc commented Apr 24, 2024

Scotchman0 commented Apr 25, 2024

siyuanfoundation May 2, 2024

Choose a reason for hiding this comment

Scotchman0 May 2, 2024

Choose a reason for hiding this comment

siyuanfoundation commented May 2, 2024

siyuanfoundation May 2, 2024

Choose a reason for hiding this comment

Scotchman0 May 2, 2024

Choose a reason for hiding this comment

Scotchman0 May 2, 2024

Choose a reason for hiding this comment

Scotchman0 May 2, 2024 • edited

Choose a reason for hiding this comment

siyuanfoundation May 2, 2024

Choose a reason for hiding this comment

Scotchman0 commented May 2, 2024 • edited

Scotchman0 commented May 3, 2024

siyuanfoundation commented May 7, 2024

Scotchman0 commented May 7, 2024

Scotchman0 commented May 7, 2024 • edited

Scotchman0 commented Apr 24, 2024 •

edited by serathius

Scotchman0 May 2, 2024 •

edited

Scotchman0 commented May 2, 2024 •

edited

Scotchman0 commented May 7, 2024 •

edited