On Thu, Sep 22, 2022 at 1:37 PM kuroda.hayato@fujitsu.com
<kuroda.hayato@fujitsu.com> wrote:
>
> ===
> applyparallelworker.c
>
> 03. declaration
>
> ```
> +/*
> + * Is there a message pending in parallel apply worker which we need to
> + * receive?
> + */
> +volatile bool ParallelApplyMessagePending = false;
> ```
>
> I checked other flags that are set by signal handlers, their datatype seemed to be sig_atomic_t.
> Is there any reasons that you use normal bool? It should be changed if not.
>
It follows the logic similar to ParallelMessagePending. Do you see any
problem with it?
> 04. HandleParallelApplyMessages()
>
> ```
> + if (winfo->error_mq_handle == NULL)
> + continue;
> ```
>
> a.
> I was not sure when the cell should be cleaned. Currently we clean up ParallelApplyWorkersList() only in the
parallel_apply_start_worker(),
> but we have chances to remove such a cell like HandleParallelApplyMessages() or HandleParallelApplyMessage(). How do
youthink?
>
Note that HandleParallelApply* are invoked during interrupt handling,
so it may not be advisable to remove it there.
>
> 12. ConfigureNamesInt
>
> ```
> + {
> + {"max_parallel_apply_workers_per_subscription",
> + PGC_SIGHUP,
> + REPLICATION_SUBSCRIBERS,
> + gettext_noop("Maximum number of parallel apply workers per subscription."),
> + NULL,
> + },
> + &max_parallel_apply_workers_per_subscription,
> + 2, 0, MAX_BACKENDS,
> + NULL, NULL, NULL
> + },
> ```
>
> This parameter can be changed by pg_ctl reload, so the following corner case may be occurred.
> Should we add a assign hook to handle this? Or, can we ignore it?
>
I think we can ignore this as it will eventually start respecting the threshold.
--
With Regards,
Amit Kapila.
