[sheepdog] [PATCH] add INFO level operation logging on each node.

[Matsuo Yoshinori]

Thank you for some advice.
I'll move sd_debug and change logging to is_admin_op.
then send v2 later.

By the way, I talked with Hitoshi & Ryusuke.
We want both dog and sheep logging. It is helpfull for tracing operations.

Wrapping dog command can also solve dog logging,
but dog command may be called directory especially in the small cluster.
And it's simpler that having a logging function in dog command
than writing log on every wrapper scripts, isn't it?

Thanks

2014/1/23 MORITA Kazutaka <morita.kazutaka at gmail.com>

> At Fri, 10 Jan 2014 15:05:21 +0900,
> Yoshinori Matsuo wrote:
> >
> > Administrator couldn't track operation on each node without this
> function.
> > This function logs only administrative operation at rx_main and tx_main.
> > Operations that is not related to administrative operation are not
> > logged in this function. such as read/write etc.
> >
> > Signed-off-by: Yoshinori Matsuo <matsuo.yoshinori at lab.ntt.co.jp>
> > ---
> >  sheep/ops.c        |   19 +++++++++++++++++++
> >  sheep/request.c    |   17 +++++++++++++++++
> >  sheep/sheep_priv.h |    1 +
> >  3 files changed, 37 insertions(+), 0 deletions(-)
> >
> > diff --git a/sheep/ops.c b/sheep/ops.c
> > index 1e9bc1e..df2b95f 100644
> > --- a/sheep/ops.c
> > +++ b/sheep/ops.c
> > @@ -26,6 +26,9 @@ struct sd_op_template {
> >       /* process request even when cluster is not working */
> >       bool force;
> >
> > +     /* log operations at info level */
> > +     bool logging;
> > +
>
> I think this variable name should explain why the operation needs to
> be logged.  How about something like "bool is_admin_op"?
>
>
> >  bool has_process_work(const struct sd_op_template *op)
> >  {
> >       return !!op->process_work;
> > diff --git a/sheep/request.c b/sheep/request.c
> > index 9f3f110..15c6cbf 100644
> > --- a/sheep/request.c
> > +++ b/sheep/request.c
> > @@ -762,6 +762,14 @@ static void rx_main(struct work *work)
> >       conn_rx_on(&ci->conn);
> >
> >       sd_debug("%d, %s:%d", ci->conn.fd, ci->conn.ipstr, ci->conn.port);
>
> This sd_debug() should be called only when is_logging_op() is false.
>
> > +     if (is_logging_op(get_sd_op(req->rq.opcode))) {
> > +             sd_info("req=%p, fd=%d, client=%s:%d, op=%s, data=%s",
> > +                     req,
> > +                     ci->conn.fd,
> > +                     ci->conn.ipstr, ci->conn.port,
> > +                     op_name(get_sd_op(req->rq.opcode)),
> > +                     (char *)req->data);
> > +     }
> >       queue_request(req);
> >  }
> >
> > @@ -800,6 +808,15 @@ static void tx_main(struct work *work)
> >
> >       refcount_dec(&ci->refcnt);
> >
> > +     if (is_logging_op(ci->tx_req->op)) {
> > +             sd_info("req=%p, fd=%d, client=%s:%d, op=%s, result=%02X",
> > +                     ci->tx_req,
> > +                     ci->conn.fd,
> > +                     ci->conn.ipstr,
> > +                     ci->conn.port,
> > +                     op_name(ci->tx_req->op),
> > +                     ci->tx_req->rp.result);
> > +     }
>
> How about adding sd_debug() when is_logging_op() is false?
>
> Thanks,
>
> Kazutaka
> --
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> http://lists.wpkg.org/mailman/listinfo/sheepdog
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wpkg.org/pipermail/sheepdog/attachments/20140124/8df62601/attachment-0004.html>