Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

reef: mds: blocklist clients with "bloated" session metadata #53329

Merged
merged 6 commits into from Oct 18, 2023
Merged

reef: mds: blocklist clients with "bloated" session metadata #53329

merged 6 commits into from Oct 18, 2023

Conversation

vshankar
Copy link
Contributor

@vshankar vshankar commented Sep 8, 2023

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

Contribution Guidelines

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)
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

@vshankar vshankar requested a review from a team September 8, 2023 05:37
@github-actions github-actions bot added this to the reef milestone Sep 8, 2023
joscollin
joscollin reviewed Sep 8, 2023
View reviewed changes
Copy link
Member

@joscollin joscollin left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you cherry-picked these two commits, it wouldn't conflict:

  1. 9377e80
  2. 22e4bcf

Just FYI.

@vshankar
Copy link
Contributor Author

vshankar commented Sep 8, 2023

If you cherry-picked these two commits, it wouldn't conflict:

  1. 9377e80

This is fine, but its not critical.

  1. 22e4bcf

This is from a feature which already is backported but not merged. SO, when it merged, the rebase will resolve one part of the conflict.

@vshankar
mds: blocklist clients with "bloated" session metadata
e33fd5c 
Buggy clients (or maybe a MDS bug) causes a huge buildup of
`completed_requests` metadata in its session information.
This could cause the MDS to go read-only when its flushing
session metadata to the journal since the bloated metadata
causes the ODSOp payload to exceed the maximum write size.

Blocklist such clients so as to allow the MDS to continue
servicing requests.

Fixes: http://tracker.ceph.com/issues/61947
Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit bc6814d)

 Conflicts:
	src/common/options/mds.yaml.in
	src/mds/MDSRank.cc

Straight-forward conflict is mds.yaml.in due to missing configs in
reef and likewise in MDSRank.cc
@vshankar
mds: add perf counter to track number of sessions evicted due to meta…
92f5cfc 
…data threshold being exceeded

Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit 59dd587)
@vshankar
test: add test to verify that a buggy client is blocklisted
3156245 
... when its session metadata is bloated due to buildup of
`completed_requests`.

Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit 84df4b3)
@vshankar
PendingReleaseNotes: add a note for mds_session_metadata_threshold …
4690b9f 
…mds config

Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit ac3ab1a)
@vshankar
qa: lower mds_session_metadata_threshold for tests
71e8009 
... and increase the number of files that are created so as to
hit the threshold with a high probability.

Fixes: http://tracker.ceph.com/issues/62873
Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit 92200d9)
@vshankar
qa: typo fix when checking for perf counter - s/md_thresh_evicted/mdt…
cd71b7c 
…hresh_evicted

Signed-off-by: Venky Shankar <vshankar@redhat.com>
(cherry picked from commit 5856a1e)
@vshankar
Copy link
Contributor Author

jenkins test windows

lxbsz
lxbsz approved these changes Oct 17, 2023
View reviewed changes
Copy link
Member

@lxbsz lxbsz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked all the 15 test failures and all are not related to this PR:

https://pulpito.ceph.com/vshankar-2023-10-09_16:48:27-fs-wip-vshankar-testing-reef-20231009.131610-testing-default-smithi/

LGTM.

@yuriw yuriw merged commit ee589bd into ceph:reef Oct 18, 2023
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joscollin joscollin joscollin left review comments

@lxbsz lxbsz lxbsz approved these changes

Assignees
No one assigned
Labels
cephfs Ceph File System common documentation needs-qa tests wip-vshankar-testing
Projects
None yet
Milestone
reef
4 participants
@vshankar @joscollin @lxbsz @yuriw