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

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: Perform streaming logical transactions by background workers and parallel apply
Date: 2022-12-17 14:04:14
Message-ID: OS0PR01MB5716C3561CF53027F22B47AF94E79@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Saturday, December 17, 2022 8:16 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Dec 16, 2022 at 4:34 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Fri, Dec 16, 2022 at 2:47 PM houzj(dot)fnst(at)fujitsu(dot)com
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > > ---
> > > > + active_workers = list_copy(ParallelApplyWorkerPool);
> > > > +
> > > > + foreach(lc, active_workers)
> > > > + {
> > > > + int slot_no;
> > > > + uint16 generation;
> > > > + ParallelApplyWorkerInfo *winfo =
> > > > (ParallelApplyWorkerInfo *) lfirst(lc);
> > > > +
> > > > + LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
> > > > + napplyworkers =
> > > > logicalrep_pa_worker_count(MyLogicalRepWorker->subid);
> > > > + LWLockRelease(LogicalRepWorkerLock);
> > > > +
> > > > + if (napplyworkers <=
> > > > max_parallel_apply_workers_per_subscription / 2)
> > > > + return;
> > > > +
> > > >
> > > > Calling logicalrep_pa_worker_count() with lwlock for each worker
> > > > seems not efficient to me. I think we can get the number of
> > > > workers once at the top of this function and return if it's
> > > > already lower than the maximum pool size. Otherwise, we attempt to stop
> extra workers.
> > >
> > > How about we directly check the length of worker pool list here
> > > which seems simpler and don't need to lock ?
> > >
> >
> > I don't see any problem with that. Also, if such a check is safe then
> > can't we use the same in pa_free_worker() as well? BTW, shouldn't
> > pa_stop_idle_workers() try to free/stop workers unless the active
> > number reaches below max_parallel_apply_workers_per_subscription?
> >
>
> BTW, can we move pa_stop_idle_workers() functionality to a later patch (say into
> v61-0006*)? That way we can focus on it separately once the main patch is
> committed.

Agreed. I have addressed all the comments and did some cosmetic changes.
Attach the new version patch set.

Best regards,
Hou zj

Attachment Content-Type Size
v62-0008-Add-a-main_worker_pid-to-pg_stat_subscription.patch application/octet-stream 8.7 KB
v62-0002-Serialize-partial-changes-to-a-file-when-the-att.patch application/octet-stream 43.3 KB
v62-0001-Perform-streaming-logical-transactions-by-parall.patch application/octet-stream 191.5 KB
v62-0006-Stop-extra-worker-if-GUC-was-changed.patch application/octet-stream 4.7 KB
v62-0003-Test-streaming-parallel-option-in-tap-test.patch application/octet-stream 80.1 KB
v62-0004-Allow-streaming-every-change-without-waiting-til.patch application/octet-stream 9.4 KB
v62-0005-Add-GUC-stream_serialize_threshold-and-test-seri.patch application/octet-stream 12.5 KB
v62-0007-Retry-to-apply-streaming-xact-only-in-apply-work.patch application/octet-stream 22.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2022-12-17 14:30:02 Re: Progress report of CREATE INDEX for nested partitioned tables
Previous Message Ted Yu 2022-12-17 12:39:29 Re: allow granting CLUSTER, REFRESH MATERIALIZED VIEW, and REINDEX