[sheepdog] [PATCH v6 6/6] zookeeper: handle session timeout for all zookeeper operations

[Liu Yuan]

On 06/21/2013 08:34 PM, Kai Zhang wrote:
> @@ -976,18 +1071,34 @@ static void zk_event_handler(int listen_fd, int events, void *data)
>  		return;
>  	}
>  
> -	if (!zk_queue_peek())
> -		goto kick_block_event;
> +	if (zk_queue_peek(&peek) == ZOK) {
> +		if (!peek)
> +			goto kick_block_event;
> +	} else {
> +		sd_eprintf("failed");

This sd_eprintf() isn't necessary. You just return if timeout, but you
don't handle it. This means some other nodes call event handler, but
some nodes don't, no? If we can't handle failure gracefully, we still
need panic(). This applies to push_join_response() too.

You seem to assume to handle session timeout in *next* event, but what
if following case:
  1 get one session timeout for some operation, we simply return
  2 never trigger a zk event operation, so we don't have chance to call
reconnect.

Why not try reconnect whenever we find a session timeout immediately?

Thanks,
Yuan

> +		return;
> +	}
>  
> -	zk_queue_pop_advance(&ev);
> +	if (zk_queue_pop_advance(&ev) != ZOK) {
> +		sd_eprintf("failed");
> +		return;
> +	}
>  	if (ev.type < zk_max_event_handlers && zk_event_handlers[ev.type])
>  		zk_event_handlers[ev.type](&ev);
>  	else
>  		panic("unhandled type %d", ev.type);
>  
> -	 /* Someone has created next event, go kick event handler. */
> -	if (zk_queue_peek()) {
> -		eventfd_write(efd, 1);
> +	if (zk_queue_peek(&peek) == ZOK) {
> +		if (peek) {
> +			/*
> +			 * Someone has created next event,
> +			 * go kick event handler.
> +			 */
> +			eventfd_write(efd, 1);
> +			return;
> +		}
> +	} else {
> +		sd_eprintf("failed");
>  		return;
>  	}