From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Subject: Re: [tarantool-patches] Re: [PATCH v2 3/4] session: store vtab in
 struct session
References: <cover.1544544183.git.v.shpilevoy@tarantool.org>
 <712b6f1f052126065eac6ca301221001c180906b.1544544183.git.v.shpilevoy@tarantool.org>
 <20181221110456.7gavqguwgxx7ijoh@esperanza>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <95de280a-429a-7aad-6381-81e88ce11c04@tarantool.org>
Date: Fri, 21 Dec 2018 14:23:55 +0300
MIME-Version: 1.0
In-Reply-To: <20181221110456.7gavqguwgxx7ijoh@esperanza>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>



On 21/12/2018 14:04, Vladimir Davydov wrote:
> On Tue, Dec 11, 2018 at 07:10:22PM +0300, Vladislav Shpilevoy wrote:
>> @@ -587,7 +587,8 @@ applier_f(va_list ap)
>>   	 * Set correct session type for use in on_replace()
>>   	 * triggers.
>>   	 */
>> -	if (session_create_on_demand(SESSION_TYPE_APPLIER) == NULL) {
>> +	if (session_create_on_demand(SESSION_TYPE_APPLIER,
>> +				     &generic_session_vtab) == NULL) {
>>   		diag_log();
>>   		return -1;
>>   	}
> 
>> @@ -51,12 +52,12 @@ lbox_session_create(struct lua_State *L, enum session_type type)
>>   {
>>   	struct session *session = fiber_get_session(fiber());
>>   	if (session == NULL) {
>> -		session = session_create_on_demand(type);
>> +		session = session_create_on_demand(type, &console_session_vtab);
>>   		if (session == NULL)
>>   			return luaT_error(L);
>>   		session->meta.fd = lua_tointeger(L, 1);
>>   	} else {
>> -		session_set_type(session, type);
>> +		session_set_type(session, type, &console_session_vtab);
>>   	}
> 
> IMHO the new API is confusing, because it allows to do e.g.
> 
>    session_set_type(SESSION_TYPE_APPLIER, &console_session_vtab);
> 
> What about leaving the session vtab registry and using it in
> session_set_type to set session->vtab consistent with a given type?

I do not like having both session->vtab and session_registry. Also,
Kostja asked to delete session_registry. But as you wish.

> Then, to kill a session we would introduce a separate method, which
> would reset session->vtab. And we could add an assertion ensuring that
> session_set_type isn't called on a dead session.
>