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 |
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 |