Re: Introduce XID age and inactive timeout based replication slot invalidation

On Wed, Mar 27, 2024 at 11:39 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> Thanks for the patch. Few trivial things:

Thanks for reviewing.

> ----------
> 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)

Done.

> 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"

I added the note that Bertrand specified upthread. But, I couldn't
find an instance of adding <note> ... </note> within a table. Hence
with "Note that ...." statments just like any other notes in the
system-views.sgml. pg_replication_slot in system-vews.sgml renders as
table, so having <note> ... </note> may not be a great idea.

> 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.
> ---------

Reworded.

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

Please check the attached v27 patch which also has Bertrand's comment
on deduplicating the TAP function. I've now moved it to Cluster.pm.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com