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