<div dir="ltr"><div>Thank you for some advice.<br></div><div>I'll move sd_debug and change logging to is_admin_op.<br></div>then send v2 later.<br><div><div><div><div><div><div class="gmail_extra"><br></div><div class="gmail_extra">
By the way, I talked with Hitoshi & Ryusuke.<br></div><div class="gmail_extra"></div><div class="gmail_extra">We want both dog and sheep logging. It is helpfull for tracing operations.<br></div><div class="gmail_extra">
<br>Wrapping dog command can also solve dog logging,<br>but dog command may be called directory especially in the small cluster.<br></div><div class="gmail_extra"></div><div class="gmail_extra">
And it's simpler that having a logging function in dog command<br>than writing log on every wrapper scripts, isn't it?<br></div><div class="gmail_extra"></div><div class="gmail_extra"><br></div><div class="gmail_extra">
Thanks<br><br></div><div class="gmail_extra"><div class="gmail_quote">
2014/1/23 MORITA Kazutaka <span dir="ltr"><<a href="mailto:morita.kazutaka@gmail.com" target="_blank">morita.kazutaka@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
At Fri, 10 Jan 2014 15:05:21 +0900,<br>
<div>Yoshinori Matsuo wrote:<br>
><br>
> Administrator couldn't track operation on each node without this function.<br>
> This function logs only administrative operation at rx_main and tx_main.<br>
> Operations that is not related to administrative operation are not<br>
> logged in this function. such as read/write etc.<br>
><br>
> Signed-off-by: Yoshinori Matsuo <<a href="mailto:matsuo.yoshinori@lab.ntt.co.jp" target="_blank">matsuo.yoshinori@lab.ntt.co.jp</a>><br>
> ---<br>
> sheep/ops.c | 19 +++++++++++++++++++<br>
> sheep/request.c | 17 +++++++++++++++++<br>
> sheep/sheep_priv.h | 1 +<br>
> 3 files changed, 37 insertions(+), 0 deletions(-)<br>
><br>
> diff --git a/sheep/ops.c b/sheep/ops.c<br>
> index 1e9bc1e..df2b95f 100644<br>
> --- a/sheep/ops.c<br>
> +++ b/sheep/ops.c<br>
> @@ -26,6 +26,9 @@ struct sd_op_template {<br>
> /* process request even when cluster is not working */<br>
> bool force;<br>
><br>
> + /* log operations at info level */<br>
> + bool logging;<br>
> +<br>
<br>
</div>I think this variable name should explain why the operation needs to<br>
be logged. How about something like "bool is_admin_op"?<br>
<div><br>
<br>
> bool has_process_work(const struct sd_op_template *op)<br>
> {<br>
> return !!op->process_work;<br>
> diff --git a/sheep/request.c b/sheep/request.c<br>
> index 9f3f110..15c6cbf 100644<br>
> --- a/sheep/request.c<br>
> +++ b/sheep/request.c<br>
> @@ -762,6 +762,14 @@ static void rx_main(struct work *work)<br>
> conn_rx_on(&ci->conn);<br>
><br>
> sd_debug("%d, %s:%d", ci->conn.fd, ci->conn.ipstr, ci->conn.port);<br>
<br>
</div>This sd_debug() should be called only when is_logging_op() is false.<br>
<div><br>
> + if (is_logging_op(get_sd_op(req->rq.opcode))) {<br>
> + sd_info("req=%p, fd=%d, client=%s:%d, op=%s, data=%s",<br>
> + req,<br>
> + ci->conn.fd,<br>
> + ci->conn.ipstr, ci->conn.port,<br>
> + op_name(get_sd_op(req->rq.opcode)),<br>
> + (char *)req->data);<br>
> + }<br>
> queue_request(req);<br>
> }<br>
><br>
> @@ -800,6 +808,15 @@ static void tx_main(struct work *work)<br>
><br>
> refcount_dec(&ci->refcnt);<br>
><br>
> + if (is_logging_op(ci->tx_req->op)) {<br>
> + sd_info("req=%p, fd=%d, client=%s:%d, op=%s, result=%02X",<br>
> + ci->tx_req,<br>
> + ci->conn.fd,<br>
> + ci->conn.ipstr,<br>
> + ci->conn.port,<br>
> + op_name(ci->tx_req->op),<br>
> + ci->tx_req->rp.result);<br>
> + }<br>
<br>
</div>How about adding sd_debug() when is_logging_op() is false?<br>
<br>
Thanks,<br>
<br>
Kazutaka<br>
<div><div>--<br>
sheepdog mailing list<br>
<a href="mailto:sheepdog@lists.wpkg.org" target="_blank">sheepdog@lists.wpkg.org</a><br>
<a href="http://lists.wpkg.org/mailman/listinfo/sheepdog" target="_blank">http://lists.wpkg.org/mailman/listinfo/sheepdog</a><br>
</div></div></blockquote></div><br></div></div></div></div></div></div></div>