Hi, On 2023-01-11 13:02:13 +0530, Bharath Rupireddy wrote: > 3. Is this feature still a 'minimal logical decoding on standby'? > Firstly, why is it 'minimal'?
It's minimal in comparison to other proposals at the time that did explicit / active coordination between primary and standby to allow logical decoding. > 0002: > 1. > - if (InvalidateObsoleteReplicationSlots(_logSegNo)) > + InvalidateObsoleteOrConflictingLogicalReplicationSlots(_logSegNo, > &invalidated, InvalidOid, NULL); > > Isn't the function name too long and verbose? +1 > How about just InvalidateLogicalReplicationSlots() let the function comment > talk about what sorts of replication slots it invalides? I'd just leave the name unmodified at InvalidateObsoleteReplicationSlots(). > 2. > + errdetail("Logical decoding on > standby requires wal_level to be at least logical on master")); > + * master wal_level is set back to replica, so existing logical > slots need to > invalidate such slots. Also do the same thing if wal_level on master > > Can we use 'primary server' instead of 'master' like elsewhere? This > comment also applies for other patches too, if any. +1 > 3. Can we show a new status in pg_get_replication_slots's wal_status > for invalidated due to the conflict so that the user can monitor for > the new status and take necessary actions? Invalidated slots are not a new concept introduced in this patchset, so I'd say we can introduce such a field separately. Greetings, Andres Freund