public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Peter Smith <[email protected]>
Cc: Nisha Moond <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Subject: Re: Avoid updating inactive_since for invalid replication slots
Date: Wed, 5 Feb 2025 08:59:26 +0530
Message-ID: <CAA4eK1LP+T6FQkKtiv2WVk58W9DpNBvLAkqNxLowsorJEg6Tcw@mail.gmail.com> (raw)
In-Reply-To: <CAHut+Pt1hOx8eYMOGOi7+HCr3aAr4kXZ-F41RidcjoihkjfvNg@mail.gmail.com>
References: <CABdArM7QdifQ_MHmMA=Cc4v8+MeckkwKncm2Nn6tX9wSCQ-+iw@mail.gmail.com>
	<OS0PR01MB57165D33B186D168E867147B94F42@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CABdArM6cV4=yb5sh4TbuEkYmR0dRdTvODiMgOLv5a-ygfxQaMA@mail.gmail.com>
	<CAHut+Pt1hOx8eYMOGOi7+HCr3aAr4kXZ-F41RidcjoihkjfvNg@mail.gmail.com>

On Wed, Feb 5, 2025 at 5:53 AM Peter Smith <[email protected]> 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.






view thread (5+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Avoid updating inactive_since for invalid replication slots
  In-Reply-To: <CAA4eK1LP+T6FQkKtiv2WVk58W9DpNBvLAkqNxLowsorJEg6Tcw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox