osd/scrub: Add stats to PG dump for number of objects scrubbed #42735

amathuria · 2021-08-10T14:23:40Z

Addition of a new column in PG dump, OBJECTS_SCRUBBED, which keeps track of the number of objects scrubbed.

Signed-off-by: Aishwarya Mathuria amathuri@redhat.com

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/osd/osd_types.cc

jdurgin · 2021-08-17T15:20:45Z

can you add a test for this? perhaps the existing scrub tests could check that the field is populated?

ronen-fr · 2021-08-18T07:32:33Z

src/osd/pg_scrubber.cc

@@ -1279,6 +1279,8 @@ void PgScrubber::scrub_compare_maps()
 {
  dout(10) << __func__ << " has maps, analyzing" << dendl;

+  int64_t primary_objects = 0;


not needed (esp. so far from the usage point)

ronen-fr · 2021-08-18T07:32:50Z

src/osd/pg_scrubber.cc

@@ -1319,6 +1321,8 @@ void PgScrubber::scrub_compare_maps()

    dout(2) << __func__ << ": primary (" << m_pg->get_primary() << ") has "
 	    << m_primary_scrubmap.objects.size() << " items" << dendl;
+    primary_objects = m_primary_scrubmap.objects.size();


int primary_objects = ...

src/osd/PG.h

neha-ojha · 2021-08-25T17:39:45Z

jenkins test api

github-actions · 2021-09-26T12:43:49Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

stale · 2022-01-09T06:27:13Z

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

Addition of a new column in PG dump, OBJECTS_SCRUBBED, which keeps track of the number of objects scrubbed. Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

…er of objects in a PG once a scrub finishes Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

ronen-fr

LGTM apart from minor comments.
'primary_objects' fix is the one I would consider a must.

ronen-fr · 2022-01-12T17:59:53Z

qa/standalone/scrub/osd-scrub-test.sh

+
+    TESTDATA="testdata.$$"
+
+    setup $dir || return 1


note that standard_scrub_cluster() in qa/standalone/scrub/scrub_helpers.sh might save you from some of this boilerplate startup code

ronen-fr · 2022-01-12T18:03:33Z

src/osd/scrubber/pg_scrubber.cc

@@ -1370,6 +1370,8 @@ void PgScrubber::scrub_compare_maps()
 {
  dout(10) << __func__ << " has maps, analyzing" << dendl;

+  int64_t primary_objects = 0;


why not just declare primary_objects where it's used? (if at all)

ronen-fr · 2022-01-12T18:05:58Z

src/osd/scrubber/pg_scrubber.cc

@@ -1410,6 +1412,8 @@ void PgScrubber::scrub_compare_maps()

    dout(2) << __func__ << ": primary (" << m_pg->get_primary() << ") has "
 	    << m_primary_scrubmap.objects.size() << " items" << dendl;
+    primary_objects = m_primary_scrubmap.objects.size();


I would just skip 'primary_objects' altogether (the only good reason to have it would
have been to use it in the dout() in the previous line)

Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

amathuria · 2022-01-14T06:34:47Z

The pg dump output will now look like this:
Before scrub:

PG_STAT  OBJECTS  MISSING_ON_PRIMARY  DEGRADED  MISPLACED  UNFOUND  BYTES     OMAP_BYTES*  OMAP_KEYS*  LOG  DISK_LOG  STATE         STATE_STAMP                      VERSION  REPORTED  UP       UP_PRIMARY  ACTING   ACTING_PRIMARY  LAST_SCRUB  SCRUB_STAMP                      LAST_DEEP_SCRUB  DEEP_SCRUB_STAMP                 SNAPTRIMQ_LEN  LAST_SCRUB_DURATION  SCRUB_SCHEDULING  OBJECTS_SCRUBBED
1.63          10                   0         0          0        0  41943040            0           0   10        10  active+clean  2022-01-13T18:03:34.044849+0530    18'10     18:21  [1,0,2]           1  [1,0,2]               1         0'0  2022-01-13T18:03:32.887271+0530              0'0  2022-01-13T18:03:32.887271+0530              0                    0  queued for scrub                 0
1.62           3                   0         0          0        0  12582912            0           0    3         3  active+clean  2022-01-13T18:03:34.023840+0530     18'3     18:14  [2,0,1]           2  [2,0,1]               2         0'0  2022-01-13T18:03:32.887271+0530              0'0  2022-01-13T18:03:32.887271+0530              0                    0  queued for scrub                 0
1.61           5                   0         0          0        0  20971520            0           0    5         5  active+clean  2022-01-13T18:03:34.066341+0530     18'5     18:16  [0,1,2]           0  [0,1,2]               0         0'0  2022-01-13T18:03:32.887271+0530              0'0  2022-01-13T18:03:32.887271+0530              0                    0  queued for scrub                 0

After scrub:

PG_STAT  OBJECTS  MISSING_ON_PRIMARY  DEGRADED  MISPLACED  UNFOUND  BYTES     OMAP_BYTES*  OMAP_KEYS*  LOG  DISK_LOG  STATE         STATE_STAMP                      VERSION  REPORTED  UP       UP_PRIMARY  ACTING   ACTING_PRIMARY  LAST_SCRUB  SCRUB_STAMP                      LAST_DEEP_SCRUB  DEEP_SCRUB_STAMP                 SNAPTRIMQ_LEN  LAST_SCRUB_DURATION  SCRUB_SCHEDULING  OBJECTS_SCRUBBED
1.63          10                   0         0          0        0  41943040            0           0   10        10  active+clean  2022-01-13T18:06:12.230989+0530    18'10     18:30  [1,0,2]           1  [1,0,2]               1       18'10  2022-01-13T18:06:12.230810+0530              0'0  2022-01-13T18:03:32.887271+0530              0                    1  scrubbing for 0s                10
1.62           3                   0         0          0        0  12582912            0           0    3         3  active+clean  2022-01-13T18:05:53.489342+0530     18'3     18:23  [2,0,1]           2  [2,0,1]               2        18'3  2022-01-13T18:05:53.489059+0530              0'0  2022-01-13T18:03:32.887271+0530              0                    1  scrubbing for 0s                 3
1.61           5                   0         0          0        0  20971520            0           0    5         5  active+clean  2022-01-13T18:06:24.108837+0530     18'5     18:25  [0,1,2]           0  [0,1,2]               0        18'5  2022-01-13T18:06:24.108653+0530              0'0  2022-01-13T18:03:32.887271+0530              0                    1  scrubbing for 0s                 5

ronen-fr · 2022-01-14T10:09:59Z

the "scrubbing for 0 seconds" in the final listing is strange. Not sure if caused by this PR or not, but if so - seems an additional stat update is needed

amathuria · 2022-01-14T11:45:43Z

the "scrubbing for 0 seconds" in the final listing is strange. Not sure if caused by this PR or not, but if so - seems an additional stat update is needed

@ronen-fr I don't think it is caused by this PR but I think it's happening in cases where the scrub duration is lesser than 1 second. https://github.com/ceph/ceph/blob/master/src/osd/scrubber/pg_scrubber.cc#L2031 we are trying to get the difference in seconds and that might be returning 0 in such a scenario.
LAST_SCRUB_DURATION looks okay because we do a ceill here : https://github.com/ceph/ceph/blob/master/src/osd/scrubber/pg_scrubber.cc#L2138

Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

ronen-fr · 2022-01-14T14:48:45Z

My problem is with the "scrubbing for" persisting after scrubbing is done. It should have read "scheduled ...". That's why I fear we're missing an update.

amathuria · 2022-01-14T15:01:32Z

My problem is with the "scrubbing for" persisting after scrubbing is done. It should have read "scheduled ...". That's why I fear we're missing an update.
Ah I see what you mean. Yes, I agree with that.

amathuria · 2022-01-14T15:32:25Z

http://pulpito.front.sepia.ceph.com/yuriw-2022-01-13_18:06:52-rados-wip-yuri3-testing-2022-01-13-0809-distro-default-smithi/

Failures unrelated, tracked in:
https://tracker.ceph.com/issues/44587
https://tracker.ceph.com/issues/53843
https://tracker.ceph.com/issues/43887
https://tracker.ceph.com/issues/53807
https://tracker.ceph.com/issues/53886
https://tracker.ceph.com/issues/53424
https://tracker.ceph.com/issues/50830

Details:
Bug #44587: failed to write 34473 to cgroup.procs
Bug #53843: mgr/dashboard: Error - yargs parser supports a minimum Node.js version of 12
Bug #43887: ceph_test_rados_delete_pools_parallel failure
Bug #53807: Dead jobs in rados/cephadm/smoke-roleless
Bug #53886: ansible: Failed to update apt cache
Bug #53424: CEPHADM_DAEMON_PLACE_FAIL in orch:cephadm/mgr-nfs-upgrade/
Bug #50830: rgw-ingress does not install

yuriw · 2022-01-14T16:43:00Z

jenkins test api

ronen-fr · 2022-01-14T16:52:47Z

jenkins test make check

neha-ojha · 2022-01-14T17:17:08Z

jenkins test api

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

A refactoring of the scrub backend code (all Scrub related code that checks/manipulates objects' (meta)data). Part of the refactoring was breaking long chunks of code into separate functions. In order to avoid passing endless lists of state parameters from/to these new methods, the scrub backend state is now maintained within its own objects. a change to note: Instead of two scrub-map collections - one that holds all incoming maps, and one that holds pointers to all of those incoming plus the one we (the Primary) manage - just use one collection for both incoming and our own. Signed-off-by: Ronen Friedman <rfriedma@redhat.com> Split from "osd/scrub: minor fixes split from main "scrub backend" commit" Split from "osd/scrub: scrub components embedded documentation" fix Signed-off-by: Ronen Friedman <rfriedma@redhat.com> osd/scrub: fix the scrubber backend to include all PR ceph#42735 Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

to publish the last scrub status report. The change is needed following the merge of PR ceph#42735. Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

to publish the last scrub status report. The change is needed following the merge of PR #42735. Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

A refactoring of the scrub backend code (all Scrub related code that checks/manipulates objects' (meta)data). Part of the refactoring was breaking long chunks of code into separate functions. In order to avoid passing endless lists of state parameters from/to these new methods, the scrub backend state is now maintained within its own objects. a change to note: Instead of two scrub-map collections - one that holds all incoming maps, and one that holds pointers to all of those incoming plus the one we (the Primary) manage - just use one collection for both incoming and our own. Signed-off-by: Ronen Friedman <rfriedma@redhat.com> Split from "osd/scrub: minor fixes split from main "scrub backend" commit" Split from "osd/scrub: scrub components embedded documentation" fix Signed-off-by: Ronen Friedman <rfriedma@redhat.com> osd/scrub: fix the scrubber backend to include all PR #42735 Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

to publish the last scrub status report. The change is needed following the merge of PR ceph#42735. Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

A refactoring of the scrub backend code (all Scrub related code that checks/manipulates objects' (meta)data). Part of the refactoring was breaking long chunks of code into separate functions. In order to avoid passing endless lists of state parameters from/to these new methods, the scrub backend state is now maintained within its own objects. a change to note: Instead of two scrub-map collections - one that holds all incoming maps, and one that holds pointers to all of those incoming plus the one we (the Primary) manage - just use one collection for both incoming and our own. Signed-off-by: Ronen Friedman <rfriedma@redhat.com> Split from "osd/scrub: minor fixes split from main "scrub backend" commit" Split from "osd/scrub: scrub components embedded documentation" fix Signed-off-by: Ronen Friedman <rfriedma@redhat.com> osd/scrub: fix the scrubber backend to include all PR ceph#42735 Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

to publish the last scrub status report. The change is needed following the merge of PR ceph#42735. Signed-off-by: Ronen Friedman <rfriedma@redhat.com> (cherry picked from commit ab032e9)

A refactoring of the scrub backend code (all Scrub related code that checks/manipulates objects' (meta)data). Part of the refactoring was breaking long chunks of code into separate functions. In order to avoid passing endless lists of state parameters from/to these new methods, the scrub backend state is now maintained within its own objects. a change to note: Instead of two scrub-map collections - one that holds all incoming maps, and one that holds pointers to all of those incoming plus the one we (the Primary) manage - just use one collection for both incoming and our own. Signed-off-by: Ronen Friedman <rfriedma@redhat.com> Split from "osd/scrub: minor fixes split from main "scrub backend" commit" Split from "osd/scrub: scrub components embedded documentation" fix Signed-off-by: Ronen Friedman <rfriedma@redhat.com> osd/scrub: fix the scrubber backend to include all PR ceph#42735 Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

github-actions bot added core mon labels Aug 10, 2021

amathuria force-pushed the wip-amathuria-scrub-stats branch from 75930e1 to 7b61c11 Compare August 10, 2021 14:42

jdurgin reviewed Aug 10, 2021

View reviewed changes

src/osd/osd_types.cc Show resolved Hide resolved

amathuria force-pushed the wip-amathuria-scrub-stats branch from 7b61c11 to 6ec731f Compare August 17, 2021 09:05

amathuria requested a review from ronen-fr August 17, 2021 10:41

ronen-fr reviewed Aug 18, 2021

View reviewed changes

amathuria force-pushed the wip-amathuria-scrub-stats branch from 990b449 to 6214650 Compare September 15, 2021 14:26

github-actions bot added the needs-rebase label Sep 26, 2021

stale bot added stale and removed stale labels Jan 9, 2022

amathuria added 2 commits January 12, 2022 14:57

osd/scrub: Add stats to PG dump for number of objects scrubbed

fbee00a

Addition of a new column in PG dump, OBJECTS_SCRUBBED, which keeps track of the number of objects scrubbed. Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

qa/standalone: add test to check if objects_scrubbed is equal to numb…

91885f1

…er of objects in a PG once a scrub finishes Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

amathuria force-pushed the wip-amathuria-scrub-stats branch from 6214650 to 91885f1 Compare January 12, 2022 12:32

github-actions bot removed the needs-rebase label Jan 12, 2022

ronen-fr reviewed Jan 12, 2022

View reviewed changes

src/osd: reset objects_scrubbed count at the beginning of a new scrub

70f3ddb

Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

ronen-fr approved these changes Jan 13, 2022

View reviewed changes

amathuria added the needs-qa label Jan 13, 2022

neha-ojha added the wip-yuri3-testing label Jan 13, 2022

doc: document new OBJECTS_SCRUBBED column in pg dump

0484c76

Signed-off-by: Aishwarya Mathuria <amathuri@redhat.com>

github-actions bot added the documentation label Jan 14, 2022

yuriw merged commit e419a29 into ceph:master Jan 14, 2022

ronen-fr added a commit to ronen-fr/ceph that referenced this pull request Jan 22, 2022

osd/scrub: fix the scrubber backend to include all PR ceph#42735

0a52053

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

ronen-fr mentioned this pull request Jan 23, 2022

scrub/osd: add a missing 'publish stats to osd' #44744

Merged

yuriw pushed a commit that referenced this pull request Jan 28, 2022

scrub/osd: add a missing 'publish stats to osd'

bd5fb7e

to publish the last scrub status report. The change is needed following the merge of PR #42735. Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

osd/scrub: Add stats to PG dump for number of objects scrubbed #42735

osd/scrub: Add stats to PG dump for number of objects scrubbed #42735

amathuria commented Aug 10, 2021

jdurgin commented Aug 17, 2021

ronen-fr Aug 18, 2021

ronen-fr Aug 18, 2021

neha-ojha commented Aug 25, 2021

github-actions bot commented Sep 26, 2021

stale bot commented Jan 9, 2022

ronen-fr left a comment

ronen-fr Jan 12, 2022

ronen-fr Jan 12, 2022

ronen-fr Jan 12, 2022

amathuria commented Jan 14, 2022

ronen-fr commented Jan 14, 2022 •

edited

amathuria commented Jan 14, 2022 •

edited

ronen-fr commented Jan 14, 2022

amathuria commented Jan 14, 2022

amathuria commented Jan 14, 2022

yuriw commented Jan 14, 2022

ronen-fr commented Jan 14, 2022

neha-ojha commented Jan 14, 2022

osd/scrub: Add stats to PG dump for number of objects scrubbed #42735

osd/scrub: Add stats to PG dump for number of objects scrubbed #42735

Conversation

amathuria commented Aug 10, 2021

Checklist

jdurgin commented Aug 17, 2021

ronen-fr Aug 18, 2021

Choose a reason for hiding this comment

ronen-fr Aug 18, 2021

Choose a reason for hiding this comment

neha-ojha commented Aug 25, 2021

github-actions bot commented Sep 26, 2021

stale bot commented Jan 9, 2022

ronen-fr left a comment

Choose a reason for hiding this comment

ronen-fr Jan 12, 2022

Choose a reason for hiding this comment

ronen-fr Jan 12, 2022

Choose a reason for hiding this comment

ronen-fr Jan 12, 2022

Choose a reason for hiding this comment

amathuria commented Jan 14, 2022

ronen-fr commented Jan 14, 2022 • edited

amathuria commented Jan 14, 2022 • edited

ronen-fr commented Jan 14, 2022

amathuria commented Jan 14, 2022

amathuria commented Jan 14, 2022

yuriw commented Jan 14, 2022

ronen-fr commented Jan 14, 2022

neha-ojha commented Jan 14, 2022

ronen-fr commented Jan 14, 2022 •

edited

amathuria commented Jan 14, 2022 •

edited