Re: [Tarantool-patches] [PATCH 1/1] applier: process synchro rows after WAL write

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

08.04.2021 13:19, Cyrill Gorcunov пишет:
> On Thu, Apr 08, 2021 at 11:39:03AM +0300, Serge Petrenko wrote:
>> Thanks for the patch!
>>
>> I'm a bit worried about two different synchro rows coming from two
>> appliers. Is everything ok in this case?
> Serge, you mean the scenario when some instances in replicaset
> have the patch applied and some are not?

No. Let's suppose everyone has this patch applied.
Now look at one particular instance. It may happen that while one of
its appliers is writing this synchro row (either CONFIRM or ROLLBACK,
doesn't matter), some other applier may still apply requests coming
from other replicaset members.

I was wondering what would happen if someone else sent this instance
another synchro row. Looks like nothing bad but I just wanted to
double-check.

And looks like there's a bug, which I'm speaking of below. It's about
someone sending us normal rows (either synchronous transactions or
asynchronous, but not CONFIRM/ROLLBACK entries) while we're waiting for
syncro row's write to end.

Say, limbo was owned by instance 1, and instance 2 has written CONFIRM
for everything there was. While we wait for 2's CONFIRM to be written to
WAL, we may receive some rows from instance 3, who has already applied 2's
CONFIRM. Since we haven't written the CONFIRM yet, we haven't applied it,
and the limbo on our instance still isn't empty. All the rows coming from
3 will get rejected and replication between 3 and us will be broken.

>
>> Or even normal rows coming from other appliers. Say some other replica
>> has already applied this synchro row and even has written some rows on
>> top of it. Its applier won't block on replica_id latch, and may fail to
>> apply
>> some txs following this synchro row, because it's not yet written to WAL
>> and thus not applied (limbo is still not empty or belongs to other
>> instance).
>>
>> Looks like this won't be a problem once synchro rows start pinning the
>> limbo to some specific replica. Because in this case only the replica that
>> has issued confirm will be able to generate new rows. And these rows will
>> be ordered by replica_id latch.
>>
>> But still, maybe this is worth fixing?
>> Am I missing something?
>>> -	struct journal_entry journal_entry;
>>> +	union {
>>> +		struct journal_entry base;
>>> +		char base_buf[sizeof(base) + sizeof(base.rows[0])];
>>> +	};
>>>    };
>> I don't understand this union stuff.
>> The journal_entry is the last entry in synchro_entry anyway.
>> Is this a hack for allowing to allocate synchro_entry on the stack?
> Yeah, the journal_entry last member is zero size array so someone
> has to preallocate memory for rows and using union allows to squash
> everything statically on stack.

Ok, I see now, thanks for the explanation!


-- 
Serge Petrenko