public inbox for [email protected]  

help / color / mirror / Atom feed

From: Drouvot, Bertrand <[email protected]>
To: Jeff Davis <[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, 3 Mar 2023 17:23:36 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CA+TgmoZ8-DXtDMMDGggFkSYyO8nmNdBKWjV6Ts-zYGR9p5sK5g@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]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

Hi,

On 3/2/23 8:45 PM, Jeff Davis wrote:
> On Thu, 2023-03-02 at 10:20 +0100, Drouvot, Bertrand wrote:
>> Right, but in our case, right after the wakeup (the one due to the CV
>> broadcast,
>> aka the one that will remove it from the wait queue) we'll exit the
>> loop due to:
>>
>> "
>>           /* check whether we're done */
>>           if (loc <= RecentFlushPtr)
>>               break;
>> "
>>
>> as the CV broadcast means that a flush/replay occurred.
> 
> But does it mean that the flush/replay advanced *enough* to be greater
> than or equal to loc?
> 

Yes I think so: loc is when we started waiting initially
and RecentFlushPtr is >= to when the broadcast has been sent.

>> - If it is awakened due to the CV broadcast, then we'll right after
>> exit the loop (see above)
> 
> ...
> 
>> I think that's not needed as we'd exit the loop right after we are
>> awakened by a CV broadcast.
> 
> See the comment here:
> WalSndWaitForWal
>   * If this process has been taken out of the wait list, then we know
>   * that it has been signaled by ConditionVariableSignal (or
>   * ConditionVariableBroadcast), so we should return to the caller. But
>   * that doesn't guarantee that the exit condition is met, only that we
>   * ought to check it.
> 
> You seem to be arguing that in this case, it doesn't matter; that
> walreceiver knows what walsender is waiting for, and will never wake it
> up before it's ready. I don't think that's true, and even if it is, it
> needs explanation.
> 

What I think is that, in this particular case, we are sure that
the loop exit condition is met as we know that loc <= RecentFlushPtr.

>>
>> I agree that's a good idea and that it should/would work too. I just
>> wanted to highlight that in this particular
>> case that might not be necessary to build this new API.
> 
> In this case it looks easier to add the right API than to be sure about
> whether it's needed or not.
> 

What I meant is that of course I might be wrong.

If we do not agree that the new API (in this particular case) is not needed then
I agree that building the new API is the way to go ;-) (+ it offers the advantage to
be able to be more precise while reporting the wait event).

Regards,

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

view thread (41+ 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], [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