MIME-Version: 1.0
References: 
 <CAA4eK1KdVRffYEYiv9=4tVWhP=Jkfh1h6FM0A1R0NEe8cLT8+g@mail.gmail.com>
 <CAA4eK1JU4J3-Z=fcMcyQw-iG-DNr=u6w_g-iM_4xwg1pKupkFQ@mail.gmail.com>
 <OS0PR01MB57163693482C5470C41914AF94EA9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAHut+PuYJz-26QWMSRN_HwiJNfYG4ZxEV3RGE-VyZtXXeFm+cw@mail.gmail.com>
 <OS0PR01MB5716C1C1D8244BA86311193894EB9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAD21AoDWd2pXau+pkYWOi87VGYrDD=OxakEDgOyUS+qV9XuAGA@mail.gmail.com>
 <CAA4eK1L58RuUHPxQUjMFrPwRRWrZbPk5CWzmKX2JNi=3TwSruQ@mail.gmail.com>
 <CAD21AoAZLesKYHbauee4wKFwQUE056emC9iFh9VtT_OAv41Jpg@mail.gmail.com>
In-Reply-To: 
 <CAD21AoAZLesKYHbauee4wKFwQUE056emC9iFh9VtT_OAv41Jpg@mail.gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Fri, 23 Dec 2022 08:50:58 +0530
Message-ID: 
 <CAA4eK1LRUjgTsdGF-DbCeP6oq8yh6GzpCoA5cNqaJ_bx3hN-+g@mail.gmail.com>
Subject: Re: Perform streaming logical transactions by background workers and
 parallel apply
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>,
 Peter Smith <smithpb2250@gmail.com>,
	"wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>,
 Dilip Kumar <dilipbalaut@gmail.com>,
	"shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAA4eK1LRUjgTsdGF-DbCeP6oq8yh6GzpCoA5cNqaJ_bx3hN-%2Bg%40mail.gmail.com>
Precedence: bulk

On Thu, Dec 22, 2022 at 6:18 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Dec 22, 2022 at 7:04 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Thu, Dec 22, 2022 at 11:39 AM Masahiko Sawada <sawada.mshk@gmail.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?
>
> Agreed. And we won't need to do that after handling stream_prepare as
> we don't do that now.
>

I think we do this for both prepare and non-prepare cases via
begin_replication_step(). Here, in both cases, as the changes are sent
to the parallel apply worker, we missed in both cases. So, I think it
is better to do in both cases.

> >
> > 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.
>
> I see your point. But looking at other parallel features such as
> parallel queries, parallel vacuum and parallel index creation, we
> don't give such messages even if the number of parallel workers
> actually launched is lower than the ideal. They also bring a big
> performance benefit.
>

Fair enough. Let's remove this LOG message.

-- 
With Regards,
Amit Kapila.