On Fri, Dec 16, 2022 at 4:34 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Dec 16, 2022 at 2:47 PM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.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. -- With Regards, Amit Kapila.