[sheepdog] [PATCH] multi-sheep cluster in one zookeeper support

[Liu Yuan]

On Fri, Feb 27, 2015 at 08:16:05PM +0800, Yu Yang wrote:

Prefix the subject header as zookeeper: xxx or zk: xxx to indicate this is a
zookeeper driver patch.

> An optional "chroot" suffix can be appended to the connection
> string(or hosts list). This will run the client commands while
> interpreting all paths relative to this root.
> Different sheep clusters have their own root znode,
> so one zk can support muilt-sheep cluster.
> If it is not set, the default root is /sheepdog,
> which is compatible with old versions.
> 
> Signed-off-by: Yu Yang <yuyang at cmss.chinamobile.com>
> ---
>  sheep/cluster/zookeeper.c |   49 ++++++++++++++++++++++++++++++++++++++++-----
>  sheep/sheep.c             |    6 ++++--
>  2 files changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/sheep/cluster/zookeeper.c b/sheep/cluster/zookeeper.c
> index e2ee248..5b706a3 100644
> --- a/sheep/cluster/zookeeper.c
> +++ b/sheep/cluster/zookeeper.c
> @@ -28,10 +28,10 @@
>  #define SESSION_TIMEOUT 30000		/* millisecond */
>  
>  #define BASE_ZNODE "/sheepdog"

Would better rename BASE_ZNODE as DEFAULT_BASE

> -#define QUEUE_ZNODE BASE_ZNODE "/queue"
> -#define MEMBER_ZNODE BASE_ZNODE "/member"
> -#define MASTER_ZNODE BASE_ZNODE "/master"
> -#define LOCK_ZNODE BASE_ZNODE "/lock"
> +#define QUEUE_ZNODE "/queue"
> +#define MEMBER_ZNODE "/member"
> +#define MASTER_ZNODE "/master"
> +#define LOCK_ZNODE "/lock"
>  
>  static int zk_timeout = SESSION_TIMEOUT;
>  static int my_master_seq;
> @@ -641,7 +641,6 @@ static inline void build_node_list(void)
>  
>  static int zk_queue_init(void)
>  {
> -	RETURN_IF_ERROR(zk_init_node(BASE_ZNODE), "path %s", BASE_ZNODE);
>  	RETURN_IF_ERROR(zk_init_node(MASTER_ZNODE), "path %s", MASTER_ZNODE);
>  	RETURN_IF_ERROR(zk_init_node(QUEUE_ZNODE), "path %s", QUEUE_ZNODE);
>  	RETURN_IF_ERROR(zk_init_node(MEMBER_ZNODE), "path %s", MEMBER_ZNODE);
> @@ -1378,6 +1377,39 @@ static void zk_unlock(uint64_t lock_id)
>  	sd_debug("unlock %"PRIu64, lock_id);
>  }
>  
> +static int zk_prepare_root(const char *hosts)
> +{
> +	char root[MAX_NODE_STR_LEN];
> +	char conn[MAX_NODE_STR_LEN];
> +	const char *p = strchr(hosts, '/');
> +	int interval, max_retry, retry;
> +	int i = 0;
> +	strcpy(root, p);
> +	while (hosts != p)
> +		conn[i++] = *hosts++;
> +	conn[i] = '\0';
> +
> +	zhandle = zookeeper_init(conn, zk_watcher, zk_timeout, NULL, NULL, 0);
> +	if (!zhandle) {
> +		sd_err("failed to initialize zk server %s", conn);
> +		return -1;
> +	}
> +
> +	interval = 100;
> +	retry = 0;
> +	max_retry = zk_timeout / interval;
> +	while (zoo_state(zhandle) != ZOO_CONNECTED_STATE) {
> +		usleep(interval * 1000);
> +		if (++retry >= max_retry) {
> +			sd_err("failed to connect to zk server %s "
> +					"after %d retries", conn, retry);
> +			return -1;
> +		}
> +	}
> +	RETURN_IF_ERROR(zk_init_node(root), "path %s", root);
> +	return 0;
> +}
> +
>  static int zk_init(const char *option)
>  {
>  	char *hosts, *to, *p;
> @@ -1397,6 +1429,13 @@ static int zk_init(const char *option)
>  		p = strstr(hosts, "timeout");
>  		*--p = '\0';
>  	}
> +	if (!strchr(hosts, '/'))
> +			strcat(hosts, BASE_ZNODE);

Please align the strcat properly.

> +	if (zk_prepare_root(hosts) != 0) {
> +		sd_err("failed to initialize zk server %s", hosts);
> +		return -1;
> +	}
> +
>  	sd_info("version %d.%d.%d, address %s, timeout %d", ZOO_MAJOR_VERSION,
>  		ZOO_MINOR_VERSION, ZOO_PATCH_VERSION, hosts, zk_timeout);
>  	zhandle = zookeeper_init(hosts, zk_watcher, zk_timeout, NULL, NULL, 0);

Since zk_prepare_root already connect to zk server, we don't need to call
zookeeper_init again.

You can make zk_prepare_root return zk handle.

> diff --git a/sheep/sheep.c b/sheep/sheep.c
> index 7d5fa0f..6f0c3ef 100644
> --- a/sheep/sheep.c
> +++ b/sheep/sheep.c
> @@ -70,10 +70,12 @@ static const char cluster_help[] =
>  "\tzookeeper: use zookeeper driver, need extra arguments\n"
>  "\n\tzookeeper arguments: address-list,timeout=value (default as 3000)\n"
>  "\nExample:\n\t"
> -"$ sheep -c zookeeper:IP1:PORT1,IP2:PORT2,IP3:PORT3,timeout=1000 ...\n"
> +"$ sheep -c zookeeper:IP1:PORT1,IP2:PORT2,IP3:PORT3/clustername,timeout=1000 ...\n"

zookeeper:IP1:PORT1,IP2:PORT2,IP3:PORT3[/cluster_id],timeout...
we use [] to indicate that it is optional

>  "This tries to use 3 node zookeeper cluster, which can be reached by\n"
>  "IP1:PORT1, IP2:PORT2, IP3:PORT3 to manage membership and broadcast message\n"
> -"and set the timeout of node heartbeat as 1000 milliseconds\n";
> +"and set the timeout of node heartbeat as 1000 milliseconds.\n"
> +"The suffix /clustername is for multi-sheep cluster support,\n"

How about following:

"/cluster_id is used to identify which cluster it belongs to. If not set, we 
use /sheepdog internally as default"

Thanks,
Yuan