RE: Exit walsender before confirming remote flush in logical replication

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "sawada(dot)mshk(at)gmail(dot)com" <sawada(dot)mshk(at)gmail(dot)com>, "michael(at)paquier(dot)xyz" <michael(at)paquier(dot)xyz>, "peter(dot)eisentraut(at)enterprisedb(dot)com" <peter(dot)eisentraut(at)enterprisedb(dot)com>, "dilipbalaut(at)gmail(dot)com" <dilipbalaut(at)gmail(dot)com>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Subject: RE: Exit walsender before confirming remote flush in logical replication
Date: 2023-02-08 07:59:09
Message-ID: TYAPR01MB5866D3EC780D251953BDE7FAF5D89@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Amit,

Thanks for giving comments!

> >
> > 0002:
> >
> > This patch doesn't seem to offer a means to change the default
> > walsender behavior. We need a subscription option named like
> > "walsender_exit_mode" to do that.
> >
>
> I don't think at this stage we need a subscription-level option, we
> can extend it later if this is really useful for users. For now, we
> can set this new option when min_apply_delay > 0.

Agreed. I wanted to keep the feature closed for PG16 and then will extend if needed.

> >
> > +ConsumeWalsenderOptions(List *options, WalSndData *data)
> >
> > I wonder if it is the right design to put options for different things
> > into a single list. I rather choose to embed the walsender option in
> > the syntax than needing this function.
> >
> > K_START_REPLICATION opt_slot opt_physical RECPTR opt_timeline
> opt_shutdown_mode
> >
> > K_START_REPLICATION K_SLOTIDENT K_LOGICAL RECPTR
> opt_shutdown_mode plugin_options
> >
> > where opt_shutdown_mode would be like "SHUTDOWN_MODE immediate".
> >
>
> The other option could have been that we just add it as a
> plugin_option for logical replication but it doesn't seem to match
> with the other plugin options. I think it would be better to have it
> as a separate option something like opt_shutdown_immediate and extend
> the logical replication syntax for now. We can later extend physical
> replication syntax when we want to expose such an option via physical
> replication.

The main intention for us is to shut down logical walsenders. Therefore, same as above,
I want to develop the feature for logical replication once and then try to extend if we want.
TBH I think adding physicalrep support seems not to be so hard,
but I want to keep the patch smaller.

The new patch will be attached soon in another mail.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-02-08 07:59:24 ICU locale validation / canonicalization
Previous Message Richard Guo 2023-02-08 07:52:58 Re: A bug in make_outerjoininfo