public inbox for [email protected]  

help / color / mirror / Atom feed
From: Bertrand Drouvot <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
Date: Mon, 18 Mar 2024 10:12:15 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACWBbSa7ccOGDHNfH1OX7_ArBJm1kNmc=iWKSfqq1NHnuA@mail.gmail.com>
References: <CAA4eK1+NUeb21bYa9K5mVsFHcw-xodGLkN9QoqJXym7sD3LtZA@mail.gmail.com>
	<CALj2ACVr8ra-yp8CwiBq=Pc9Kkq64z=zDvoFxMK3K4wSDhKMmg@mail.gmail.com>
	<CAA4eK1KxsMg4bnxRZ0U3dZ3irFoR2=TfUCOFig3ccHoRPp5iWg@mail.gmail.com>
	<CALj2ACU-sJdiLfAcWd+8hnt4HSojH0UWFGnaGr9gxb2bYzYYSA@mail.gmail.com>
	<CAA4eK1++=fOpmsQKugQQRtQr2wmiintjKEhpuOch9=phxirAfg@mail.gmail.com>
	<CALj2ACVd_dizYQiZwwUfsb+hG-fhGYo_kEDq0wn_vNwQvOrZHg@mail.gmail.com>
	<CAA4eK1+02XhKht3EB=RgzQ=2zyb9Sv4=9iC=yt1j4FfVy0-5kg@mail.gmail.com>
	<CALj2ACVRS2LZbycpM8rZT8QynR05AtvttOHfFOhczjyqU7zZSA@mail.gmail.com>
	<CAJpy0uChxSm7pod6CQwU4z4pSvV03b4iRCy_9zb0zVbSs__qOg@mail.gmail.com>
	<CALj2ACWBbSa7ccOGDHNfH1OX7_ArBJm1kNmc=iWKSfqq1NHnuA@mail.gmail.com>

Hi,

On Sat, Mar 16, 2024 at 09:29:01AM +0530, Bharath Rupireddy wrote:
> Please see the attached v11 patch set with all the above review
> comments addressed.

Thanks!

Looking at 0001:

1 ===

+       True if this logical slot conflicted with recovery (and so is now
+       invalidated). When this column is true, check

Worth to add back the physical slot mention "Always NULL for physical slots."?

2 ===

@@ -1023,9 +1023,10 @@ CREATE VIEW pg_replication_slots AS
             L.wal_status,
             L.safe_wal_size,
             L.two_phase,
-            L.conflict_reason,
+            L.conflicting,
             L.failover,
-            L.synced
+            L.synced,
+            L.invalidation_reason

What about making invalidation_reason close to conflict_reason?

3 ===

- * Maps a conflict reason for a replication slot to
+ * Maps a invalidation reason for a replication slot to

s/a invalidation/an invalidation/?

4 ===

While at it, shouldn't we also rename "conflict" to say "invalidation_cause" in
InvalidatePossiblyObsoleteSlot()?

5 ===

+ * rows_removed and wal_level_insufficient are only two reasons

s/are only two/are the only two/?

Regards,

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






view thread (18+ 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]
  Subject: Re: Introduce XID age and inactive timeout based replication slot invalidation
  In-Reply-To: <[email protected]>

* 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