On Fri, Jul 12, 2019 at 3:20 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Jul 11, 2019 at 06:22:25PM +0200, Julien Rouhaud wrote:
> > I think t hat it makes the code quite cleaner to have the selection
> > outside ConsumeIdleSlot.
>
> Actually, you have an issue with ConsumeIdleSlot() if there is only
> one parallel slot, no? In this case the current patch returns
> immediately the slot available without waiting. I think that we
> should wait until the slot becomes free in that case as well, and
> switch isFree to false.
It shouldn't be a problem, I reused the same infrastructure as for
vacuumdb. so run_reindex_command has a new "async" parameter, so when
there's no parallelism it's using executeMaintenanceCommand (instead
of PQsendQuery) which will block until query completion. That's why
there's no isFree usage at all in this case.
> If you want to keep things splitted, that's
> fine by me, I would still use "Get" within the name for the routine,
> and rename the other to get_idle_slot_internal() or
> get_idle_slot_guts() to point out that it has an internal role.
Ok, I'll change to get_idle_slot_internal then.