Thread: pg_recvlogical cannot create slots with failover=true
Dear hackers, While reviewing threads I found $SUBJECT. I think it does not affect to output, but I could not find strong reasons not to support it. Also, this can be used for testing purpose, i.e., DBAs can verify the slot-sync can work on their system by only using pg_recvlogical. Attached patch implements it. Since -f/-F option has already been used, -O was chosen for the short-version. Better idea is very welcomed. How do you feel? Best regards, Hayato Kuroda FUJITSU LIMITED
Attachment
Hi, On Tue, Apr 01, 2025 at 08:01:32AM +0000, Hayato Kuroda (Fujitsu) wrote: > Attached patch implements it. Since -f/-F option has already been used, -O was > chosen for the short-version. Better idea is very welcomed. Maybe we don't need a short option at all for this, at least initially? Michael
Dear Michael, > Maybe we don't need a short option at all for this, at least initially? Indeed, updated. Best regards, Hayato Kuroda FUJITSU LIMITED
Attachment
On Wed, Apr 2, 2025 at 11:57 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Michael,
>
> > Maybe we don't need a short option at all for this, at least initially?
>
> Indeed, updated.
Thank you for updating the patch. Here are some minor comments:
The <option>--two-phase</option> can be specified with
<option>--create-slot</option> to enable decoding of prepared
transactions.
+ Also, <option>--falover</option> can be specified with
+ <option>--create-slot</option> to create replication slot which can be
+ synchronized to the standby.
How about rewording the whole sentence like:
+ The <option>--two-phase</option> and
<option>--failover</option> options
+ can be specified with <option>--create-slot</option>.
Explaining both options here seems redundant to me.
---
+ <para>
+ Allows created replication slot to be synchronized to the standbys.
+ This option may only be specified with <option>--create-slot</option>.
+ </para>
How about slightly rewording to like:
+ Enables the slot to be synchronized to the standbys. This
option may only be
+ specified with <option>--create-slot</option>.
Also, the descriptions of pg_recvlogical options are written in
alphabetical order. Please put the description for --failover option
after -E/--endpos.
The rest looks good to me.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
Dear Sawada-san, > Thank you for updating the patch. Here are some minor comments: > > The <option>--two-phase</option> can be specified with > <option>--create-slot</option> to enable decoding of prepared > transactions. > + Also, <option>--falover</option> can be specified with > + <option>--create-slot</option> to create replication slot which can > be > + synchronized to the standby. > > How about rewording the whole sentence like: > > + The <option>--two-phase</option> and > <option>--failover</option> options > + can be specified with <option>--create-slot</option>. > > Explaining both options here seems redundant to me. Fixed. > --- > + <para> > + Allows created replication slot to be synchronized to the standbys. > + This option may only be specified with > <option>--create-slot</option>. > + </para> > > How about slightly rewording to like: > > + Enables the slot to be synchronized to the standbys. This > option may only be > + specified with <option>--create-slot</option>. Fixed. The description in usage() is adjusted based on this. > Also, the descriptions of pg_recvlogical options are written in > alphabetical order. Please put the description for --failover option > after -E/--endpos. Right. I put because it had short-term '-o' in old version, but it was removed. Fixed. PSA new version. Best regards, Hayato Kuroda FUJITSU LIMITED
Attachment
On Thu, Apr 3, 2025 at 8:28 PM Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> wrote: > > Dear Sawada-san, > > > Thank you for updating the patch. Here are some minor comments: > > > > The <option>--two-phase</option> can be specified with > > <option>--create-slot</option> to enable decoding of prepared > > transactions. > > + Also, <option>--falover</option> can be specified with > > + <option>--create-slot</option> to create replication slot which can > > be > > + synchronized to the standby. > > > > How about rewording the whole sentence like: > > > > + The <option>--two-phase</option> and > > <option>--failover</option> options > > + can be specified with <option>--create-slot</option>. > > > > Explaining both options here seems redundant to me. > > Fixed. > > > --- > > + <para> > > + Allows created replication slot to be synchronized to the standbys. > > + This option may only be specified with > > <option>--create-slot</option>. > > + </para> > > > > How about slightly rewording to like: > > > > + Enables the slot to be synchronized to the standbys. This > > option may only be > > + specified with <option>--create-slot</option>. > > Fixed. The description in usage() is adjusted based on this. > > > Also, the descriptions of pg_recvlogical options are written in > > alphabetical order. Please put the description for --failover option > > after -E/--endpos. > > Right. I put because it had short-term '-o' in old version, but it was removed. > Fixed. Thank you for updating the patch! Pushed with small cosmetic changes. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Sat, Apr 5, 2025 at 1:45 AM Masahiko Sawada wrote: Hi, > Thank you for updating the patch! Pushed with small cosmetic changes. Thanks for pushing the feature ! I noticed one typo in the doc and here is a tiny patch to fix it. - The <option>--two-phase</option> and <option>--falover</option> options + The <option>--two-phase</option> and <option>--failover</option> options Best Regards, Hou zj
Attachment
On Sun, Apr 6, 2025 at 7:19 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote: > > On Sat, Apr 5, 2025 at 1:45 AM Masahiko Sawada wrote: > > Hi, > > > Thank you for updating the patch! Pushed with small cosmetic changes. > > Thanks for pushing the feature ! > > I noticed one typo in the doc and here is a tiny patch to fix it. > > - The <option>--two-phase</option> and <option>--falover</option> options > + The <option>--two-phase</option> and <option>--failover</option> options Thank you for the report and the patch! Pushed. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On 07.04.25 21:15, Masahiko Sawada wrote: > On Sun, Apr 6, 2025 at 7:19 PM Zhijie Hou (Fujitsu) > <houzj.fnst@fujitsu.com> wrote: >> >> On Sat, Apr 5, 2025 at 1:45 AM Masahiko Sawada wrote: >> >> Hi, >> >>> Thank you for updating the patch! Pushed with small cosmetic changes. >> >> Thanks for pushing the feature ! >> >> I noticed one typo in the doc and here is a tiny patch to fix it. >> >> - The <option>--two-phase</option> and <option>--falover</option> options >> + The <option>--two-phase</option> and <option>--failover</option> options > > Thank you for the report and the patch! Pushed. I wonder if the option name --failover is ideal. To me, it sounds like an action "do a failover!". Also consider that pg_recvlogical has other action options such as --start and --create-slot, so it sounds a bit like those. Maybe something like --enable-failover would be better?
On Thu, Jun 12, 2025 at 10:42 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > On 07.04.25 21:15, Masahiko Sawada wrote: > > On Sun, Apr 6, 2025 at 7:19 PM Zhijie Hou (Fujitsu) > > <houzj.fnst@fujitsu.com> wrote: > >> > >> On Sat, Apr 5, 2025 at 1:45 AM Masahiko Sawada wrote: > >> > >> Hi, > >> > >>> Thank you for updating the patch! Pushed with small cosmetic changes. > >> > >> Thanks for pushing the feature ! > >> > >> I noticed one typo in the doc and here is a tiny patch to fix it. > >> > >> - The <option>--two-phase</option> and <option>--falover</option> options > >> + The <option>--two-phase</option> and <option>--failover</option> options > > > > Thank you for the report and the patch! Pushed. > > I wonder if the option name --failover is ideal. To me, it sounds like > an action "do a failover!". Also consider that pg_recvlogical has other > action options such as --start and --create-slot, so it sounds a bit > like those. Fair point. > Maybe something like --enable-failover would be better? Sounds better, but probably the --two-phase option has the same issue? Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
On Sat, Jun 14, 2025 at 3:11 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Thu, Jun 12, 2025 at 10:42 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > >> > > >> - The <option>--two-phase</option> and <option>--falover</option> options > > >> + The <option>--two-phase</option> and <option>--failover</option> options > > > > > > Thank you for the report and the patch! Pushed. > > > > I wonder if the option name --failover is ideal. To me, it sounds like > > an action "do a failover!". Also consider that pg_recvlogical has other > > action options such as --start and --create-slot, so it sounds a bit > > like those. > > Fair point. > > > Maybe something like --enable-failover would be better? > > Sounds better, but probably the --two-phase option has the same issue? > Ideally, we should change both to maintain consistency across various slot options. OTOH, as we have already described these options as: " The --two-phase and --failover options can be specified with --create-slot.", it is clear that these are slot options. The previous version docs have a description: "The --two-phase can be specified with --create-slot to enable decoding of prepared transactions." which makes it even more clear that the two-phase is a slot option. The options are named similarly in pg_create_logical_replication_slot API and during CREATE SUBSCRIPTION, so, to some level, there is a consistency in naming of these options across all APIs/tools. But, their usage in a tool like pg_recvlogical could be perceived differently as well, so it is also okay to consider naming them differently. -- With Regards, Amit Kapila.