Re: [Tarantool-patches] [PATCH v14 5/6] qsync: filter incoming synchro requests

[Cyrill Gorcunov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On Sun, Sep 12, 2021 at 05:44:15PM +0200, Vladislav Shpilevoy wrote:
> > @@ -1686,15 +1685,17 @@ box_issue_promote(uint32_t prev_leader_id, int64_t promote_lsn)
> >  		.lsn = promote_lsn,
> >  		.term = raft->term,
> >  	};
> > -	txn_limbo_process(&txn_limbo, &req);
> > +	if (txn_limbo_process(&txn_limbo, &req) != 0)
> > +		return -1;
> 
> 5. There was already done txn_limbo_write_promote() above. If you
> bail now, you have an inconsistent state - in WAL the promote is
> written, in the limbo it is not applied. What will happen on recovery?
> 
> It seems you need to lock box_promote(), box_promote_qsync(),
> and box_demote(). Otherwise you have the exact same problem with
> local promotions vs coming from the applier as the one you tried
> to fix for applier vs applier.

That's a good point, but as you pointed in previous reviews we
should try to remove locking from api (which i did in new yet
not sent patches) thus we need some kind of a safe filter which
would take a lock, filter request, and release the lock then...
As to me our try to hide locking was a mistake in first place,
locks I proposed simply serialize access to terms they are underlated
to begin/commit/rollback semantics. Actually for now I'm not sure
which approach would fit your architecture ideas. Maybe some
txn_limbo_filter() helper exposed?