[tarantool-patches] Re: [PATCH v2 3/4] session: store vtab in struct session

[Vladislav Shpilevoy]

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