Re: Minimal logical decoding on standbys

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Jeff Davis <pgsql@j-davis.com>, Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@gmail.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-04-06T13:02:00Z
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.

Hi,

On 4/6/23 11:55 AM, Amit Kapila wrote:
> On Thu, Apr 6, 2023 at 12:10 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Wed, Apr 5, 2023 at 9:27 PM Drouvot, Bertrand
>> <bertranddrouvot.pg@gmail.com> wrote:
>>>
>>
>> Another comment on 0001.
>>   extern void CheckSlotRequirements(void);
>>   extern void CheckSlotPermissions(void);
>> +extern void ResolveRecoveryConflictWithLogicalSlots(Oid dboid,
>> TransactionId xid, char *reason);
>>
>> This doesn't seem to be called from anywhere.
>>
> 
> Few other comments:
> ==================
> 0004
> 1.
> + *  - physical walsenders in case of new time line and cascade
> + *  replication is allowed.
> + *  - logical walsenders in case of new time line or recovery is in progress
> + *  (logical decoding on standby).
> + */
> + WalSndWakeup(switchedTLI && AllowCascadeReplication(),
> + switchedTLI || RecoveryInProgress());
> 
> Do we need AllowCascadeReplication() check specifically for physical
> walsenders? I think this should be true for both physical and logical
> walsenders.
> 

I don't think it could be possible to create logical walsenders on a standby if
AllowCascadeReplication() is not true, or am I missing something?

If so, I think it has to be set to true for the logical walsenders in all the case (like
done in V62 posted up-thread).

Andres, made the point up-thread that RecoveryInProgress() is always true, and
as we don't want to be woken up only when there is a time line change then I think
it has to be always true for logical walsenders.

> 0005
> 2.
> --- a/src/backend/access/transam/xlogfuncs.c
> +++ b/src/backend/access/transam/xlogfuncs.c
> @@ -38,6 +38,7 @@
>   #include "utils/pg_lsn.h"
>   #include "utils/timestamp.h"
>   #include "utils/tuplestore.h"
> +#include "storage/standby.h"
> 
> The header includes should be in alphabetical order.
> 

Good catch, thanks! Done in V62.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com