Re: Copy function for logical replication slots

On Tue, Feb 19, 2019 at 1:28 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2019-02-18 16:57:07 +0900, Masahiko Sawada wrote:
> > > Stupid question, but couldn't we optimize this to something like:
> > >
> > > /*
> > > * First copy current data of the slot. Then install those in the
> > > * new slot. The src slot could have progressed while installing,
> > > * but the installed values prevent global horizons from progressing
> > > * further. Therefore a second copy is sufficiently up2date.
> > > */
> > > SpinLockAcquire(&src->mutex);
> > > copy_lsn = src->data.restart_lsn;
> > > copy_xid = ...;
> > > SpinLockRelease(&src->mutex);
> > >
> > > /* install copied values */
> > >
> > >
> > > SpinLockAcquire(&src->mutex);
> > > /* copy data of slot again */
> > > SpinLockRelease(&src->mutex);
> > >
> > > /* install again */
> > >
> > > ?
> >
> > With this optimization since we don't need to acquire the source slot
> > we can copy even from a slot that has already been acquired by
> > someone, which is great. However is it possible that once released the
> > first spinlock of the source slot it could be dropped and the global
> > horizons can progress before installing the copied values?
>
> Well, I'd not thought we'd do it without acquiring the other slot. But
> that still seems to be easy enough to address, we just need to recheck
> whether the slot still exists (with the right name) the second time we
> acquire the spinlock?

Yeah, I think that would work. The attached patch takes this
direction. Please review it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center