Hi,
On 12/5/23 11:29 AM, shveta malik wrote:
> On Tue, Dec 5, 2023 at 2:18 PM Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.com> wrote:
>> Wouldn't that make sense to move it once we are sure that
>> walrcv_startstreaming() returns true and first_stream is true, here?
>>
>> "
>> if (first_stream)
>> + {
>> ereport(LOG,
>> (errmsg("started streaming WAL from primary at %X/%X on timeline
%u",
>> LSN_FORMAT_ARGS(startpoint), startpointTLI)));
>> + SpinLockAcquire(&walrcv->mutex);
>> + walrcv->walRcvState = WALRCV_STREAMING;
>> + SpinLockRelease(&walrcv->mutex);
>> + }
>> "
>>
>
> Yes, it makes sense and is the basis for current slot-sync worker
> changes being discussed.
I think this change deserves its own dedicated thread and patch, does
that make sense?
If so, I'll submit one.
>>
>> 2) and 3) looks good to me but with a check on walrcv->walRcvState
>> looking for WALRCV_STREAMING state instead of looking for a non null
>> WalRcv->pid.
>
> yes. But I think, the worker should enter no-op, when walRcvState is
> WALRCV_STOPPED and not when walRcvState != WALRCV_STREAMING as it is
> okay to have WALRCV_WAITING/STARTING/RESTARTING. But the worker should
> exit no-op only when it finds walRcvState switched back to
> WALRCV_STREAMING.
>
Yeah, fully agree.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com