Re: Minimal logical decoding on standbys

Bertrand Drouvot <bertranddrouvot.pg@gmail.com>

From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
To: Andres Freund <andres@anarazel.de>
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-16T14:28:54Z
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/13/23 10:17 AM, Drouvot, Bertrand wrote:
> Hi,
> 
> On 1/11/23 9:27 PM, Andres Freund wrote:
>> Hi,
>>
>> On 2023-01-06 10:52:06 +0100, Drouvot, Bertrand wrote:
>>
>> The problem I have with that is that I saw a lot of flakiness in the tests due
>> to the race condition. So introducing them in that order just doesn't make a
>> whole lot of sense to me. 
> 
> You are right it does not make sense to introduce fixing the race condition after the TAP tests
> and after introducing the decoding logic. I'll reorder the sub-patches.
> 

V40 attached is changing the sub-patches ordering.

>> The suggested path in earlier versions to avoid doing so was to make sure that
>> we pass down the Relation for the table into the necessary functions. Did you
>> explore that any further?
> 
> So, for gistXLogPageReuse() and _bt_delitems_delete() this is "easy" to pass the Heap Relation.
> This is what was done in earlier versions of this patch series.
> 
> But we would need to define a way to propagate the Heap Relation for those 2 functions:
> 
> _bt_log_reuse_page()
> vacuumRedirectAndPlaceholder()
> 

V40 is getting rid of the new indisusercatalog field in pg_index and is passing the
heap relation all the way down to _bt_log_reuse_page() and vacuumRedirectAndPlaceholder() instead
(and obviously to gistXLogPageReuse() and _bt_delitems_delete() too).

Remarks:

1) V40 adds the heap relation in the IndexVacuumInfo and ParallelVacuumState structs. It is used
for the _bt_log_reuse_page() and vacuumRedirectAndPlaceholder() cases where I did not find any place
where to get the heap relation from in the existing code path.

2) V40 adds a "real" heap relation to all the _bt_getbuf() calls. Another option could have been
to add it only for the code paths leading to _bt_log_reuse_page() but I thought it is cleaner to
do it for all of them.

> I will give it another look, also because I just realized that it could be beneficial
> for vacuumRedirectAndPlaceholder() too, as per this comment:
> 
> "
>      /* XXX: providing heap relation would allow more pruning */
>      vistest = GlobalVisTestFor(NULL);
> "

Now, we could also pass the heap relation to GlobalVisTestFor() in vacuumRedirectAndPlaceholder().
Could be done in or independently of this patch series once committed (it's not part of V40).

Regards,

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