Tarantool development patches archive
 help / color / mirror / Atom feed
From: Konstantin Osipov <kostja@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 6/6] swim: introduce suspicion
Date: Wed, 24 Apr 2019 20:01:01 +0300	[thread overview]
Message-ID: <20190424170101.GF13687@atlas> (raw)
In-Reply-To: <0810d2237700cc59a23559f083cd238c1a88d8ec.1556116199.git.v.shpilevoy@tarantool.org>

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/24 18:50]:
> +	/**
> +	 * Number of attempts to reach out a member via another
> +	 * members when it did not answer on a regular ping .
> +	 */
> +	INDIRECT_PING_COUNT = 2,

As far as I can see it's not the number of attempts, it's the
number of cluster members involved as proxies for sending an
indirect ping. The fact that you attempt an indirect ping only
once before switching member state to dead is secondary.

This has to do with lack of a description in changeset or code
comment of indirect pings strategy. 

> +/**
> + * Put the member into a list of ACK waiters. @a hop_count says
> + * how many hops from one member to another the ACK is expected to
> + * do.
> + */
>  static void
> -swim_wait_ack(struct swim *swim, struct swim_member *member)
> +swim_wait_ack(struct swim *swim, struct swim_member *member, int hop_count)
>  {
>  	if (heap_node_is_stray(&member->in_wait_ack_heap)) {
> -		member->ping_deadline = swim_time() + swim->wait_ack_tick.at;
> +		double timeout = swim->wait_ack_tick.at * hop_count;

Is hop_count ever greater than two? If not, I would not use int
hop_count, but use bool is_indirect_ping, and add a comment
explaining that in case of indirect pings the number of hops is
exactly two, so we need to increase the timeout.

> +/**
> + * When indirect pings are sent, each is represented by this
> + * structure.
> + */
> +struct swim_iping_task {
> +	/** Base task used by the scheduler. */
> +	struct swim_task base;
> +	/**
> +	 * Reference to a block of other indirect ping tasks sent
> +	 * at the same moment. Used to decide whether it is needed
> +	 * to start waiting for an ACK, and on which member.
> +	 */
> +	struct swim_iping_block *block;

I don't understand this comment, why do you need a reference?

I would not use it at all, seems like we have a separate data
structure when existing structures would do just fine.


> +};
> +
> +/**
> + * An array of indirect ping tasks sent simultaneously via
> + * different proxies. The block contains meta information allowing
> + * to 1) start waiting for an ACK only after first successful
> + * send; 2) determine which member has sent the pings - UUID is
> + * needed for that, inet address is not enough.
> + *
> + * The block is deleted when the last task is finished.
> + */
> +struct swim_iping_block {
> +	/** Array of indirect ping tasks. */
> +	struct swim_iping_task tasks[INDIRECT_PING_COUNT];
> +	/**
> +	 * UUID of the destination member. Used to set ping
> +	 * deadline in that member when at least one is sent
> +	 * successfully.
> +	 */
> +	struct tt_uuid dst_uuid;
> +	/**
> +	 * The flag is used to wait for an ACK only once after a
> +	 * first ping is sent to protect from a case, when an ACK
> +	 * is received faster, than the last ping is sent. Then
> +	 * the whole indirect ping block should be considered
> +	 * acked.
> +	 */

Why care if indirect ping block is acknowledged or not? What
matters is member status, and it is changed when we get an ack.
In other words, I'd drop the whole idea with blocks and just send
multiple independent pings.

>  	switch (def.type) {
>  	case SWIM_FD_MSG_PING:
> -		if (! swim_task_is_scheduled(&member->ack_task))
> +		if (proxy != NULL) {
> +			if (swim_send_indirect_ack(swim, &member->addr,
> +						   proxy) != 0)
> +				diag_log();
> +		} else if (! swim_task_is_scheduled(&member->ack_task)) {
>  			swim_send_ack(swim, &member->ack_task, &member->addr);
> +		}
>  		break;

Why do you ever need to distinguish direct and indirect acks? Why not
respond to the sender address, while preserving the routing table
from the ping request, assuming the sender can figure out whether
the response is addressed to him directly or it should proxy it
further based on the received proxy section of the response?

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

  reply	other threads:[~2019-04-24 17:01 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-24 14:36 [tarantool-patches] [PATCH 0/6] swim suspicion Vladislav Shpilevoy
2019-04-24 14:36 ` [tarantool-patches] [PATCH 1/6] test: rename swim_cluster_node to swim_cluster_member Vladislav Shpilevoy
2019-04-24 16:37   ` [tarantool-patches] " Konstantin Osipov
2019-04-24 14:36 ` [tarantool-patches] [PATCH 2/6] test: remove swim packet filter destructors Vladislav Shpilevoy
2019-04-24 16:37   ` [tarantool-patches] " Konstantin Osipov
2019-04-24 14:36 ` [tarantool-patches] [PATCH 3/6] test: introduce swim packet filter by destination address Vladislav Shpilevoy
2019-04-24 16:38   ` [tarantool-patches] " Konstantin Osipov
2019-04-24 14:36 ` [tarantool-patches] [PATCH 4/6] swim: wrap sio_strfaddr() Vladislav Shpilevoy
2019-04-24 16:40   ` [tarantool-patches] " Konstantin Osipov
2019-04-24 20:23     ` Vladislav Shpilevoy
2019-04-25 10:34       ` Konstantin Osipov
2019-04-25 13:50         ` Vladislav Shpilevoy
2019-04-24 14:36 ` [tarantool-patches] [PATCH 5/6] swim: introduce routing Vladislav Shpilevoy
2019-04-24 16:46   ` [tarantool-patches] " Konstantin Osipov
2019-04-24 20:25     ` Vladislav Shpilevoy
2019-04-25 10:39       ` Konstantin Osipov
2019-04-25 13:50         ` Vladislav Shpilevoy
2019-04-25 13:57           ` Konstantin Osipov
2019-04-24 14:36 ` [tarantool-patches] [PATCH 6/6] swim: introduce suspicion Vladislav Shpilevoy
2019-04-24 17:01   ` Konstantin Osipov [this message]
2019-04-24 20:28     ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-25 10:42       ` Konstantin Osipov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190424170101.GF13687@atlas \
    --to=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH 6/6] swim: introduce suspicion' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox