[sheepdog] [PATCH 2/2] sheep: cleanup prepare_schedule_oid and oid_in_recovery

[Liu Yuan]

On Fri, Aug 30, 2013 at 11:24:32PM +0900, MORITA Kazutaka wrote:
> From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> 
> This moves all the check of recovering objects from
> prepare_schedule_oid() to oid_in_recovery(), and makes the code easier
> to read.
> 
> This also fixes a problem that sheep doesn't recover an object when
>  - sys->cinfo.disable_recovery is true,
>  - rinfo->state is RW_PREPARE_LIST, and
>  - oid_in_recovery() is called against the object
> and fixes an error of tests/functional/010, which happens in rare
> cases.
> 
> Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> ---
>  include/sheep.h  |    5 ++++
>  sheep/recovery.c |   77 ++++++++++++++++++++++++++++++------------------------
>  2 files changed, 48 insertions(+), 34 deletions(-)
> 
> diff --git a/include/sheep.h b/include/sheep.h
> index 68c852f..f9945c6 100644
> --- a/include/sheep.h
> +++ b/include/sheep.h
> @@ -251,6 +251,11 @@ static inline const char *sd_strerror(int err)
>  	return descs[err];
>  }
>  
> +static inline int oid_cmp(const uint64_t *oid1, const uint64_t *oid2)
> +{
> +	return intcmp(*oid1, *oid2);
> +}
> +
>  static inline int node_id_cmp(const struct node_id *node1,
>  			      const struct node_id *node2)
>  {
> diff --git a/sheep/recovery.c b/sheep/recovery.c
> index 6faaeab..4305e48 100644
> --- a/sheep/recovery.c
> +++ b/sheep/recovery.c
> @@ -331,24 +331,11 @@ bool node_in_recovery(void)
>  static inline void prepare_schedule_oid(uint64_t oid)
>  {
>  	struct recovery_info *rinfo = main_thread_get(current_rinfo);
> -	uint64_t i;
>  
> -	for (i = 0; i < rinfo->nr_prio_oids; i++)
> -		if (rinfo->prio_oids[i] == oid)
> -			return;
> -	/*
> -	 * We need this check because oid might not be recovered.
> -	 * Very much unlikely though, but it might happen indeed.
> -	 */
> -	for (i = 0; i < rinfo->done; i++)
> -		if (rinfo->oids[i] == oid) {
> -			sd_debug("%"PRIx64" not recovered, don't schedule it",
> -				 oid);
> -			return;
> -		}

This check is removed entirely? I don't remember exactly how this check was
introduced though.

> -	/* When recovery is not suspended, oid is currently being recovered */
> -	if (!rinfo->suspended && rinfo->oids[rinfo->done] == oid)
> +	if (xlfind(&oid, rinfo->prio_oids, rinfo->nr_prio_oids, oid_cmp)) {
> +		sd_debug("%" PRIx64 " has been already in prio_oids", oid);
>  		return;
> +	}

is this correct? prio_oids are never sorted.

Thanks
Yuan