[sheepdog] [PATCH v3 1/2] sheep: enhance STAT_RECOVERY for prividing information of recovery state

[Hitoshi Mitake]

At Fri, 2 Aug 2013 16:28:54 +0800,
Liu Yuan wrote:
> 
> On Fri, Aug 02, 2013 at 04:52:05PM +0900, Hitoshi Mitake wrote:
> > Current SD_OP_STAT_RECOVERY only provides an information that
> > indicates the node is doing recovery or not. For providing more useful
> > information about progress of recovery, this patch enhances this
> > request.
> > 
> > The enhanced SD_OP_STAT_RECOVERY returns the information with struct
> > recovery_state. The struct contains these information:
> > 1. a flag which indicates a sheep process is in progress or not
> > 2. state of recovery. With this information, collie can know which
> >    phase of progress sheep is in.
> > 3. Number of copied object during a recovery process.
> > 4. Number of entire objects which should be copied during a recovery
> >    process.
> > 
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > ---
> > v3:
> >  - make struct recovery_state a general mechanism for getting recovery
> >    status. Ordinal "collie node recovery" uses struct recovery_state
> >    for detecting recovery state instead of a result of request.
> > 
> > v2:
> >  - don't use new variables for indicating the progress
> >  - clean coding style
> >  -- change names of struct recovery_info's members
> >  -- fill_recovery_progress() -> get_recovery_progress()
> > 
> >  collie/node.c            |    8 +++++---
> >  include/internal_proto.h |   13 +++++++++++++
> >  sheep/ops.c              |    4 ++--
> >  sheep/recovery.c         |   22 ++++++++++++++++------
> >  sheep/sheep_priv.h       |    1 +
> >  5 files changed, 37 insertions(+), 11 deletions(-)
> > 
> > diff --git a/collie/node.c b/collie/node.c
> > index 69229f4..4230af5 100644
> > --- a/collie/node.c
> > +++ b/collie/node.c
> > @@ -132,17 +132,19 @@ static int node_recovery(int argc, char **argv)
> >  	for (i = 0; i < sd_nodes_nr; i++) {
> >  		char host[128];
> >  		struct sd_req req;
> > -		struct sd_rsp *rsp = (struct sd_rsp *)&req;
> > +		struct recovery_state state;
> >  
> > +		memset(&state, 0, sizeof(state));
> >  		addr_to_str(host, sizeof(host), sd_nodes[i].nid.addr, 0);
> >  
> >  		sd_init_req(&req, SD_OP_STAT_RECOVERY);
> > +		req.data_length = sizeof(state);
> >  
> > -		ret = collie_exec_req(host, sd_nodes[i].nid.port, &req, NULL);
> > +		ret = collie_exec_req(host, sd_nodes[i].nid.port, &req, &state);
> >  		if (ret < 0)
> >  			return EXIT_SYSFAIL;
> >  
> > -		if (rsp->result == SD_RES_NODE_IN_RECOVERY) {
> > +		if (state.in_recovery) {
> >  			addr_to_str(host, sizeof(host),
> >  					sd_nodes[i].nid.addr, sd_nodes[i].nid.port);
> >  			printf(raw_output ? "%d %s %d %d\n" : "%4d   %-20s%5d%11d\n",
> > diff --git a/include/internal_proto.h b/include/internal_proto.h
> > index 0061007..e42cd1b 100644
> > --- a/include/internal_proto.h
> > +++ b/include/internal_proto.h
> > @@ -192,4 +192,17 @@ static inline __attribute__((used)) void __sd_epoch_format_build_bug_ons(void)
> >  	BUILD_BUG_ON(sizeof(struct sd_node) != SD_NODE_SIZE);
> >  }
> >  
> > +enum rw_state {
> > +	RW_PREPARE_LIST, /* the recovery thread is preparing object list */
> > +	RW_RECOVER_OBJ, /* the thread is recoering objects */
> > +	RW_NOTIFY_COMPLETION, /* the thread is notifying recovery completion */
> > +};
> > +
> > +struct recovery_state {
> > +	uint32_t in_recovery;
> > +	enum rw_state state;
> > +	uint32_t nr_finished;
> > +	uint32_t nr_total;
> > +};
> > +
> >  #endif /* __INTERNAL_PROTO_H__ */
> > diff --git a/sheep/ops.c b/sheep/ops.c
> > index 5d7686c..c25aead 100644
> > --- a/sheep/ops.c
> > +++ b/sheep/ops.c
> > @@ -390,8 +390,8 @@ static int local_stat_sheep(struct request *req)
> >  static int local_stat_recovery(const struct sd_req *req, struct sd_rsp *rsp,
> >  					void *data)
> >  {
> > -	if (node_in_recovery())
> > -		return SD_RES_NODE_IN_RECOVERY;
> > +	get_recovery_state(data);
> > +	rsp->data_length = sizeof(struct recovery_state);
> >  
> >  	return SD_RES_SUCCESS;
> >  }
> > diff --git a/sheep/recovery.c b/sheep/recovery.c
> > index ea3101a..43598a6 100644
> > --- a/sheep/recovery.c
> > +++ b/sheep/recovery.c
> > @@ -11,12 +11,6 @@
> >  
> >  #include "sheep_priv.h"
> >  
> > -enum rw_state {
> > -	RW_PREPARE_LIST, /* the recovery thread is preparing object list */
> > -	RW_RECOVER_OBJ, /* the thread is recoering objects */
> > -	RW_NOTIFY_COMPLETION, /* the thread is notifying recovery completion */
> > -};
> > -
> >  /* base structure for the recovery thread */
> >  struct recovery_work {
> >  	uint32_t epoch;
> > @@ -882,3 +876,19 @@ static void queue_recovery_work(struct recovery_info *rinfo)
> >  
> >  	queue_work(sys->recovery_wqueue, &rw->work);
> >  }
> > +
> > +void get_recovery_state(struct recovery_state *state)
> > +{
> > +	struct recovery_info *rinfo = main_thread_get(current_rinfo);
> > +
> > +	if (!rinfo) {
> > +		state->in_recovery = false;
> > +		return;
> > +	}
> > +
> > +	state->in_recovery = true;
> 
> ->in_recovery isn't boolean, so assign it 1 or 0 instead of true or false.
> Besides, use uint8_t for .in_recovery is good enough.

Thanks for your pointing. I'll fix it in v4.

Thanks,
Hitoshi