Re: Avoid updating inactive_since for invalid replication slots

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?

--
With Regards,
Amit Kapila.