mon/PGMap: remove DIRTY field in `ceph df detail` when cache tiering is not in use #40337

ideepika · 2021-03-23T09:22:56Z

[ideepika@vossi03 build]$ bin/ceph df detail
*** DEVELOPER MODE: setting PATH, PYTHONPATH and LD_LIBRARY_PATH ***
2021-03-23T09:15:27.623+0000 7f5134720700 -1 WARNING: all dangerous and experimental features are enabled.
2021-03-23T09:15:27.644+0000 7f5134720700 -1 WARNING: all dangerous and experimental features are enabled.
--- RAW STORAGE ---
CLASS  SIZE  AVAIL  USED  RAW USED  %RAW USED
TOTAL   0 B    0 B   0 B       0 B          0

--- POOLS ---
POOL  ID  PGS  STORED  (DATA)  (OMAP)  OBJECTS  USED  (DATA)  (OMAP)  %USED  MAX AVAIL  QUOTA OBJECTS  QUOTA BYTES  USED COMPR  UNDER COMPR

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

src/mon/PGMap.cc

ideepika · 2021-06-07T22:29:49Z

jenkins test make check

src/mon/PGMap.cc

dismissed.

ideepika · 2021-06-22T14:56:57Z

@neha-ojha all tests passed, can you take a look, thanks!

neha-ojha

@ideepika The current version of your PR does not remove the DIRTY field entirely, it only displays the field, wherever it is relevant, but your commit messages still says "remove", can you please update them. Also, let's also avoid using "deprecate" in them, I don't think we have officially deprecated cache tiering.

doc/rados/operations/monitoring.rst

neha-ojha

we could add a is_tier() check for the formatted output too here https://github.com/ceph/ceph/blob/master/src/mon/PGMap.cc#L930

src/mon/PGMap.cc

ideepika · 2021-07-21T13:29:24Z

jenkins test api

neha-ojha · 2021-07-21T16:51:33Z

@ideepika the api test and make check failures seem unrelated, can you please verify and create tracker tickets if they aren't related, let me know when this PR is ready for a final review

ideepika · 2021-07-22T05:52:23Z

@neha-ojha think we need better error messages here, I checked if new PR's are failing after rebase, so tried testing using integer... worked! fixed and good for review, thanks :)

neha-ojha

@epuertat can you please verify the dashboard specific test change a0543ea?

epuertat · 2021-07-23T08:16:15Z

jenkins test dashboard

epuertat · 2021-07-23T08:16:59Z

@epuertat can you please verify the dashboard specific test change a0543ea?

I think it's ok, but I'm launching the Dashboard e2e tests to be fully sure.

epuertat · 2021-07-23T12:18:01Z

👍🏼

tchaikov · 2021-07-25T11:35:54Z

https://pulpito.ceph.com/kchai-2021-07-25_06:46:11-rados-wip-kefu-testing-2021-07-25-1126-distro-basic-smithi/

qa/tasks/mgr/dashboard/test_health.py

tchaikov · 2021-07-25T11:42:57Z

src/mon/PGMap.cc

