Tarantool development patches archive
 help / color / mirror / Atom feed
From: lvasiliev <lvasiliev@tarantool.org>
To: Konstantin Osipov <kostja.osipov@gmail.com>,
	alexander.turenko@tarantool.org,
	tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH 3/6] iproto: Add negotiation phase
Date: Thu, 26 Mar 2020 14:18:04 +0300	[thread overview]
Message-ID: <09a887d4-7459-683b-8a10-c3a0d27bc8c3@tarantool.org> (raw)
In-Reply-To: <20200325084205.GG18984@atlas>

Hi! Еhanks for the feedback.

On 25.03.2020 11:42, Konstantin Osipov wrote:
> * lvasiliev <lvasiliev@tarantool.org> [20/03/25 10:36]:
>>> Why not make it a key in IPROTO_AUTH, and require a separate
>>> round-trip?
>> Hi. Because it's not a part of AAA.
> 
> 1. Auth packet is a session control packet. Authentication
> is only one thing you may want to do with the session, you may
> want to simply switch between already authenticated users.
Now it does't look like a session control packet because auth packet 
will be sent to server only if authentication is needed ("Authentication 
in Tarantool is optional, if no authentication is performed, session 
user is ‘guest’"). But, theoretically, it can be used.
In this case, the answer must be changed from ok/fail to the answer with 
payload.
In my opinion the negotiation looks as:
- the client offers options for the session
- the server sends the resulting response with options (which may differ 
from the requested)
- the client decides to work with such settings or disconnect
I think that negotiation phase can be used for flexible session setup in 
the future (not only for errors)
> 
> 2. Adding an extra package adds an extra round-trip and slows down each
> handshake by at least 1 millisecond.
Yes, but it used only if you don't want use default settings of the session.
> 
> 3. Auth package is synchronous by default. Any other package is
>     not, it's executed in async mode. Meaning the semantics of the
>     new package is broken, it is unclear what responses it affects
>     - those that happen to be not sent yet when this package is
>       handled, but not necessarily those that started processing
>       after this package arrived.
I agree that this operation should be synchronous.
As I understand remote:request from the net_box.lua will wait for a 
response. Is it incorrectly?
> 
>>> Why have it at all and not look at server version, which is part
>>> of the greeting already?
>>>
>> The cause is backward compatibility.
>> For example: a client application may expect an error as a string (IPROTO_OK
>> case) and instead of which it will receive an error as an “object”. A
>> greeting is sent only from the server side to the client, but the server
>> must know what format should be used to send errors (what format does the
>> client expect).
> 
> A much simpler way to do it is to have a server switch to enable
> new features.
> It is less flexible, of course, because you can't have old and new
> clients, but do you really want to have old and new clients?
> 
It's not about what I want, it's about what is possible and backward 
compatibility. I think this is a realistic situation.

  parent reply	other threads:[~2020-03-26 11:18 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24 12:45 [Tarantool-patches] [PATCH 0/6] Extending error functionality Leonid Vasiliev
2020-03-24 12:45 ` [Tarantool-patches] [PATCH 1/6] error: Add a Lua backtrace to error Leonid Vasiliev
2020-04-05 22:14   ` Vladislav Shpilevoy
2020-04-08 13:56     ` Igor Munkin
2020-03-24 12:46 ` [Tarantool-patches] [PATCH 2/6] error: Add the custom error type Leonid Vasiliev
2020-04-05 22:14   ` Vladislav Shpilevoy
2020-03-24 12:46 ` [Tarantool-patches] [PATCH 3/6] iproto: Add negotiation phase Leonid Vasiliev
2020-03-24 20:02   ` Konstantin Osipov
2020-03-25  7:35     ` lvasiliev
2020-03-25  8:42       ` Konstantin Osipov
2020-03-25 10:56         ` Eugene Leonovich
2020-03-25 11:13           ` Konstantin Osipov
2020-03-26 11:37           ` lvasiliev
2020-03-26 11:18         ` lvasiliev [this message]
2020-03-26 12:16           ` Konstantin Osipov
2020-03-26 12:54             ` Kirill Yukhin
2020-03-26 13:19               ` Konstantin Osipov
2020-03-26 13:31                 ` Konstantin Osipov
2020-03-26 21:13       ` Alexander Turenko
2020-03-26 21:53         ` Alexander Turenko
2020-03-27  8:28         ` Konstantin Osipov
2020-03-26 23:35       ` Alexander Turenko
2020-03-27  8:39         ` Konstantin Osipov
2020-03-24 12:46 ` [Tarantool-patches] [PATCH 4/6] error: Add extended error transfer format Leonid Vasiliev
2020-03-24 12:46 ` [Tarantool-patches] [PATCH 5/6] error: Add test for extended error Leonid Vasiliev
2020-03-24 12:46 ` [Tarantool-patches] [PATCH 6/6] error: Transmit an error through IPROTO_OK as object Leonid Vasiliev
2020-03-27 23:11 ` [Tarantool-patches] [PATCH 0/6] Extending error functionality lvasiliev
2020-03-28 13:54   ` Alexander Turenko
2020-03-30 10:48     ` lvasiliev
2020-04-01 15:35 ` Alexander Turenko

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=09a887d4-7459-683b-8a10-c3a0d27bc8c3@tarantool.org \
    --to=lvasiliev@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=kostja.osipov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 3/6] iproto: Add negotiation phase' \
    /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