Re: [Tarantool-patches] [PATCH 13/16] tx: introduce txm_story

[Aleksandr Lyapunov <alyapunov@tarantool.org>]

Hi, thank for review!

On 15.07.2020 02:46, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> See 3 comments below.
>
>
> 1. What if index count of the space will change in a concurrect
> DDL transaction?
That's a good question. I know what should be - DDL must create
a new space structure for new transaction while leaving old
space structure for old transactions.. But I'm sure it does not
work that way now. I'll write a test and file a ticket.
>
>> +}
>> +
>> +static struct txm_story *
>> +txm_story_new_add_stmt(struct tuple *tuple, struct txn_stmt *stmt)
> 2. Probably would be better to use the existing terms: old_tuple and new_tuple.
> So the functions would be txm_story_add_new_stmt and txm_story_add_old_stmt. The
> same in all the other places. 'add_stmt' -> 'new_stmt'. 'del_stmt' -> 'old_stmt'.
I thought about it. I fear in terms of txm_story, when we speak about
lifetime of a tuple it would be weird. add_stmt - is the statement that
added the tuple. But what is new_stmt? new_psn?
>
>> +{
>> +	struct txm_story *res = txm_story_new(tuple, stmt->space->index_count);
>> +	if (res == NULL)
>> +		return NULL;
>> +	res->add_stmt = stmt;
>> +	assert(stmt->add_story == NULL);
>> +	stmt->add_story = res;
>> +	return res;
>> +}
>> +
>> +static struct txm_story *
>> +txm_story_new_del_stmt(struct tuple *tuple, struct txn_stmt *stmt)
>> +{
>> +	struct txm_story *res = txm_story_new(tuple, stmt->space->index_count);
>> +	if (res == NULL)
>> +		return NULL;
>> +	res->del_stmt = stmt;
>> +	assert(stmt->del_story == NULL);
>> +	stmt->del_story = res;
>> +	return res;
>> +}
>> +
>> +void
>> +txm_history_prepare_stmt(struct txn_stmt *stmt)
>> +{
>> +	assert(stmt->txn->psn != 0);
>> +
>> +	/* Move story to the past to prepared stories. */
>> +
>> +	struct txm_story *story = stmt->add_story;
>> +	uint32_t index_count = story == NULL ? 0 : story->index_count;
>> +	for (uint32_t i = 0; i < index_count; ) {
>> ...
>> ...
>> ...
>> +	}
>> +	if (stmt->add_story != NULL)
> 3. According to a check in the beginning of the function stmt can
> be NULL. But then this will crash.
No, index_count is initialized as 0 in that case and we will not enter 
the loop.