public inbox for [email protected]  

help / color / mirror / Atom feed
From: Drouvot, Bertrand <[email protected]>
To: Andres Freund <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Ibrar Ahmed <[email protected]>
Cc: Amit Khandekar <[email protected]>
Cc: [email protected]
Cc: tushar <[email protected]>
Cc: Rahila Syed <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: Minimal logical decoding on standbys
Date: Fri, 13 Jan 2023 10:17:27 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+TgmoZ8-DXtDMMDGggFkSYyO8nmNdBKWjV6Ts-zYGR9p5sK5g@mail.gmail.com>
	<CA+TgmobmTvEUDVOBzARa9eD9XLqsvDW+v29dV+WvDKZPJAwNTQ@mail.gmail.com>
	<CA+TgmoaOsoQuXEo6gMq0GP8SyvCaS=ptYqdYc0uzDMoeShf_nw@mail.gmail.com>
	<CA+TgmoYTTsxP8y6uknZvCBNCRq+1FJ4zGbX8Px1TGW459fGsaQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CA+TgmoY8LAaTwAhS+YNQNrM+3K8eqxGvPT3gbJaHER86XShvFw@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>

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






view thread (6+ messages)  latest in thread

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Minimal logical decoding on standbys
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox