Skip to content

client: add option to disable collecting and sending metrics#45370

Merged
vshankar merged 2 commits intoceph:masterfrom
lxbsz:wip-54411
Apr 26, 2022
Merged

client: add option to disable collecting and sending metrics#45370
vshankar merged 2 commits intoceph:masterfrom
lxbsz:wip-54411

Conversation

@lxbsz
Copy link
Member

@lxbsz lxbsz commented Mar 14, 2022 •
edited
Loading

Fixes: https://tracker.ceph.com/issues/54411
Signed-off-by: Xiubo Li xiubli@redhat.com

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

@github-actions github-actions bot added cephfs Ceph File System common labels Mar 14, 2022
@lxbsz lxbsz requested review from a team and vshankar March 14, 2022 06:32
@lxbsz lxbsz closed this Mar 14, 2022
mchangir
mchangir reviewed Mar 14, 2022
View reviewed changes
@lxbsz lxbsz reopened this Mar 14, 2022
@lxbsz lxbsz mentioned this pull request Mar 14, 2022
Closed
14 tasks
gregsfortytwo
gregsfortytwo previously requested changes Mar 14, 2022
View reviewed changes
Copy link
Member

@gregsfortytwo gregsfortytwo left a comment

Choose a reason for hiding this comment

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

If we're trying to match #43974 don't we need to use this option to turn off the metrics collection in the v16.2.4.yaml frag? :)

@lxbsz
Copy link
Member Author

lxbsz commented Mar 15, 2022

If we're trying to match #43974 don't we need to use this option to turn off the metrics collection in the v16.2.4.yaml frag? :)

Yeah, we need it. I missed that file, I thought it was removed and only exists in Pacific branch.
Will fix it.

@lxbsz lxbsz mentioned this pull request Mar 15, 2022
Closed
14 tasks
joscollin
joscollin reviewed Mar 15, 2022
View reviewed changes
kotreshhr
kotreshhr reviewed Mar 15, 2022
View reviewed changes
batrick
batrick previously requested changes Mar 15, 2022
View reviewed changes
@lxbsz lxbsz force-pushed the wip-54411 branch 2 times, most recently from 8daf1ca to e801db1 Compare March 17, 2022 03:16
@lxbsz
Copy link
Member Author

lxbsz commented Mar 17, 2022 •
edited
Loading

Updated this by using the new approach:

1, Add one client_collect_and_send_global_metrics option, which is false as default, with this users can force enabling sending the metrics to MDSes. This will make sure that the clients still could send metrics if users are using the old ceph clusters which are safe to receive unknown metrics.

2, When opening the sessions the MDSes will fill the metric flags they supported and send back to clients, and then in Clients side the clients will only send the metrics supported by MDSes as default. While users still could force enabling to send all the metrics by enabling the client_collect_and_send_global_metrics option.

vshankar
vshankar reviewed Mar 23, 2022
View reviewed changes
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.

LGTM

@lxbsz
Copy link
Member Author

lxbsz commented Apr 14, 2022

Just rebased it to the latest code, nothing changed.

lxbsz added 2 commits April 18, 2022 10:18
@lxbsz
client: add option to enable and force collecting and sending metrics
beb9f16 
To be careful to enable this because it may crash the old MDSes while
upgrading.

Fixes: https://tracker.ceph.com/issues/54411
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
mds, client: only send the metrices supported by MDSes
e9a26c5 
For the old ceph clusters the clients won't send any metrics to
them as default unless they have backported this commit, but there
has one option 'client_collect_and_send_global_metrics' still could
be used to enable it manually.

This will fix the crash bug when upgrading from old ceph clusters,
which will crash the MDSes once they receive unknown metrics.

Fixes: https://tracker.ceph.com/issues/54411
Signed-off-by: Xiubo Li <xiubli@redhat.com>
@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2022

Just rebased it to the latest code, nothing changed.

@lxbsz
Copy link
Member Author

lxbsz commented Apr 18, 2022

jenkins test make check

@vshankar
Copy link
Contributor

vshankar commented Apr 25, 2022

https://tracker.ceph.com/projects/cephfs/wiki/Master#2022-Apr-25

@vshankar vshankar dismissed stale reviews from batrick and gregsfortytwo April 26, 2022 04:24

comments addressed.

@vshankar vshankar merged commit 210bae7 into ceph:master Apr 26, 2022
@lxbsz lxbsz mentioned this pull request May 5, 2022
Merged
14 tasks
@lxbsz lxbsz mentioned this pull request Jun 1, 2022
Merged
14 tasks
@lxbsz lxbsz mentioned this pull request Jun 22, 2022
Merged
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vshankar vshankar vshankar approved these changes

@batrick batrick Awaiting requested review from batrick

@gregsfortytwo gregsfortytwo Awaiting requested review from gregsfortytwo

@joscollin joscollin Awaiting requested review from joscollin

@mchangir mchangir Awaiting requested review from mchangir

@kotreshhr kotreshhr Awaiting requested review from kotreshhr

Assignees

No one assigned

Labels

cephfs Ceph File System common mgr pybind wip-vshankar-testing

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@lxbsz @vshankar @batrick @gregsfortytwo @kotreshhr @mchangir @joscollin