[Tarantool-patches] [PATCH v14 4/6] qsync: order access to the limbo terms
Cyrill Gorcunov
gorcunov at gmail.com
Mon Sep 13 13:52:23 MSK 2021
On Sun, Sep 12, 2021 at 05:44:11PM +0200, Vladislav Shpilevoy wrote:
> > err:
> > + txn_limbo_term_unlock(&txn_limbo);
> > diag_log();
>
> 1. This function can go to 'err' before the lock is taken.
yup, thanks!
>
>
> 2. As for your complaint about the begin/commit/rollback API
> being not working because you can't unlock from a non-owner
> fiber - well, it works in your patch somehow, doesn't it?
Yes, it works with my patch because journal_write() are ordered,
we take and release lock explicitly inside caller code, ie inside
one same fiber(). Imagine two appliers running simultaneously
applier 1 applier 2 sched
--------- --------- -----
apply_synchro_row
txn_limbo_term_lock
journal_write
context-switch --> apply_synchro_row
txn_limbo_term_lock
wait for release
apply_synchro_row_cb
context-switch
txn_limbo_term_unlock <-- --+
return
txn_limbo_term_lock finishes
...
> Why do you in your patch unlock here, but in the newly proposed
> API you only tried to unlock in the trigger?
Because you proposed to *hide* locking completely inside limbo code,
so the caller won't know anything about it. So our commit/rollback
would handle locking internally, unfortunately this doesn't work.
>
> You could call commit/rollback from this function, like you
> do with unlock now.
Not sure if I follow you here. Our journal engine implies completions
to be called. We pass such completion into journal entry creation. With
my patch everything remains as is except we take a lock explicitly and
release it then. Could you please point more explicitly what you've in mind?
> > +
> > +/** Fetch replica's term with lock taken. */
> > +static inline uint64_t
> > +txn_limbo_replica_term_locked(struct txn_limbo *limbo, uint32_t replica_id)
>
> 3. Limbo can be made const here.
ok
More information about the Tarantool-patches
mailing list