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

cephfs-top: Multiple filesystem support #45473

Merged
merged 2 commits into from May 4, 2022
Merged

Conversation

neesingh-rh
Copy link
Contributor

@neesingh-rh neesingh-rh commented Mar 17, 2022

Cephfs-top command was only showing metrics for one filesystem due to absence of metadata in perf_stats command.

Fixes: https://tracker.ceph.com/issues/50470
Signed-off-by: Neeraj Pratap Singh neesingh@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

@neesingh-rh neesingh-rh added the cephfs Ceph File System label Mar 17, 2022
@neesingh-rh neesingh-rh requested a review from a team March 17, 2022 04:47
@neesingh-rh neesingh-rh self-assigned this Mar 17, 2022
@neesingh-rh
Copy link
Contributor Author

Adding snaps:
1.perf_stats:
perf_stats
2.cephfs-top:
cephfs-top

@lxbsz
Copy link
Member

lxbsz commented Mar 17, 2022

Will it make sense to show the clients from different filesystems in dedicated windows ?

@neesingh-rh
Copy link
Contributor Author

Will it make sense to show the clients from different filesystems in dedicated windows ?

I guess its in respect with cephfs-top window,right?How about adding one column with file system name?

@lxbsz
Copy link
Member

lxbsz commented Mar 17, 2022

Will it make sense to show the clients from different filesystems in dedicated windows ?

I guess its in respect with cephfs-top window,right?

Yeah. It will be split into several virtual windows in the terminal.

How about adding one column with file system name?

Yeah, name or the fscid, but I am afraid in product use case the name or fscid will be very long(?), so add a new column will make the couldn't display it all in one line ?

@neesingh-rh
Copy link
Contributor Author

Will it make sense to show the clients from different filesystems in dedicated windows ?

I guess its in respect with cephfs-top window,right?

Yeah. It will be split into several virtual windows in the terminal.

How about adding one column with file system name?

Yeah, name or the fscid, but I am afraid in product use case the name or fscid will be very long(?), so add a new column will make the couldn't display it all in one line ?

Regarding separate windows for filesystems, I think we can have it in two ways:

  1. Add a method in cephfs-top which fetches fs_name and client counts or other details as required.
  2. We will have to change the structure of perf_stats o/p, so that we can directly fetch the required info from there.Structure will be something like this:
    {version:”value”,....,client_metadata:{fs1:{client1.id:{“IP”,....,valid_metrics},client2.id:{“IP”,....,valid_metrics}},fs2:{client1.id:{“IP”,....,valid_metrics}},global_metrics:{...},.....}
    Any suggestions,which one will be better?Or some other better way of doing it?

@vshankar
Copy link
Contributor

Will it make sense to show the clients from different filesystems in dedicated windows ?

Nice idea. @neesingh-rh Let's do this as a separate PR.

@vshankar
Copy link
Contributor

@neesingh-rh Please add tests in `qa/tasks/cephfs/test_mds_metrics.py' to verify that client metrics and metadata are updated as expected.

@lxbsz
Copy link
Member

lxbsz commented Mar 18, 2022

Will it make sense to show the clients from different filesystems in dedicated windows ?

Nice idea. @neesingh-rh Let's do this as a separate PR.

Certainly.

@lxbsz
Copy link
Member

lxbsz commented Mar 18, 2022

Yeah, name or the fscid, but I am afraid in product use case the name or fscid will be very long(?), so add a new column will make the couldn't display it all in one line ?

Regarding separate windows for filesystems, I think we can have it in two ways:

  1. Add a method in cephfs-top which fetches fs_name and client counts or other details as required.
  2. We will have to change the structure of perf_stats o/p, so that we can directly fetch the required info from there.Structure will be something like this:
    {version:”value”,....,client_metadata:{fs1:{client1.id:{“IP”,....,valid_metrics},client2.id:{“IP”,....,valid_metrics}},fs2:{client1.id:{“IP”,....,valid_metrics}},global_metrics:{...},.....}
    Any suggestions,which one will be better?Or some other better way of doing it?

The first one sounds simpler and easier to do ?

@neesingh-rh
Copy link
Contributor Author

Yeah, name or the fscid, but I am afraid in product use case the name or fscid will be very long(?), so add a new column will make the couldn't display it all in one line ?

Regarding separate windows for filesystems, I think we can have it in two ways:

  1. Add a method in cephfs-top which fetches fs_name and client counts or other details as required.
  2. We will have to change the structure of perf_stats o/p, so that we can directly fetch the required info from there.Structure will be something like this:
    {version:”value”,....,client_metadata:{fs1:{client1.id:{“IP”,....,valid_metrics},client2.id:{“IP”,....,valid_metrics}},fs2:{client1.id:{“IP”,....,valid_metrics}},global_metrics:{...},.....}
    Any suggestions,which one will be better?Or some other better way of doing it?

The first one sounds simpler and easier to do ?

Yes.

src/pybind/mgr/stats/fs/perf_stats.py Outdated Show resolved Hide resolved
qa/tasks/cephfs/test_mds_metrics.py Outdated Show resolved Hide resolved
qa/tasks/cephfs/test_mds_metrics.py Outdated Show resolved Hide resolved
qa/tasks/cephfs/test_mds_metrics.py Show resolved Hide resolved
qa/tasks/cephfs/test_mds_metrics.py Outdated Show resolved Hide resolved
@neesingh-rh neesingh-rh force-pushed the fix_50470 branch 2 times, most recently from bf27623 to 0561552 Compare March 23, 2022 10:11
Signed-off-by: Neeraj Pratap Singh <neesingh@redhat.com>
@neesingh-rh neesingh-rh force-pushed the fix_50470 branch 4 times, most recently from 620a007 to b830239 Compare March 29, 2022 07:02
Signed-off-by: Neeraj Pratap Singh <neesingh@redhat.com>
@neesingh-rh
Copy link
Contributor Author

jenkins test make check

@gregsfortytwo
Copy link
Member

@vshankar you had a comment which looks like it is resolved, is there something blocking this from an integration branch? :)

@vshankar
Copy link
Contributor

@vshankar you had a comment which looks like it is resolved, is there something blocking this from an integration branch? :)

It's in my branch for test run over weekend, just that I didn't tag it.

Tagging it now..

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
Projects
None yet
7 participants