Re: Minimal logical decoding on standbys

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