Horton

Re: Minimal logical decoding on standbys

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

View thread
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
To: Andres Freund <andres@anarazel.de>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>, Amit Khandekar <amitdkhan.pg@gmail.com>, fabriziomello@gmail.com, tushar <tushar.ahuja@enterprisedb.com>, Rahila Syed <rahila.syed@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2023-01-27T12:09:16Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Reduce the log level in 035_standby_logical_decoding.pl.

  2. 035_standby_logical_decoding: Add missing waits for replication

  3. For cascading replication, wake physical and logical walsenders separately

  4. Handle logical slot conflicts on standby

  5. Support invalidating replication slots due to horizon and wal_level

  6. Prevent use of invalidated logical slot in CreateDecodingContext()

  7. Replace replication slot's invalidated_at LSN with an enum

  8. Pass down table relation into more index relation functions

  9. Assert only valid flag bits are passed to visibilitymap_set()

  10. Remove unused _bt_delitems_delete() argument.

  11. Add xl_btree_delete optimization.

Attachments

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