On 01 July 2014 03:31, Jeff Janes Wrote,
>
> I get a compiler warning when building on Windows. When I started
> looking into that, I see that two files have too much code duplication
> between them:
Thanks for Reviewing,
>
> src/bin/scripts/vac_parallel.c (new file)
> src/bin/pg_dump/parallel.c (existing file)
>
> In particular, pgpipe is almost an exact duplicate between them, except
> the copy in vac_parallel.c has fallen behind changes made to parallel.c.
> (Those changes would have fixed the Windows warnings). I think that
> this function (and perhaps other parts as well--"exit_horribly" for
> example) need to refactored into a common file that both files can
> include. I don't know where the best place for that would be, though.
> (I haven't done this type of refactoring
> myself.)
When I started doing this patch, I thought of sharing the common code b/w vacuumdb and pg_dump, But if we notice
Pg_dump code is tightly coupled with ArchiveHandle, almost all function take this parameter as input or they operate on
this,and other functions
uses some structure like ParallelState or ParallelSlot which has ArchiveHandle member. I think making this code common
mayneed to change complete code of
Parallel pg_dump.
However there are some function which are independent of Archive Handle and can directly move to common code,
As you mention pg_pipe, piperead, readMessageFromPipe, select_loop.
For moving them to common place we need to decide where the common file to be placed.
Thoughts ?
>
> Also, there are several places in the patch which use spaces for
> indentation where tabs are called for by the coding style. It looks
> like you may have copied the code from one terminal window and copied
> it into another one, converting tabs to spaces in the process. This
> makes it hard to evaluate the amount of code duplication.
>
> In some places the code spins in a tight loop while waiting for a
> worker process to become free. If I strace the process, I got a long
> list of selects with 0 time outs:
>
> select(13, [6 8 10 12], NULL, NULL, {0, 0}) = 0 (Timeout)
>
> I have not tried to track down the code that causes it. I did notice
> that vacuumdb spends an awful lot of time at the top of the Linux "top"
> output, and this is probably why.
I will look into these and fix..
Thanks & Regards,
Dilip Kumar