Re: [Tarantool-patches] [PATCH 4/4] txn_limbo: add ROLLBACK processing

[Serge Petrenko <sergepetrenko@tarantool.org>]

19.06.2020 01:15, Vladislav Shpilevoy пишет:
> Thanks for the patch!

Thanks for the review!

>
>> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
>> index a715a136e..c55f5bda1 100644
>> --- a/src/box/txn_limbo.c
>> +++ b/src/box/txn_limbo.c
>> @@ -84,6 +84,16 @@ txn_limbo_remove(struct txn_limbo *limbo, struct txn_limbo_entry *entry)
>>   	rlist_del_entry(entry, in_queue);
>>   }
>>   
>> +static inline void
>> +txn_limbo_pop(struct txn_limbo *limbo, struct txn_limbo_entry *entry)
>> +{
>> +	assert(!rlist_empty(&entry->in_queue));
>> +	assert(rlist_last_entry(&limbo->queue, struct txn_limbo_entry,
>> +				 in_queue) == entry);
>> +	(void) limbo;
>> +	rlist_del_entry(entry, in_queue);
>> +}
> txn_limbo_remove is exactly the same as txn_limbo_pop. I suggest to keep
> one of them.
>
> Everything else looks nice.

Assertions are different. I wanted to stress  that `pop` removes entries 
starting

from the tail, and `remove`, on the contrary,  removes them starting 
from the

head.

Looks strange, though, I agree.

If  you merge the functions and put an assertion

`rlist_first_entry == ... || rlist_last_entry == ...`

you'll lose some strictness in their use.

Feel free to decide what to do.


-- 
Serge Petrenko