Re: [Tarantool-patches] [PATCH 3/8] txn: add TXN_WAIT_ACK flag

[Serge Petrenko <sergepetrenko@tarantool.org>]

21.06.2020 19:25, Vladislav Shpilevoy пишет:
> Thanks for the review!
>
>>>        stailq_foreach_entry(stmt, &txn->stmts, next) {
>>>            if (stmt->has_triggers) {
>>>                txn_init_triggers(txn);
>>>                rlist_splice(&txn->on_commit, &stmt->on_commit);
>>>            }
>>> +        is_sync = is_sync || stmt->space->def->opts.is_sync;
>> Seems like
>> "... || (stmt->space != NULL && stmt->space->def->opts.is_sync);"
>> can be moved here from "replication: make sync transactions wait quorum".
>>
>> LGTM.
> I don't think it is a good idea. At this commit stmt->space can't be
> NULL. This becomes possible only after Sergey P. starts using txns
> to write confirm and rollback entries, which don't relate to any
> space.

Actually, stmt->space is NULL for NOPs.

>
> However I changed this place - I moved is_sync calculation after
> stmt->row == NULL check. So as a read from a sync space wouldn't
> turn the transaction into a synchronous one:
>
> ====================
> commit e29fd482e0954913b87776d0a83ba04ef14683d8
> Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
> Date:   Sun Jun 21 18:12:53 2020 +0200
>
>      txn: don't do sync flag calculation for read statements
>      
>      [TO BE SQUASHED INTO THE PREVIOUS COMMIT]
>
> diff --git a/src/box/txn.c b/src/box/txn.c
> index b50e7c1f8..22591a365 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -502,12 +502,13 @@ txn_journal_entry_new(struct txn *txn)
>   			txn_init_triggers(txn);
>   			rlist_splice(&txn->on_commit, &stmt->on_commit);
>   		}
> -		is_sync = is_sync || stmt->space->def->opts.is_sync;
>   
>   		/* A read (e.g. select) request */
>   		if (stmt->row == NULL)
>   			continue;
>   
> +		is_sync = is_sync || stmt->space->def->opts.is_sync;
> +
>   		if (stmt->row->replica_id == 0)
>   			*local_row++ = stmt->row;
>   		else
>
-- 
Serge Petrenko