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

[Konstantin Osipov <kostja@tarantool.org>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/25 00:46]:
> 
> 
> 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:
This comment is good but could be even better if it contains
answers to all of the questions, not just how routing is done:

> > - 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.

OK, but when I look at routing itself I want to know how it's
used. Even if this patch does not use the routing, I want a patch
with a comment about the usage. Please feel free to add to the
current comment or submit separately.

> 
> 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.

Please document this decision and the reasons for it as you stated
above.
> >> +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.

Then swim_task_set_proxy(). A method should have a verb in its
name, otherwise the name looks like a constructor.

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