Re: [PATCH v2 2/5] ceph: periodically send perf metrics to ceph

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020/6/19 20:22, Jeff Layton wrote:
On Fri, 2020-06-19 at 11:35 +0200, Ilya Dryomov wrote:
On Fri, Jun 19, 2020 at 2:38 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
On 2020/6/18 22:42, Jeff Layton wrote:
On Thu, 2020-06-18 at 07:59 -0400, xiubli@xxxxxxxxxx wrote:
From: Xiubo Li <xiubli@xxxxxxxxxx>

This will send the caps/read/write/metadata metrics to any available
MDS only once per second as default, which will be the same as the
userland client, or every metric_send_interval seconds, which is a
module parameter.

URL: https://tracker.ceph.com/issues/43215
Signed-off-by: Xiubo Li <xiubli@xxxxxxxxxx>
---
   fs/ceph/mds_client.c         |   3 +
   fs/ceph/metric.c             | 134 +++++++++++++++++++++++++++++++++++++++++++
   fs/ceph/metric.h             |  78 +++++++++++++++++++++++++
   fs/ceph/super.c              |  49 ++++++++++++++++
   fs/ceph/super.h              |   2 +
   include/linux/ceph/ceph_fs.h |   1 +
   6 files changed, 267 insertions(+)


I think 3/5 needs to moved ahead of this one or folded into it, as we'll
have a temporary regression otherwise.

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index c9784eb1..5f409dd 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -27,6 +27,9 @@
   #include <linux/ceph/auth.h>
   #include <linux/ceph/debugfs.h>

+static DEFINE_MUTEX(ceph_fsc_lock);
+static LIST_HEAD(ceph_fsc_list);
+
   /*
    * Ceph superblock operations
    *
@@ -691,6 +694,10 @@ static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt,
      if (!fsc->wb_pagevec_pool)
              goto fail_cap_wq;

+    mutex_lock(&ceph_fsc_lock);
+    list_add_tail(&fsc->list, &ceph_fsc_list);
+    mutex_unlock(&ceph_fsc_lock);
+
      return fsc;

   fail_cap_wq:
@@ -717,6 +724,10 @@ static void destroy_fs_client(struct ceph_fs_client *fsc)
   {
      dout("destroy_fs_client %p\n", fsc);

+    mutex_lock(&ceph_fsc_lock);
+    list_del(&fsc->list);
+    mutex_unlock(&ceph_fsc_lock);
+
      ceph_mdsc_destroy(fsc);
      destroy_workqueue(fsc->inode_wq);
      destroy_workqueue(fsc->cap_wq);
@@ -1282,6 +1293,44 @@ static void __exit exit_ceph(void)
      destroy_caches();
   }

+static int param_set_metric_interval(const char *val, const struct kernel_param *kp)
+{
+    struct ceph_fs_client *fsc;
+    unsigned int interval;
+    int ret;
+
+    ret = kstrtouint(val, 0, &interval);
+    if (ret < 0) {
+            pr_err("Failed to parse metric interval '%s'\n", val);
+            return ret;
+    }
+
+    if (interval > 5) {
+            pr_err("Invalid metric interval %u\n", interval);
+            return -EINVAL;
+    }
+
Why do we want to reject an interval larger than 5s? Is that problematic
for some reason?
IMO, a larger interval doesn't make much sense, to limit the interval
value in 5s to make sure that the ceph side could show the client real
metrics in time. Is this okay ? Or should we use a larger limit ?
5s may seem like a lot now, but in 5-10 years we might be wishing we had
the ability to change this to something else (e.g.: maybe we find that
stats every 5s cause undue load on the MDS, and need to increase it).

I wonder if providing the option to tune the interval makes sense
at all then.  Since most clients will be sending their metrics every
second, the MDS may eventually start relying on that in some way.
Would a simple on/off switch, to be used if sending metrics causes
unforeseen trouble, work?

Yeah, I'm leery of grabbing values out of our nether regions like this,
and it doesn't seem like making this that configurable has many
benefits.

Let's use an on/off switch here.

Perhaps the preferred interval should be something advertised by the MDS
in some way? New field in the mdsmap, perhaps?

For this maybe we could add one MDS or global option in ceph or one parameter in MDS perf query interface based PR[1] in future.

For now the on/off switch should be good and enough.

[1] https://github.com/ceph/ceph/pull/29951

Thanks

BRs




[Index of Archives]     [CEPH Users]     [Ceph Large]     [Ceph Dev]     [Information on CEPH]     [Linux BTRFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux