[Tarantool-patches] [PATCH 3/6] iproto: Add negotiation phase

[lvasiliev]

Hi! Еhanks for the feedback.

On 25.03.2020 11:42, Konstantin Osipov wrote:
> * lvasiliev <lvasiliev at 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.