On Wed, 26 Nov 2025 at 15:21, Shlok Kyal <[email protected]> wrote:
>
> On Wed, 26 Nov 2025 at 14:23, Zhijie Hou (Fujitsu)
> <[email protected]> wrote:
> >
> > On Wednesday, November 26, 2025 2:29 PM Shlok Kyal 
> > <[email protected]> wrote:
> > > I have also addressed the remaining comments and attached the updated
> > > patch.
> >
> > Thanks for updating the patch, I have few comments:
> >
> > 1.
> > +/*
> > + * Map a SlotSyncSkipReason enum to a human-readable string
> > + */
> > +static char *
> > +GetSlotSyncSkipReasonName(SlotSyncSkipReason reason)
> >
> > Shall we add a static array to map the Enum value to the reason name
> > instead of adding the following function ?
> >
> I think static array would be more clean and consistent with
> ConflictTypeNames and SlotInvalidationCauses.
> Made the changes for same.
>
> > 2.
> > +          <literal>remote_behind</literal> means that the slot is ahead of 
> > the
> > +          corresponding failover slot on the primary.
> >
> > I think the current naming and doc is not easy for user to understand. So, I
> > suggest mentioning the explicit reason of this skip, e.g., the required 
> > WALs and
> > rows are removed or at the risk of removal. We can rename this reason to
> > "wal_or_rows_removed" and make the document similar to the content in
> > logicaldecoding.sgml.
> >
> I agree. Included the changes for same.
>
> I have also addressed the comments by Shveta in [1].
> [1]: 
> https://www.postgresql.org/message-id/CAJpy0uDKC0QubC0pL%3DbZ4Qnq3eQbykLnFu5x%3DwmDkOmL44QL7g%40mail.gmail.com

I have made some minor changes in documentation and comments. Attached
the updated patch.

Thanks,
Shlok Kyal

Attachment: v15-0001-Add-slotsync_skip_reason-to-pg_replication_slots.patch
Description: Binary data

Reply via email to