mds: don't add counters in warning for standby-replay MDS by rishabh-d-dave · Pull Request #56065 · ceph/ceph

rishabh-d-dave · 2024-03-08T15:36:54Z

Don't print cache size warnings for standby-replay MDS since it is
simply replaying. Repeating warnings confuses users that cache for 2
MDSs is crossing the limit.

Fixes: https://tracker.ceph.com/issues/63514

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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 dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

src/mds/Beacon.cc

rishabh-d-dave · 2024-03-13T10:00:18Z

Note to reviewers, the test is 90% ready. Leaving it here for now till we're sure that modifications to src/ are 100% correct.

qa/tasks/cephfs/test_failover.py

src/mds/Beacon.cc

qa/tasks/cephfs/test_failover.py

rishabh-d-dave · 2024-03-14T06:19:53Z

Note to reviewers, the test is 90% ready. Leaving it here for now till we're sure that modifications to src/ are 100% correct.

@vshankar Based on your reviews, I am assuming that I am moving in the right direction, I am proceeding to add finishing touches to this PR.

qa/tasks/vstart_runner.py

qa/tasks/cephfs/test_failover.py

qa/tasks/cephfs/filesystem.py

qa/tasks/vstart_runner.py

qa/tasks/cephfs/test_failover.py

rishabh-d-dave · 2024-04-19T04:08:15Z

@vshankar PTAL

Don't include inode and stray counters in the health warnings printed for standby-replay MDSs. Since these counters are present in the health warnings only due to replay, it can confuse users, and therefore, do not include them. Fixes: https://tracker.ceph.com/issues/63514 Signed-off-by: Rishabh Dave <ridave@redhat.com>

rishabh-d-dave · 2024-04-19T08:29:41Z

Unrelated error on make check failure (Agent went offline during the build). Running it again.

rishabh-d-dave · 2024-04-19T08:30:26Z

jenkins make check

rishabh-d-dave · 2024-04-19T08:30:43Z

jenkins test make check

rishabh-d-dave · 2024-04-19T11:15:04Z

make check passed - https://jenkins.ceph.com/job/ceph-pull-requests/133611/

rishabh-d-dave · 2024-04-22T13:15:15Z

Minor, non-functional change that'll make traceback more readable.

- Add tests to verify that inode and stray counters are not replayed/included in the health warnings printed for the standby-replay MDS. - Add "MDS_CACHE_OVERSIZED" health warning to ignorelist to failover.yaml. - Add a helper method to qa.tasks.cephfs.filesystem.Filesystem to get MDS name of standby-replay MDS. Signed-off-by: Rishabh Dave <ridave@redhat.com>

rishabh-d-dave · 2024-04-24T09:23:00Z

Added MDS_CACHE_OVERSIZED to ignorelist, otherwise test_failover.py finishes running successfully but teuthology job is marked as failed since this warning wasn't part of ignorelist for the job.

rishabh-d-dave · 2024-04-24T09:25:03Z

I've run tests several times and for last few times it has been running fine -

https://pulpito.ceph.com/rishabh-2024-04-24_07:28:07-fs:multifs-rishabh-mds-health-testing-default-smithi
https://pulpito.ceph.com/rishabh-2024-04-24_05:44:40-fs:multifs-rishabh-mds-health-testing-default-smithi

This PR is ready for QA.

vshankar · 2024-04-29T05:09:31Z

Minor, non-functional change that'll make traceback more readable.

Could you please explain a bit more about this? Why wasn't the traceback less readable before this update?

rishabh-d-dave · 2024-04-29T05:18:51Z

@vshankar

Minor, non-functional change that'll make traceback more readable.

Could you please explain a bit more about this? Why wasn't the traceback less readable before this update?

The command arguments were not on the same line as the function call, which made it impossible to look at what command failed by looking at traceback. It looked like following -

