Re: Perform streaming logical transactions by background workers and parallel apply

On Thu, Dec 22, 2022 at 11:39 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> Thank you for updating the patch. Here are some comments on v64 patches:
>
> While testing the patch, I realized that if all streamed transactions
> are handled by parallel workers, there is no chance for the leader to
> call maybe_reread_subscription() except for when waiting for the next
> message. Due to this, the leader didn't stop for a while even if the
> subscription gets disabled. It's an extreme case since my test was
> that pgbench runs 30 concurrent transactions and logical_decoding_mode
> = 'immediate', but we might want to make sure to call
> maybe_reread_subscription() at least after committing/preparing a
> transaction.
>

Won't it be better to call it only if we handle the transaction by the
parallel worker?

> ---
> + if (pg_atomic_read_u32(&MyParallelShared->pending_stream_count) == 0)
> + {
> + if (pa_has_spooled_message_pending())
> + return;
> +
> + elog(ERROR, "invalid pending streaming block number");
> + }
>
> I think it's helpful if the error message shows the invalid block number.
>

+1. Additionally, I suggest changing the message to "invalid pending
streaming chunk"?

> ---
> On Wed, Dec 7, 2022 at 10:13 PM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > ---
> > > If a value of max_parallel_apply_workers_per_subscription is not
> > > sufficient, we get the LOG "out of parallel apply workers" every time
> > > when the apply worker doesn't launch a worker. But do we really need
> > > this log? It seems not consistent with
> > > max_sync_workers_per_subscription behavior. I think we can check if
> > > the number of running parallel workers is less than
> > > max_parallel_apply_workers_per_subscription before calling
> > > logicalrep_worker_launch(). What do you think?
> >
> > (Sorry, I missed this comment in last email)
> >
> > I personally feel giving a hint might help user to realize that the
> > max_parallel_applyxxx is not enough for the current workload and then they can
> > adjust the parameter. Otherwise, user might have an easy way to check if more
> > workers are needed.
> >
>
> Sorry, I missed this comment.
>
> I think the number of concurrent transactions on the publisher could
> be several hundreds, and the number of streamed transactions among
> them could be several tens. I agree setting
> max_parallel_apply_workers_per_subscription to a value high enough is
> ideal but I'm not sure we want to inform users immediately that the
> setting value is not enough. I think that with the default value
> (i.e., 2), it will not be enough for many systems and the server logs
> could be flood with the LOG "out of parallel apply workers".
>

It seems currently we give a similar message when the logical
replication worker slots are finished "out of logical replication
worker slots" or when we are not able to register background workers
"out of background worker slots". Now, OTOH, when we exceed the limit
of sync workers "max_sync_workers_per_subscription", we don't display
any message. Personally, I think if any user has used the streaming
option as "parallel" she wants all large transactions to be performed
in parallel and if the system is not able to deal with it, displaying
a LOG message will be useful for users. This is because the
performance difference for large transactions between parallel and
non-parallel is big (30-40%) and it is better for users to know as
soon as possible instead of expecting them to run some monitoring
query to notice the same.

--
With Regards,
Amit Kapila.