Re: releasing ParallelApplyTxnHash when pa_launch_parallel_worker returns NULL

On Tue, Jan 10, 2023 at 9:43 AM Ted Yu <yuzhihong(at)gmail(dot)com> wrote:

>
>
> On Tue, Jan 10, 2023 at 9:26 AM Ted Yu <yuzhihong(at)gmail(dot)com> wrote:
>
>>
>>
>> On Tue, Jan 10, 2023 at 9:25 AM Ted Yu <yuzhihong(at)gmail(dot)com> wrote:
>>
>>> Hi,
>>> I was reading src/backend/replication/logical/applyparallelworker.c .
>>> In `pa_allocate_worker`, when pa_launch_parallel_worker returns NULL, I
>>> think the `ParallelApplyTxnHash` should be released.
>>>
>>> Please see the patch.
>>>
>>> Thanks
>>>
>> Here is the patch :-)
>>
>
> In `pa_process_spooled_messages_if_required`, the `pa_unlock_stream` call
> immediately follows `pa_lock_stream`.
> I assume the following is the intended sequence of calls. If this is the
> case, I can add it to the patch.
>
> Cheers
>
> diff --git a/src/backend/replication/logical/applyparallelworker.c
> b/src/backend/replication/logical/applyparallelworker.c
> index 2e5914d5d9..9879b3fff2 100644
> --- a/src/backend/replication/logical/applyparallelworker.c
> +++ b/src/backend/replication/logical/applyparallelworker.c
> @@ -684,9 +684,9 @@ pa_process_spooled_messages_if_required(void)
> if (fileset_state == FS_SERIALIZE_IN_PROGRESS)
> {
> pa_lock_stream(MyParallelShared->xid, AccessShareLock);
> - pa_unlock_stream(MyParallelShared->xid, AccessShareLock);
>
> fileset_state = pa_get_fileset_state();
> + pa_unlock_stream(MyParallelShared->xid, AccessShareLock);
> }
>
> /*
>
Looking closer at the comment above this code and other part of the file,
it seems the order is intentional.

Please disregard my email about `pa_process_spooled_messages_if_required`.