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.

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

client: do not send metrics until the MDS rank is ready #51850

Merged
merged 1 commit into from Jul 17, 2023

Conversation

lxbsz
Copy link
Member

@lxbsz lxbsz commented May 31, 2023

In some cases when there are a lot of clients and these clients have a lots of known requests need to replay too, the metrics requests will be dropped by the MDS because the MDS is still in the clientreplay state, and also the useless metric requests will slow down MDS.

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

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

@lxbsz lxbsz requested a review from a team May 31, 2023 10:56
@github-actions github-actions bot added the cephfs Ceph File System label May 31, 2023
src/client/Client.cc Outdated Show resolved Hide resolved
@lxbsz lxbsz force-pushed the wip-61523 branch 3 times, most recently from a6bef3e to a2be5ec Compare June 1, 2023 03:09
@lxbsz
Copy link
Member Author

lxbsz commented Jun 1, 2023

@dparmar18 just remind me this code location in the function and I just moved it in front of the Client::collect_and_send_global_metrics(). Because if the mds.0 is not in up:active state there still could be opened sessions.

Thanks @dparmar18

Copy link
Member

@joscollin joscollin left a comment

Choose a reason for hiding this comment

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

I think it's better to have these checks in the next tick
https://github.com/ceph/ceph/blob/main/src/client/Client.cc#L6868

@lxbsz
Copy link
Member Author

lxbsz commented Jun 1, 2023

I think it's better to have these checks in the next tick https://github.com/ceph/ceph/blob/main/src/client/Client.cc#L6868

Sound good to me and will fix it. Thanks @joscollin

src/client/Client.cc Outdated Show resolved Hide resolved
In some cases when there are a lot of clients and these clients
have a lots of known requests need to replay too, the metrics
requests will be dropped by the MDS because the MDS is still in
the clientreplay state, and also the useless metric requests will
slow down MDS.

Fixes: https://tracker.ceph.com/issues/61523
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@vshankar vshankar added the wip-rishabh-testing Rishabh's testing label label Jun 26, 2023
@vshankar
Copy link
Contributor

@rishabh-d-dave please take this one.

@vshankar vshankar added wip-vshankar-testing4 and removed wip-rishabh-testing Rishabh's testing label labels Jun 28, 2023
@vshankar
Copy link
Contributor

@rishabh-d-dave please take this one.

@rishabh-d-dave Included this in my test branch.

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cephfs Ceph File System
Projects
None yet
4 participants