public inbox for [email protected]  

help / color / mirror / Atom feed

From: Peter Smith <[email protected]>
To: Nisha Moond <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Amit Kapila <[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 11:23:19 +1100
Message-ID: <CAHut+Pt1hOx8eYMOGOi7+HCr3aAr4kXZ-F41RidcjoihkjfvNg@mail.gmail.com> (raw)
In-Reply-To: <CABdArM6cV4=yb5sh4TbuEkYmR0dRdTvODiMgOLv5a-ygfxQaMA@mail.gmail.com>
References: <CABdArM7QdifQ_MHmMA=Cc4v8+MeckkwKncm2Nn6tX9wSCQ-+iw@mail.gmail.com>
	<OS0PR01MB57165D33B186D168E867147B94F42@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CABdArM6cV4=yb5sh4TbuEkYmR0dRdTvODiMgOLv5a-ygfxQaMA@mail.gmail.com>

Hi Nisha,

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.

~~~

Apart from the above comment the v2 patch looked ok to me.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

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: <CAHut+Pt1hOx8eYMOGOi7+HCr3aAr4kXZ-F41RidcjoihkjfvNg@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