[sheepdog] [PATCH] sheep: rework update node logic
MORITA Kazutaka
morita.kazutaka at gmail.com
Tue Jul 9 02:28:18 CEST 2013
At Sat, 6 Jul 2013 01:23:41 +0800,
Liu Yuan wrote:
>
> @@ -343,6 +346,13 @@ static bool __corosync_dispatch_one(struct corosync_event *cevent)
> sd_notify_handler(&cevent->sender.ent, cevent->msg,
> cevent->msg_len);
> break;
> + case COROSYNC_EVENT_TYPE_UPDATE_NODE:
> + node = &cevent->sender.ent;
> + idx = find_sd_node(cpg_nodes, nr_cpg_nodes, node);
> + assert(idx != 0);
assert(idx >= 0)?
idx can be zero if the node is the first member of cpg_nodes.
Actually, tests/functional 63 and 64 fail on my environment.
> + cpg_nodes[idx].ent = *node;
> + sd_update_node_handler(node);
> + break;
> }
>
> return true;
> @@ -806,12 +824,9 @@ again:
>
> static void corosync_update_node(struct sd_node *node)
> {
> - int idx;
> -
> - idx = find_sd_node(cpg_nodes, nr_cpg_nodes, node);
> - if (idx < 0)
> - return;
> - cpg_nodes[idx].ent = *node;
> + this_node.ent = *node;
For all the nodes to see the same node information, we shouldn't
update this_node here but in __corosync_dispatch_one().
> + send_message(COROSYNC_MSG_TYPE_UPDATE_NODE, 0, &this_node,
> + NULL, 0, NULL, 0);
send_message() can fail. Return the result to the caller.
> }
>
> static struct cluster_driver cdrv_corosync = {
> diff --git a/sheep/cluster/local.c b/sheep/cluster/local.c
> index 307a69e..2c5d197 100644
> --- a/sheep/cluster/local.c
> +++ b/sheep/cluster/local.c
> @@ -474,6 +474,7 @@ static bool local_process_event(void)
> sd_notify_handler(&ev->sender.node, ev->buf, ev->buf_len);
> break;
> case EVENT_UPDATE_NODE:
> + sd_update_node_handler(&ev->sender.node);
> break;
> }
> out:
> @@ -551,16 +552,12 @@ static int local_init(const char *option)
> return 0;
> }
>
> -/* FIXME: we have to call nr of nodes times to update nodes information */
> static void local_update_node(struct sd_node *node)
> {
> - struct local_node n = {
> - .node = *node,
> - };
> -
> + this_node.node = *node;
Update this_node in local_process_event().
> shm_queue_lock();
>
> - add_event(EVENT_UPDATE_NODE, &n, NULL, 0);
> + add_event(EVENT_UPDATE_NODE, &this_node, NULL, 0);
>
> shm_queue_unlock();
> }
> diff --git a/sheep/cluster/shepherd.c b/sheep/cluster/shepherd.c
> index fba329c..e0aa6ea 100644
> --- a/sheep/cluster/shepherd.c
> +++ b/sheep/cluster/shepherd.c
> @@ -664,8 +664,7 @@ static void shepherd_unblock(void *msg, size_t msg_len)
> /* FIXME: shepherd server also has to udpate node information */
> static void shepherd_update_node(struct sd_node *node)
> {
> - struct sd_node *n = xlfind(node, nodes, nr_nodes, node_cmp);
> - *n = *node;
> + panic("not implemented");
panic is not good at all. Please at least return SD_RES_NO_SUPPORT to
the caller.
> }
>
> static struct cluster_driver cdrv_shepherd = {
> diff --git a/sheep/cluster/zookeeper.c b/sheep/cluster/zookeeper.c
> index 0feb8cf..6c40b5f 100644
> --- a/sheep/cluster/zookeeper.c
> +++ b/sheep/cluster/zookeeper.c
> @@ -47,6 +47,7 @@ enum zk_event_type {
> EVENT_BLOCK,
> EVENT_UNBLOCK,
> EVENT_NOTIFY,
> + EVENT_UPDATE_NODE,
> };
>
> struct zk_node {
> @@ -993,6 +994,21 @@ static void zk_handle_notify(struct zk_event *ev)
> sd_notify_handler(&ev->sender.node, ev->buf, ev->buf_len);
> }
>
> +static void zk_handle_update_node(struct zk_event *ev)
> +{
> + struct zk_node *t, *n = &ev->sender;
> +
> + sd_dprintf("%s", node_to_str(&n->node));
> +
> + pthread_rwlock_rdlock(&zk_tree_lock);
> + t = zk_tree_search_nolock(&n->node.nid);
> + assert(t);
> + t->node = n->node;
> + build_node_list();
> + pthread_rwlock_unlock(&zk_tree_lock);
> + sd_update_node_handler(&n->node);
> +}
> +
> static void (*const zk_event_handlers[])(struct zk_event *ev) = {
> [EVENT_JOIN_REQUEST] = zk_handle_join_request,
> [EVENT_JOIN_RESPONSE] = zk_handle_join_response,
> @@ -1000,6 +1016,7 @@ static void (*const zk_event_handlers[])(struct zk_event *ev) = {
> [EVENT_BLOCK] = zk_handle_block,
> [EVENT_UNBLOCK] = zk_handle_unblock,
> [EVENT_NOTIFY] = zk_handle_notify,
> + [EVENT_UPDATE_NODE] = zk_handle_update_node,
> };
>
> static const int zk_max_event_handlers = ARRAY_SIZE(zk_event_handlers);
> @@ -1135,20 +1152,8 @@ static int zk_init(const char *option)
>
> static void zk_update_node(struct sd_node *node)
> {
> - struct zk_node n = {
> - .node = *node,
> - };
> - struct zk_node *t;
> -
> - sd_dprintf("%s", node_to_str(&n.node));
> -
> - pthread_rwlock_rdlock(&zk_tree_lock);
> - t = zk_tree_search_nolock(&n.node.nid);
> - if (t) {
> - t->node = n.node;
> - build_node_list();
> - }
> - pthread_rwlock_unlock(&zk_tree_lock);
> + this_node.node = *node,
Update this_node in zk_handle_update_node().
> + add_event(EVENT_UPDATE_NODE, &this_node, NULL, 0);
> }
>
> static struct cluster_driver cdrv_zookeeper = {
> diff --git a/sheep/group.c b/sheep/group.c
> index 6e01a8d..73a9ed4 100644
> --- a/sheep/group.c
> +++ b/sheep/group.c
> @@ -985,16 +985,15 @@ void sd_leave_handler(const struct sd_node *left, const struct sd_node *members,
> sockfd_cache_del(&left->nid);
> }
>
> -void update_node_size(struct sd_node *node)
> +static void update_node_size(struct sd_node *node)
> {
> struct vnode_info *cur_vinfo = main_thread_get(current_vnode_info);
> int idx = get_node_idx(cur_vinfo, node);
> assert(idx != -1);
> cur_vinfo->nodes[idx].space = node->space;
> - sys->cdrv->update_node(node);
> }
>
> -void kick_node_recover(void)
> +static void kick_node_recover(void)
> {
> struct vnode_info *old = main_thread_get(current_vnode_info);
>
> @@ -1006,6 +1005,12 @@ void kick_node_recover(void)
> put_vnode_info(old);
> }
>
> +void sd_update_node_handler(struct sd_node *node)
> +{
> + update_node_size(node);
> + kick_node_recover();
> +}
> +
> int create_cluster(int port, int64_t zone, int nr_vnodes,
> bool explicit_addr)
> {
> diff --git a/sheep/ops.c b/sheep/ops.c
> index bf975bf..49630a9 100644
> --- a/sheep/ops.c
> +++ b/sheep/ops.c
> @@ -662,33 +662,9 @@ static int cluster_recovery_completion(const struct sd_req *req,
> return SD_RES_SUCCESS;
> }
>
> -static void do_reweight(struct work *work)
> +static void reweight_this_node(void)
> {
> - struct sd_req hdr;
> - int ret;
> -
> - sd_init_req(&hdr, SD_OP_UPDATE_SIZE);
> - hdr.flags = SD_FLAG_CMD_WRITE;
> - hdr.data_length = sizeof(sys->this_node);
> -
> - ret = exec_local_req(&hdr, &sys->this_node);
> - if (ret != SD_RES_SUCCESS)
> - sd_eprintf("failed to update node size");
> -}
> -
> -static void reweight_done(struct work *work)
> -{
> - free(work);
> -}
> -
> -static void reweight_node(void)
> -{
> - struct work *rw = xzalloc(sizeof(*rw));
> -
> - rw->fn = do_reweight;
> - rw->done = reweight_done;
> -
> - queue_work(sys->recovery_wqueue, rw);
> + sys->cdrv->update_node(&sys->this_node);
> }
>
> static bool node_size_varied(void)
> @@ -724,18 +700,7 @@ static int cluster_reweight(const struct sd_req *req, struct sd_rsp *rsp,
> void *data)
> {
> if (node_size_varied())
> - reweight_node();
> -
> - return SD_RES_SUCCESS;
> -}
> -
> -static int cluster_update_size(const struct sd_req *req, struct sd_rsp *rsp,
> - void *data)
> -{
> - struct sd_node *node = (struct sd_node *)data;
> -
> - update_node_size(node);
> - kick_node_recover();
> + reweight_this_node();
Calling sys->cdrv->update_node() directly looks simpler and easier to
read.
Thanks,
Kazutaka
More information about the sheepdog
mailing list