+      if (pool->is_tier()) {
+        f->dump_int("dirty", sum.num_objects_dirty);
+      } else {
+	f->dump_int("dirty", 0);


would be better if we could be more consistent at using tab versus space for indent?

Time for a c++ linter?

time = when(any(hero.is_willing_to(create(a pre-commit hook) or create(a jenkins job using something like clang-format)) for hero in us))

Hehehe, I think that the main thing that divert us from doing this is the amount of work required to clean up everything... If we could find a way to only fail on the modified code, that'd probably be more encouraging... Do you have any clue on how to do that?

something like a script which

runs clang-format on all changed files

pipes the output to a filter which checks if the line number falls into the area defined by git diff

if there is any thing shows at the other end of the pipe, the author of the change should be alerted.

'ceph df detail' reports a column for DIRTY objects under POOLS even though cache tiers not being used. In replicated or EC pool all objects in the pool are reported as logically DIRTY as they have never been flushed . we display N/A for DIRTY objects if the pool is not a cache tier. Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

DIRTY field should only be available if cache tiering is in use. Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

dismissed

tchaikov · 2021-07-28T16:21:36Z

failures tracked by

https://tracker.ceph.com/issues/49633

github-actions bot added core documentation mon labels Mar 23, 2021

ideepika requested review from neha-ojha and jdurgin March 23, 2021 09:27

ideepika added the wip-deepika-testing label May 5, 2021

neha-ojha changed the title ~~mon/PGMap: remove DIRTY field in ceph df detail since cache tiering is depreciated~~ mon/PGMap: remove DIRTY field in ceph df detail since cache tiering is deprecated May 17, 2021

neha-ojha reviewed May 18, 2021

View reviewed changes

src/mon/PGMap.cc Outdated Show resolved Hide resolved

ideepika force-pushed the wip-bugzilla-1857447 branch from 2f779c3 to 7ce5fed Compare May 18, 2021 20:54

neha-ojha reviewed May 18, 2021

View reviewed changes

src/mon/PGMap.cc Outdated Show resolved Hide resolved

ideepika force-pushed the wip-bugzilla-1857447 branch 2 times, most recently from ff3d7fe to 284b431 Compare May 20, 2021 12:13

ideepika requested a review from neha-ojha May 20, 2021 12:14

ideepika removed the wip-deepika-testing label May 26, 2021

tchaikov previously requested changes Jun 8, 2021

View reviewed changes

src/mon/PGMap.cc Outdated Show resolved Hide resolved

ideepika force-pushed the wip-bugzilla-1857447 branch 2 times, most recently from 53d145c to 7cc7351 Compare June 8, 2021 07:41

github-actions bot added the tests label Jun 8, 2021

ideepika added the needs-review label Jun 22, 2021

neha-ojha reviewed Jun 22, 2021

View reviewed changes

doc/rados/operations/monitoring.rst Show resolved Hide resolved

doc/rados/operations/monitoring.rst Outdated Show resolved Hide resolved

neha-ojha removed the needs-review label Jun 22, 2021

ideepika changed the title ~~mon/PGMap: remove DIRTY field in ceph df detail since cache tiering is deprecated~~ mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not enabled Jun 23, 2021

ideepika changed the title ~~mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not enabled~~ mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use Jun 23, 2021

ideepika force-pushed the wip-bugzilla-1857447 branch 2 times, most recently from 6820a23 to ba4ada4 Compare June 23, 2021 05:16

neha-ojha requested changes Jun 23, 2021

View reviewed changes

src/mon/PGMap.cc Outdated Show resolved Hide resolved

ideepika force-pushed the wip-bugzilla-1857447 branch from ba4ada4 to 6beed3e Compare June 24, 2021 09:44

ideepika requested a review from aaSharma14 July 21, 2021 11:53

ideepika force-pushed the wip-bugzilla-1857447 branch from 1f8ce8f to 8377d61 Compare July 21, 2021 12:03

github-actions bot added the dashboard label Jul 21, 2021

ideepika force-pushed the wip-bugzilla-1857447 branch 2 times, most recently from 12fba9d to a0543ea Compare July 22, 2021 05:50

ideepika requested a review from neha-ojha July 22, 2021 18:44

neha-ojha approved these changes Jul 22, 2021

View reviewed changes

neha-ojha added the needs-qa label Jul 22, 2021

epuertat changed the title ~~mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use~~ mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use Jul 23, 2021

tchaikov added the wip-kefu-testing label Jul 24, 2021

tchaikov previously requested changes Jul 25, 2021

View reviewed changes

tchaikov removed the wip-kefu-testing label Jul 25, 2021

Deepika Upadhyay added 3 commits July 27, 2021 07:06

test: update test for when cache tier is not available

e80a80c

Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

doc/rados/op: ceph/df show DIRTY field N/A

ba52429

DIRTY field should only be available if cache tiering is in use. Signed-off-by: Deepika Upadhyay <dupadhya@redhat.com>

ideepika force-pushed the wip-bugzilla-1857447 branch from a0543ea to ba52429 Compare July 27, 2021 01:36

tchaikov added the wip-kefu-testing label Jul 27, 2021

tchaikov merged commit b148620 into ceph:master Jul 28, 2021

ideepika deleted the wip-bugzilla-1857447 branch July 28, 2021 17:25

This was referenced Aug 20, 2021

octopus: mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use #42862

Merged

pacific: mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use #42860

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mon/PGMap: remove DIRTY field in `ceph df detail` when cache tiering is not in use #40337

mon/PGMap: remove DIRTY field in `ceph df detail` when cache tiering is not in use #40337

ideepika commented Mar 23, 2021

ideepika commented Jun 7, 2021

ideepika commented Jun 22, 2021

neha-ojha left a comment

neha-ojha left a comment

ideepika commented Jul 21, 2021

neha-ojha commented Jul 21, 2021

ideepika commented Jul 22, 2021

neha-ojha left a comment

epuertat commented Jul 23, 2021

epuertat commented Jul 23, 2021

epuertat commented Jul 23, 2021

tchaikov commented Jul 25, 2021

tchaikov Jul 25, 2021

epuertat Jul 26, 2021

tchaikov Jul 28, 2021

epuertat Jul 28, 2021

tchaikov Jul 28, 2021 •

edited

tchaikov commented Jul 28, 2021

mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use #40337

mon/PGMap: remove DIRTY field in ceph df detail when cache tiering is not in use #40337

Conversation

ideepika commented Mar 23, 2021

Checklist

ideepika commented Jun 7, 2021

ideepika commented Jun 22, 2021

neha-ojha left a comment

Choose a reason for hiding this comment

neha-ojha left a comment

Choose a reason for hiding this comment

ideepika commented Jul 21, 2021

neha-ojha commented Jul 21, 2021

ideepika commented Jul 22, 2021

neha-ojha left a comment

Choose a reason for hiding this comment

epuertat commented Jul 23, 2021

epuertat commented Jul 23, 2021

epuertat commented Jul 23, 2021

tchaikov commented Jul 25, 2021

tchaikov Jul 25, 2021

Choose a reason for hiding this comment

epuertat Jul 26, 2021

Choose a reason for hiding this comment

tchaikov Jul 28, 2021

Choose a reason for hiding this comment

epuertat Jul 28, 2021

Choose a reason for hiding this comment

tchaikov Jul 28, 2021 • edited

Choose a reason for hiding this comment

tchaikov commented Jul 28, 2021

mon/PGMap: remove DIRTY field in `ceph df detail` when cache tiering is not in use #40337

mon/PGMap: remove DIRTY field in `ceph df detail` when cache tiering is not in use #40337

tchaikov Jul 28, 2021 •

edited