[tarantool-patches] Re: [PATCH 5/6] swim: introduce routing

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 24/04/2019 19:46, Konstantin Osipov wrote:
> * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/24 18:50]:
> 
> there is a couple of things I don't understand:
> 
> - how do you make a decision that you need to use a proxy? is it
>   random? is it if there are too many unacknowledged pings?

First of all this patch does not use proxies at all. It just introduces
new section SWIM_META_ROUTING.

Answering your question - the second option. The last patch, which
introduces suspicion, uses proxy when too many direct pings are
unacked.

> 
> - how do you make a decision which proxy to use? Do you choose a
>   single peer or all available peers when you need to send a
>   message via a proxy?

I choose multiple random proxies as the SWIM paper says.

> 
> - how is the result returned to the sender?

I send an answer via the same proxy.

>   As far as I can see
>   from the route section, there is only source and destination
>   addresses, but it doesn't contain entire routing path.

In fact it does. Meta contains SWIM_META_ROUTING +
{SWIM_META_SRC_PORT, SWIM_META_SRC_ADDRESS}. It is 3 ip/port pairs.

SWIM_META_SRC_PORT/ADDRESS is the message sender, always.

SWIM_META_ROUING contains the message originator (it is not
always sender), and the destination.

Let's consider an example. There are 3 nodes S1, S2, S3. S1
sends a ping to S3 via S2.

S1 -> S2
{
    route: {src: S1, dst: S3},
    src: S1
}

S2 -> S3
{
    route: {src: S1, dst: S3},
    src: S2
}

S3 -> S2
{
    route: {src: S3, dst: S1},
    src: S3
}

S2 -> S1
{
    route: {src: S3, dst: S1},
    src: S2
}


As you can see, 'src' is set to real sender
on each hop. 'route' is unchanged until the
message reaches the end.

'src' is used by S3 to send the response back
via the same proxy.

>   Do you
>   assume there is always only one intermediate hop in case of an
>   indirect ping and always respond to the direct sender, assuming
>   the sender has a direct connection to the originator?

Yes, I send an indirect message only via one intermediate member,
as the SWIM paper says. And respond by the same route.

> 
> Please document answers to these questions in the code, it
> constitutes the mechanics of indirect pings.

Most of your questions are answered in comments inside the
next commit. But you fairly noticed that it is worth
documenting that only one additional hop is possible. Arbitrary
routes of any length are not implemented. I thought about it, but
decided it would be ultra overkill for such a simple task.

A new comment:

===================================================================
diff --git a/src/lib/swim/swim_proto.h b/src/lib/swim/swim_proto.h
index 045e55415..f70ac708a 100644
--- a/src/lib/swim/swim_proto.h
+++ b/src/lib/swim/swim_proto.h
@@ -427,6 +427,32 @@ enum swim_meta_key {
 	 */
 	SWIM_META_SRC_ADDRESS,
 	SWIM_META_SRC_PORT,
+	/**
+	 * Routing section allows to specify routes of up to 3
+	 * nodes: source, proxy, and destination. It contains two
+	 * addresses - the message originator and the final
+	 * destination. Here is an example of an indirect message
+	 * transmission. Assume, there are 3 nodes: S1, S2, S3.
+	 * S1 sends a message to S3 via S2. The following steps
+	 * are executed in order to deliver the message:
+	 *
+	 * S1 -> S2
+	 * { src: S1, routing: {src: S1, dst: S3}, body: ... }
+	 *
+	 * S2 receives the message and sees: routing.dst != S2 -
+	 * it is a foreign packet. S2 forwards it to S3 preserving
+	 * all the data - body and routing sections.
+	 *
+	 *
+	 * S2 -> S3
+	 * {src: S2, routing: {src: S1, dst: S3}, body: ...}
+	 *
+	 * S3 receives the message and sees: routing.dst == S3 -
+	 * the message is delivered. If S3 wants to answer, it
+	 * sends a response via the same proxy. It knows, that the
+	 * message was delivered from S2, and sends an answer via
+	 * S2.
+	 */
 	SWIM_META_ROUTING,
 };
===================================================================

> 
>> +void
>> +swim_task_proxy(struct swim_task *task, const struct sockaddr_in *proxy)
> 
> Why not "swim_task_set_proxy_addr"?

1) Because the transport level operates on addresses only. It is excessive
to say everywhere '_addr' suffix. It is impossible to send to UUID or
anything else except inet address;

2) For consistency - 'swim_task_send()' is not named
'swim_task_send_to_addr()';

3) '_addr' suffix is too long. I tried it on the first SWIM version,
but it does not help at all, only pads the code out.