Re: Add parallelism and glibc dependent only options to reindexdb

On Thu, Jul 11, 2019 at 3:34 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Jul 11, 2019 at 11:48:20AM +0200, Julien Rouhaud wrote:
> > On Thu, Jul 11, 2019 at 6:04 AM Michael Paquier <michael@paquier.xyz> wrote:
> >> Get* would be my choice, because we look at the set of parallel slots,
> >> and get an idle one.
> >
> > That's what the former GetIdleSlot (that I renamed to get_idle_slot as
> > it's not static) is doing.  ConsumeIdleSlot() actually get an idle
> > slot and mark it as being used.  That's probably some leakage of
> > internal implementation, but having a GetIdleParallelSlot (or
> > ParallelSlotGetIdle) *and* a get_idle_slot sounds like a bad idea, and
> > I don't have a better idea on how to rename get_idle_slot.  If you
> > really prefer Get* and you're fine with a static get_idle_slot, that's
> > fine by me.
>
> Do we actually need get_idle_slot?  ConsumeIdleSlot is its only
> caller.

I think t hat it makes the code quite cleaner to have the selection
outside ConsumeIdleSlot.

> And while scanning the code...
>
> + * getQuerySucess
> Typo here.

Argh, I thought I caught all of them, thanks!

> - * Pump the conn till it's dry of results; return false if any are errors.
> This comment could be improved on the way, like "Go through all the
> connections and make sure to consume any remaining results.  If any
> error is found, false is returned after processing all the parallel
> slots."

You're talking about getQuerySuccess right? That was actually the
original comment of a function I renamed.  +1 to improve it, but this
function is in common.c and doesn't deal with parallel slot at all, so
I'll just drop the slang parts.