Re: Avoid updating inactive_since for invalid replication slots

On Wed, Feb 5, 2025 at 2:29 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Feb 5, 2025 at 5:53 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > Some review comments for v2-0001.
> >
> > ======
> > doc/src/sgml/system-views.sgml
> >
> > 1.
> > The time when the slot became inactive. NULL if the slot is currently
> > being streamed. If the slot becomes invalid, this value will never be
> > updated. Note that for slots on the standby that are being synced from
> > a primary server (whose synced field is true), the inactive_since
> > indicates the time when slot synchronization (see Section 47.2.3) was
> > most recently stopped. NULL if the slot has always been synchronized.
> > On standby, this is useful for slots that are being synced from a
> > primary server (whose synced field is true) so they know when the slot
> > stopped being synchronized.
> >
> > ~
> >
> > (maybe not strictly related to this patch, but perhaps you can fix it
> > in passing because it will help the readability of the newly added
> > sentence also...)
> >
> > There are 2 different explanations for NULL:
> > "NULL if the slot is currently being streamed."
> > "NULL if the slot has always been synchronized."
> >
> > I'm assuming that 2nd description is only to be read in the scope of
> > "Note that for slots on the standby that are being synced from a
> > primary server...". IMO inserting a blank line before "Note that for
> > slots on the standby..." will help separate these two quite different
> > descriptions for the same field.
> >
>
> This is unrelated to this patch, but I don't mind you proposing a
> separate patch if you feel it will make it clear. Did you see separate
> paragraphs in other descriptions?
>

OK, I have started a new thread [1] for this.

======
[1] https://www.postgresql.org/message-id/CAHut%2BPssvVMTWVtUPto6HbPO8pgVsvtzndt_FdBomA_Oq4zf3w%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia