Message-ID: <bb6f1c0d-0c1e-2ef3-97f2-ce90e964fc5a@gmail.com>
Date: Fri, 3 Mar 2023 17:23:36 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.8.0
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
Subject: Re: Minimal logical decoding on standbys
To: Jeff Davis <pgsql@j-davis.com>, 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>
References: <CA+TgmoZ8-DXtDMMDGggFkSYyO8nmNdBKWjV6Ts-zYGR9p5sK5g@mail.gmail.com>
 <CA+TgmoYTTsxP8y6uknZvCBNCRq+1FJ4zGbX8Px1TGW459fGsaQ@mail.gmail.com>
 <1f2ad9fd-53ff-74cc-a602-72d45423bcab@gmail.com>
 <a78de011-81cf-729c-e110-e6d636fdca4d@gmail.com>
 <05c0c4f3-82eb-bc6d-c876-5a265c05927b@gmail.com>
 <CA+TgmoY8LAaTwAhS+YNQNrM+3K8eqxGvPT3gbJaHER86XShvFw@mail.gmail.com>
 <20230106034036.2m4qnn7ep7b5ipet@awork3.anarazel.de>
 <490206c5-bcde-e558-628a-dc67dae4d439@gmail.com>
 <20230119024617.jzsgpcrgjdc6ysge@awork3.anarazel.de>
 <a343b646-c632-4b9a-0e01-b661075d1d8b@gmail.com>
 <b74520d5-5b9e-1477-2abc-95ba897f23e4@gmail.com>
 <94d9d47e-72f4-393a-246e-e3fe8db7b741@gmail.com>
 <a4c03fad-2986-ddab-c12e-ea2ee0119181@gmail.com>
 <2d314c22b9e03415aa1c7d8fd1f698dae60effa7.camel@j-davis.com>
 <061eeb26-156e-6011-02b6-b4b1422e2aa6@gmail.com>
 <ec744999afcdbd6bec5d9e6572aba875060019cb.camel@j-davis.com>
 <8a7da0c2-b7fe-1c78-4e47-a16239bf4839@gmail.com>
 <d29e4458e7406b276c50197d9f3b31a5247fe915.camel@j-davis.com>
Content-Language: en-US
In-Reply-To: <d29e4458e7406b276c50197d9f3b31a5247fe915.camel@j-davis.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/bb6f1c0d-0c1e-2ef3-97f2-ce90e964fc5a%40gmail.com>
Precedence: bulk

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