Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]

On Mon, Dec 8, 2014 at 7:33 AM, Dilip kumar <dilip(dot)kumar(at)huawei(dot)com> wrote:
> On 06 December 2014 20:01 Amit Kapila Wrote
>
> >I wanted to understand what exactly the above loop is doing.
>
>
>
> >a.
>
> >first of all the comment on top of it says "Some of the slot
>
> >are free, ...", if some slot is free, then why do you want
>
> >to process the results? (Do you mean to say that *None* of
>
> >the slot is free....?)
>
>
>
> This comment is wrong, I will remove this.
>

I suggest rather than removing, edit the comment to indicate
the idea behind code at that place.

> >b.
>
> >IIUC, you have called function select_loop(maxFd, &slotset)
>
> >to check if socket descriptor is readable, if yes then why
>
> >in do..while loop the same maxFd is checked always, don't
>
> >you want to check different socket descriptors? I am not sure
>
> >if I am missing something here
>
>
>
> select_loop(maxFd, &slotset)
>
>
> So it’s not only for a maxFd, it’s for all the descriptors. And it’s in
do..while loop, because it possible that select_loop come out because of
some intermediate message on any of the socket but still query is not
complete,
>

Okay, I think this part of code is somewhat similar to what
is done in pg_dump/parallel.c with some differences related
to handling of inAbort. One thing I have noticed here which
could lead to a problem is that caller of select_loop() function
assumes that return value is less than zero only if there is a
cancel request which I think is wrong, because select system
call could also return -1 in case of error. I am referring below
code in above context:

+ if (i < 0)
+ {
+ /*
+ * This can only happen if user has sent the cancel request using
+ * Ctrl+C, Cancel is handled by 0th slot, so fetch the error result.
+ */
+
+ GetQueryResult(pSlot[0].connection, dbname, progname,
+ completedb);

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com