Re: Minimal logical decoding on standbys

Hi,

On 4/3/23 7:20 AM, Amit Kapila wrote:
> On Mon, Apr 3, 2023 at 1:31 AM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>>
>> On Sun, 2023-04-02 at 10:11 +0200, Drouvot, Bertrand wrote:
>>> I was thinking that, if a new LogicalWalSndWakeup() replaces
>>> "ConditionVariableBroadcast(&XLogRecoveryCtl->replayedCV);"
>>> in ApplyWalRecord() then, it could be possible that some walsender(s)
>>> are requested to wake up while they are actually doing decoding (but
>>> I might be wrong).
>>
>> I don't think that's a problem, right?
>>
>
> Agreed, I also don't see a problem because of the reason you mentioned
> below that if the latch is already set, we won't do anything in
> SetLatch.

Thanks for the feedback, I do agree too after Jeff's and your explanation.

>
>> We are concerned about wakeups when they happen repeatedly when there's
>> no work to do, or when the wakeup doesn't happen when it should (and we
>> need to wait for a timeout).
>>
>>>>
>>>> Currently, we wake up walsenders only after writing some WAL
>>>> records
>>>> at the time of flush, so won't it be better to wake up only after
>>>> applying some WAL records rather than after applying each record?
>>>
>>> Yeah that would be better.
>>
>> Why? If the walsender is asleep, and there's work to be done, why not
>> wake it up?
>>
>
> I think we can wake it up when there is work to be done even if the
> work unit is smaller. The reason why I mentioned waking up the
> walsender only after processing some records is to avoid the situation
> where it may not need to wait again after decoding very few records.
> But probably the logic in WalSndWaitForWal() will help us to exit
> before starting to wait by checking the replay location.
>

Okay, I'll re-write the sub-patch related to the startup/walsender corner
case with this new approach.

Regards,

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