MIME-Version: 1.0
References: <CAJpy0uB-yE+Riw7JQ4hW0+igJxvPc+rq+9c7WyTa1Jz7+2gAiA@mail.gmail.com>
 <CALj2ACXRFx9g7A9RFJZF7eBe=zxk7=apMRFuCgJJKYB7O=vgwg@mail.gmail.com>
 <ZgJ9D0p28ijwsjsl@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA4eK1KrPGwfZV9LYGidjxHeW+rxJ=E2ThjXvwRGLO=iLNuo=Q@mail.gmail.com>
 <ZgKGIDC5lttWTdJH@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAJpy0uDkTW+t1k3oPkaipFBzZePfFNB5DmiA==pxRGcAdpF=Pg@mail.gmail.com>
 <ZgKhgtq+PN0IsURZ@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAJpy0uB=caX5t6jJe6J+jGUFxnYoKPoAE+8Eaz8t+YXxm-esxw@mail.gmail.com>
 <CALj2ACWLctoiH-pSjWnEpR54q4DED6rw_BRJm5pCx86_Y01MoQ@mail.gmail.com>
 <CALj2ACUXS0SfbHzsX8bqo+7CZhocsV52Kiu7OWGb5HVPAmJqnA@mail.gmail.com>
 <ZgMLTBD3xJIP9W93@ip-10-97-1-34.eu-west-3.compute.internal>
 <CALj2ACX-M+Lx6Qv4id7HtqqTKVKA_ti4whK9cgpTPHdHLi943w@mail.gmail.com> <CALj2ACXEwHxzuP9imWoJKc24HcSB1izPaot7U9XGzX0qGWq3DA@mail.gmail.com>
In-Reply-To: <CALj2ACXEwHxzuP9imWoJKc24HcSB1izPaot7U9XGzX0qGWq3DA@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Wed, 27 Mar 2024 11:39:04 +0530
Message-ID: <CAJpy0uAyG44xp4pzGd7X+xrJMuPfD0dYC0tA9Gep9zMuALzFGg@mail.gmail.com>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, 
	Nathan Bossart <nathandbossart@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAJpy0uAyG44xp4pzGd7X%2BxrJMuPfD0dYC0tA9Gep9zMuALzFGg%40mail.gmail.com>
Precedence: bulk

On Wed, Mar 27, 2024 at 11:05=E2=80=AFAM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> Fixed an issue in synchronize_slots where DatumGetLSN is being used in
> place of DatumGetTimestampTz. Found this via CF bot member [1], not on
> my dev system.
>
> Please find the attached v6 patch.

Thanks for the patch. Few trivial things:

----------
1)
system-views.sgml:

a) "Note that the slots" --> "Note that the slots on the standbys,"
--it is good to mention "standbys" as synced could be true on primary
as well (promoted standby)

b) If you plan to add more info which Bertrand suggested, then it will
be better to make a <note> section instead of using "Note"

2)
commit msg:

"The impact of this
on a promoted standby inactive_since is always NULL for all
synced slots even after server restart.
"
Sentence looks broken.
---------

Apart from the above trivial things, v26-001 looks good to me.

thanks
Shveta