Re: [PATCH v3] libceph: add osd op counter metric support

[Ilya Dryomov <idryomov@gmail.com>]

On Thu, Nov 12, 2020 at 3:30 AM Xiubo Li <xiubli@redhat.com> wrote:
>
> On 2020/11/11 23:18, Patrick Donnelly wrote:
> > On Tue, Nov 10, 2020 at 7:45 AM Ilya Dryomov <idryomov@gmail.com> wrote:
> >> On Tue, Nov 10, 2020 at 3:19 PM <xiubli@redhat.com> wrote:
> >>> From: Xiubo Li <xiubli@redhat.com>
> >>>
> >>> The logic is the same with osdc/Objecter.cc in ceph in user space.
> >>>
> >>> URL: https://tracker.ceph.com/issues/48053
> >>> Signed-off-by: Xiubo Li <xiubli@redhat.com>
> >>> ---
> >>>
> >>> V3:
> >>> - typo fixing about oring the _WRITE
> >>>
> >>>   include/linux/ceph/osd_client.h |  9 ++++++
> >>>   net/ceph/debugfs.c              | 13 ++++++++
> >>>   net/ceph/osd_client.c           | 56 +++++++++++++++++++++++++++++++++
> >>>   3 files changed, 78 insertions(+)
> >>>
> >>> diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h
> >>> index 83fa08a06507..24301513b186 100644
> >>> --- a/include/linux/ceph/osd_client.h
> >>> +++ b/include/linux/ceph/osd_client.h
> >>> @@ -339,6 +339,13 @@ struct ceph_osd_backoff {
> >>>          struct ceph_hobject_id *end;
> >>>   };
> >>>
> >>> +struct ceph_osd_metric {
> >>> +       struct percpu_counter op_ops;
> >>> +       struct percpu_counter op_rmw;
> >>> +       struct percpu_counter op_r;
> >>> +       struct percpu_counter op_w;
> >>> +};
> >> OK, so only reads and writes are really needed.  Why not expose them
> >> through the existing metrics framework in fs/ceph?  Wouldn't "fs top"
> >> want to display them?  Exposing latency information without exposing
> >> overall counts seems rather weird to me anyway.
> > `fs top` may want to eventually display this information but the
> > intention was to have a "perf dump"-like debugfs file that has
> > information about the number of osd op reads/writes. We need that for
> > this test:
> >
> > https://github.com/ceph/ceph/blob/master/qa/tasks/cephfs/test_readahead.py#L20
> >
> > Pulling the information out through `fs top` is not a direction I'd like to go.
> >
> >> The fundamental problem is that debugfs output format is not stable.
> >> The tracker mentions test_readahead -- updating some teuthology test
> >> cases from time to time is not a big deal, but if a user facing tool
> >> such as "fs top" starts relying on these, it would be bad.
> > `fs top` will not rely on debugfs files.
> >
> There has one bug in the "test_readahead.py", I have fixed it in [1]. I
> think the existing cephfs metric framework is far enough for us to
> support the readahead qa test for kclient.
>
> [1] https://github.com/ceph/ceph/pull/38016

Yeah, it already provides a debugfs file, so the test wouldn't need
"fs top" to pull that counter out.

Thanks,

                Ilya