Re: releasing ParallelApplyTxnHash when pa_launch_parallel_worker returns NULL

Ted Yu <yuzhihong@gmail.com>

From: Ted Yu <yuzhihong@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: "akapila@postgresql.org" <akapila@postgresql.org>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-01-11T02:20:54Z
Lists: pgsql-hackers

Attachments

On Tue, Jan 10, 2023 at 6:13 PM houzj.fnst@fujitsu.com <
houzj.fnst@fujitsu.com> wrote:

> On Wednesday, January 11, 2023 1:25 AM Ted Yu <yuzhihong@gmail.com> wrote:
>
> > 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.
>
> Thanks for reporting.
>
> ParallelApplyTxnHash is used to cache the state of streaming transactions
> being
> applied. There could be multiple streaming transactions being applied in
> parallel and their information were already saved in ParallelApplyTxnHash,
> so
> we should not release them just because we don't have a worker available to
> handle a new transaction here.
>
> Best Regards,
> Hou zj
>
Hi,

        /* First time through, initialize parallel apply worker state
hashtable. */
        if (!ParallelApplyTxnHash)

I think it would be better if `ParallelApplyTxnHash` is created by the
first successful parallel apply worker.

Please take a look at the new patch and see if it makes sense.

Cheers

Commits

  1. Avoid creating parallel apply state hash table unless required.