On Mon, Mar 18, 2024 at 3:42 PM Bertrand Drouvot <bertranddrouvot...@gmail.com> wrote: > > Hi, > > Looking at 0001:
Thanks for reviewing. > 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."? Will change. > 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? Not required I think. One can pick the required columns in the SELECT clause anyways. > 3 === > > - * Maps a conflict reason for a replication slot to > + * Maps a invalidation reason for a replication slot to > > s/a invalidation/an invalidation/? Will change. > 4 === > > While at it, shouldn't we also rename "conflict" to say "invalidation_cause" > in > InvalidatePossiblyObsoleteSlot()? That's inline with our understanding about conflict vs invalidation, and keeps the function generic. Will change. > 5 === > > + * rows_removed and wal_level_insufficient are only two reasons > > s/are only two/are the only two/? Will change.. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
