[sheepdog] [PATCH v2] sheep: make sheep request stat finer

[Hitoshi Mitake]

At Mon, 6 Jan 2014 15:52:53 +0800,
Liu Yuan wrote:
> 
> On Mon, Jan 06, 2014 at 04:49:30PM +0900, Hitoshi Mitake wrote:
> > At Mon, 6 Jan 2014 15:30:42 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Mon, Jan 06, 2014 at 04:24:28PM +0900, Hitoshi Mitake wrote:
> > > > At Sun,  5 Jan 2014 01:11:32 +0800,
> > > > Liu Yuan wrote:
> > > > > 
> > > > > With this patch we can poke more information about the node stat:
> > > > > 
> > > > > $ dog node stat -w
> > > > > 
> > > > > ...
> > > > > Client	12	340681	100459	233694	0	6528	44 GB	26 GB	95 MB	0.0 MB
> > > > > Peer	6	41013	4008	37005	0	0	8.6 GB	3.9 GB	23 MB	0.0 MB
> > > > > Request	Active	Total	Write	Read	Remove	Flush	All WR	All RD	WRBW	RDBW
> > > > > Client	5	341362	100459	234375	0	6528	44 GB	26 GB	98 MB	0.0 MB
> > > > > Peer	0	41128	4008	37120	0	0	8.6 GB	3.9 GB	24 MB	0.0 MB
> > > > > Request	Active	Total	Write	Read	Remove	Flush	All WR	All RD	WRBW	RDBW
> > > > > Client	0	341969	100459	234982	0	6528	44 GB	26 GB	92 MB	0.0 MB
> > > > > Peer	0	41220	4008	37212	0	0	8.6 GB	3.9 GB	24 MB	0.0 MB
> > > > > Request	Active	Total	Write	Read	Remove	Flush	All WR	All RD	WRBW	RDBW
> > > > > Client	8	342584	100459	235597	0	6528	44 GB	26 GB	113 MB	0.0 MB
> > > > > Peer	0	41335	4008	37327	0	0	8.6 GB	3.9 GB	23 MB	0.0 MB
> > > > > Request	Active	Total	Write	Read	Remove	Flush	All WR	All RD	WRBW	RDBW
> > > > > Client	6	343069	100460	236081	0	6528	44 GB	26 GB	86 MB	4.0 MB
> > > > > Peer	2	41429	4009	37420	0	0	8.7 GB	3.9 GB	26 MB	1.0 MB
> > > > > ...
> > > > > 
> > > > > Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> > > > > ---
> > > > >  dog/node.c               | 64 ++++++++++++++++++++++++++++++++++++------------
> > > > >  include/internal_proto.h |  7 ++++++
> > > > >  sheep/request.c          | 34 +++++++++++++++++++++++++
> > > > >  3 files changed, 90 insertions(+), 15 deletions(-)
> > > > > 
> > > > > diff --git a/dog/node.c b/dog/node.c
> > > > > index f896078..71486a0 100644
> > > > > --- a/dog/node.c
> > > > > +++ b/dog/node.c
> > > > > @@ -275,9 +275,9 @@ static int node_stat(int argc, char **argv)
> > > > >  {
> > > > >  	struct sd_req hdr;
> > > > >  	struct sd_rsp *rsp = (struct sd_rsp *)&hdr;
> > > > > -	struct sd_stat stat;
> > > > > +	struct sd_stat stat, last;
> > > > >  	int ret;
> > > > > -	uint32_t i = node_cmd_data.watch ? UINT32_MAX : 0;
> > > > > +	bool watch = node_cmd_data.watch ? true : false, first = true;
> > > > >  
> > > > >  again:
> > > > >  	sd_init_req(&hdr, SD_OP_STAT);
> > > > > @@ -292,21 +292,55 @@ again:
> > > > >  		return EXIT_FAILURE;
> > > > >  	}
> > > > >  
> > > > > -	printf("%s%"PRIu64"\t%"PRIu64"\t%s\t%s\n",
> > > > > -	       raw_output ? "" :
> > > > > -	       "Request\tActive\tTotal\tIn\tOut\nClient\t",
> > > > > -	       stat.r.gway_active_nr, stat.r.gway_total_nr,
> > > > > -	       strnumber(stat.r.gway_total_rx),
> > > > > -	       strnumber(stat.r.gway_total_tx));
> > > > > -	printf("%s%"PRIu64"\t%"PRIu64"\t%s\t%s\n",
> > > > > -	       raw_output ? "" : "Peer\t",
> > > > > -	       stat.r.peer_active_nr, stat.r.peer_total_nr,
> > > > > -	       strnumber(stat.r.peer_total_rx),
> > > > > -	       strnumber(stat.r.peer_total_tx));
> > > > > -	if (i > 0) {
> > > > > -		clear_screen();
> > > > 
> > > > Why did you remove clear_screen()? I think it is useful when user
> > > > invokes dog node stat with -w option.
> > > 
> > > Its because most of linux tools like iostat, dstat, vmstat..etc.. they all print
> > > new lines after old ones. I just respect this tradition and the old value is
> > > sometimes really useful.
> > 
> > If we follow the convention, we should print the header line (Request
> > Active	Total...) only once.
> > 
> > Other part looks good to me.
> 
> That can be a future work and harder than simply removing the header because
> after the header is out of screen, then we should print it again. This need more
> code and should be a future patch.


OK,
Reviewed-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>

Thanks,
Hitoshi