Re: Minimal logical decoding on standbys

From: "Drouvot, Bertrand" <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>, fabriziomello(at)gmail(dot)com, tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>, Rahila Syed <rahila(dot)syed(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Minimal logical decoding on standbys
Date: 2023-01-27 12:09:16
Message-ID: bdc49e0b-cd39-bcd3-e391-b0ad6e48b5cf@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v44-0006-Doc-changes-describing-details-about-logical-dec.patch text/plain 2.1 KB
v44-0005-New-TAP-test-for-logical-decoding-on-standby.patch text/plain 26.5 KB
v44-0004-Fixing-Walsender-corner-case-with-logical-decodi.patch text/plain 7.5 KB
v44-0003-Allow-logical-decoding-on-standby.patch text/plain 11.8 KB
v44-0002-Handle-logical-slot-conflicts-on-standby.patch text/plain 37.0 KB
v44-0001-Add-info-in-WAL-records-in-preparation-for-logic.patch text/plain 73.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2023-01-27 12:15:57 Re: Deadlock between logrep apply worker and tablesync worker
Previous Message Vigneshk Kvignesh 2023-01-27 11:57:40 Re: PG11 to PG14 Migration Slowness