self.get_ceph_cmd_stdout(

With that change it will look like following -

self.get_ceph_cmd_stdout('health detail'

It might look minor but while debugging I don't need to remember the line number from traceback and then check QA code to find out what command failed. The command failed is apparent from code snippet in traceback itself which makes debugging much much easier and quicker since a test method may have multiple calls to such methods.

vshankar · 2024-05-02T09:54:04Z

@vshankar

Minor, non-functional change that'll make traceback more readable.

Could you please explain a bit more about this? Why wasn't the traceback less readable before this update?

The command arguments were not on the same line as the function call, which made it impossible to look at what command failed by looking at traceback. It looked like following -
self.get_ceph_cmd_stdout(
With that change it will look like following -
self.get_ceph_cmd_stdout('health detail'
It might look minor but while debugging I don't need to remember the line number from traceback and then check QA code to find out what command failed. The command failed is apparent from code snippet in traceback itself which makes debugging much much easier and quicker since a test method may have multiple calls to such methods.

As discussed, please share how the old and new backtraces look like. @rishabh-d-dave

rishabh-d-dave · 2024-05-02T18:38:14Z

@vshankar
Snippet from Traceback 1 -

        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:  File "/home/teuthworker/src/github.com_rishabh-d-dave_ceph_fade9250abf1848492bcb8474d2c5b8ce569ede4/qa/tasks/cephfs/test_failover.py", line 747, in test_health_warn_oversize_cache_has_no_counters
        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:    self.get_ceph_cmd_stdout(

Snippet from Traceback 2 -

        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:  File "/home/teuthworker/src/github.com_rishabh-d-dave_ceph_fade9250abf1848492bcb8474d2c5b8ce569ede4/qa/tasks/cephfs/test_failover.py", line 747, in test_health_warn_oversize_cache_has_no_counters
        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:    self.get_ceph_cmd_stdout('health detail'

* refs/pull/56065/head: qa/cephfs: test that counters are not printed for SR MDS mds: add no counters in warning for standby-replay MDS Reviewed-by: Venky Shankar <vshankar@redhat.com> Reviewed-by: Dhairya Parmar <dparmar@redhat.com>

rishabh-d-dave

QA run was successful - https://tracker.ceph.com/projects/cephfs/wiki/main#3-May-2024.

Testing took more time than expected because there were 25-30 new failures. Most of them caused by a PR in the testing branch but these were resolved on removing that PR.

vshankar · 2024-05-03T07:43:35Z

@vshankar Snippet from Traceback 1 -

        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:  File "/home/teuthworker/src/github.com_rishabh-d-dave_ceph_fade9250abf1848492bcb8474d2c5b8ce569ede4/qa/tasks/cephfs/test_failover.py", line 747, in test_health_warn_oversize_cache_has_no_counters
        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:    self.get_ceph_cmd_stdout(

Snippet from Traceback 2 -

        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:  File "/home/teuthworker/src/github.com_rishabh-d-dave_ceph_fade9250abf1848492bcb8474d2c5b8ce569ede4/qa/tasks/cephfs/test_failover.py", line 747, in test_health_warn_oversize_cache_has_no_counters
        2024-04-12T15:31:30.745 INFO:tasks.cephfs_test_runner:    self.get_ceph_cmd_stdout('health detail'

Looks fine.

rishabh-d-dave · 2024-05-03T07:56:47Z

Linking to QA run summary once more - #56065 (review)

rishabh-d-dave requested a review from a team as a code owner March 8, 2024 15:36

rishabh-d-dave added the cephfs Ceph File System label Mar 8, 2024

rishabh-d-dave requested a review from a team March 8, 2024 15:36

github-actions bot added the documentation label Mar 8, 2024

rishabh-d-dave force-pushed the mds-counters branch from c6d7941 to 8ad1787 Compare March 8, 2024 15:37

rishabh-d-dave removed the request for review from a team March 8, 2024 15:38

rishabh-d-dave removed the documentation label Mar 8, 2024

rishabh-d-dave changed the title ~~src/mds: no cache size warning for standby-replay MDS~~ mds: no cache size warning for standby-replay MDS Mar 8, 2024

rishabh-d-dave force-pushed the mds-counters branch from 8ad1787 to 23ce887 Compare March 8, 2024 15:44

mchangir reviewed Mar 12, 2024

View reviewed changes

src/mds/Beacon.cc Outdated Show resolved Hide resolved

vshankar requested a review from a team March 12, 2024 06:28

rishabh-d-dave force-pushed the mds-counters branch from 23ce887 to 84c984c Compare March 12, 2024 20:00

github-actions bot added the tests label Mar 12, 2024

rishabh-d-dave force-pushed the mds-counters branch from 84c984c to 4dd42d1 Compare March 13, 2024 08:40

vshankar requested a review from mchangir March 13, 2024 08:43

vshankar requested changes Mar 14, 2024

View reviewed changes

rishabh-d-dave force-pushed the mds-counters branch from 4dd42d1 to ed43db4 Compare March 14, 2024 06:18

rishabh-d-dave force-pushed the mds-counters branch from ed43db4 to e5556a5 Compare March 26, 2024 06:15

vshankar requested changes Apr 1, 2024

View reviewed changes

rishabh-d-dave force-pushed the mds-counters branch 7 times, most recently from dffdca8 to dea6c6f Compare April 7, 2024 14:08

vshankar reviewed Apr 8, 2024

View reviewed changes

qa/tasks/cephfs/test_failover.py Outdated Show resolved Hide resolved

vshankar reviewed Apr 8, 2024

View reviewed changes

qa/tasks/cephfs/test_failover.py Outdated Show resolved Hide resolved

rishabh-d-dave force-pushed the mds-counters branch from ca81997 to bc99fe7 Compare April 19, 2024 04:09

vshankar approved these changes Apr 19, 2024

View reviewed changes

dparmar18 approved these changes Apr 19, 2024

View reviewed changes

rishabh-d-dave force-pushed the mds-counters branch from bc99fe7 to b4bf71e Compare April 22, 2024 12:58

rishabh-d-dave changed the title ~~mds: no cache size warning for standby-replay MDS~~ mds: don't add counters in warning for standby-replay MDS Apr 22, 2024

rishabh-d-dave force-pushed the mds-counters branch from b4bf71e to 2784e22 Compare April 23, 2024 14:58

rishabh-d-dave added the wip-rishabh-testing2 label Apr 24, 2024

rishabh-d-dave commented May 3, 2024

View reviewed changes

rishabh-d-dave merged commit 0b71779 into ceph:main May 3, 2024

rishabh-d-dave deleted the mds-counters branch May 3, 2024 09:50

This was referenced May 30, 2024

squid: mds: don't add counters in warning for standby-replay MDS #57795

Merged

reef: mds: don't add counters in warning for standby-replay MDS #57834

Merged

quincy: mds: don't add counters in warning for standby-replay MDS #57835

Closed

Conversation

rishabh-d-dave commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

Uh oh!

rishabh-d-dave commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rishabh-d-dave commented Mar 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rishabh-d-dave commented Apr 19, 2024

Uh oh!

rishabh-d-dave commented Apr 19, 2024

Uh oh!

rishabh-d-dave commented Apr 19, 2024

Uh oh!

rishabh-d-dave commented Apr 19, 2024

Uh oh!

rishabh-d-dave commented Apr 19, 2024

Uh oh!

rishabh-d-dave commented Apr 22, 2024

Uh oh!

rishabh-d-dave commented Apr 24, 2024

Uh oh!

rishabh-d-dave commented Apr 24, 2024

Uh oh!

vshankar commented Apr 29, 2024

Uh oh!

rishabh-d-dave commented Apr 29, 2024

Uh oh!

vshankar commented May 2, 2024

Uh oh!

rishabh-d-dave commented May 2, 2024

Uh oh!

rishabh-d-dave left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vshankar commented May 3, 2024

Uh oh!

rishabh-d-dave commented May 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rishabh-d-dave commented Mar 8, 2024 •

edited

Loading

rishabh-d-dave commented Mar 13, 2024 •

edited

Loading

rishabh-d-dave commented Mar 14, 2024 •

edited

Loading

rishabh-d-dave left a comment •

edited

Loading