On Tue, Apr 4, 2023 at 10:55 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Tue, Apr 4, 2023 at 3:17 AM Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Hi,
> >
> > On 4/3/23 8:10 AM, Drouvot, Bertrand wrote:
> > > Hi,
> > >
> > > On 4/3/23 7:35 AM, Amit Kapila wrote:
> > >> On Mon, Apr 3, 2023 at 4:26 AM Jeff Davis <pgsql@j-davis.com> wrote:
> > >>
> > >> Agreed, even Bertrand and myself discussed the same approach few
> > >> emails above. BTW, if we have this selective logic to wake
> > >> physical/logical walsenders and for standby's, we only wake logical
> > >> walsenders at the time of ApplyWalRecord() then do we need the new
> > >> conditional variable enhancement being discussed, and if so, why?
> > >>
> > >
> > > Thank you both for this new idea and discussion. In that case I don't think
> > > we need the new CV API and the use of a CV anymore. As just said up-thread I'll submit
> > > a new proposal with this new approach.
> > >
> >
> > Please find enclosed V57 implementing the new approach in 0004.
>
> Regarding 0004 patch:
>
> @@ -2626,6 +2626,12 @@ InitWalSenderSlot(void)
> walsnd->sync_standby_priority = 0;
> walsnd->latch = &MyProc->procLatch;
> walsnd->replyTime = 0;
> +
> + if (MyDatabaseId == InvalidOid)
> + walsnd->kind = REPLICATION_KIND_PHYSICAL;
> + else
> + walsnd->kind = REPLICATION_KIND_LOGICAL;
> +
>
> I think we might want to set the replication kind when processing the
> START_REPLICATION command. The walsender using a logical replication
> slot is not necessarily streaming (e.g. when COPYing table data).
>
Discussing with Bertrand off-list, it's wrong as the logical
replication slot creation also needs to read WAL records so a
walsender who is creating a logical replication slot needs to be woken
up. We can set it the replication kind when processing
START_REPLICATION and CREATE_REPLICATION_SLOT, but it seems better to
set it in one place. So I agree to set it in InitWalSenderSlot().
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com