Hi,
On 1/11/23 5:52 PM, Andres Freund wrote:
> 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().
>
>
Done in V44 attached.
>> 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
>
Done in V44.
>
>> 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.
>
In V44, adding a new field "conflicting" in pg_replication_slots which is:
- NULL for physical slots
- True if the slot is a logical one and has been invalidated due to recovery conflict
- False if the slot is a logical one and has not been invalidated due to recovery conflict
I'm not checking if recovery is in progress while displaying the "conflicting". The reason is to still display
the right status after a promotion.
TAP tests are also updated to test that this new field behaves as expected (for both physical and logical slots).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
