On Thu, Jul 11, 2019 at 3:34 PM Michael Paquier <mich...@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 <mich...@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.