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>
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-13T09:17:27Z
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 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.

> It's also something that can be committed
> independently, I think.

Right but could this race condition occur outside of the context of this new feature?
  
>> That's right it's started retrieving this information from the relation.
>>
>> Then, Robert made a comment in [1] saying it's not safe to call
>> table_open() while holding a buffer lock.
> 
> 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()

When I first looked at it and saw the number of places where _bt_getbuf() is called
then I preferred to have a look to the current proposal.

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);
"

Regards,

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