Re: pg_dump directory archive format / parallel pg_dump

On Tue, Feb 1, 2011 at 11:32 PM, Joachim Wieland <joe(at)mcknight(dot)de> wrote:
> On Sun, Jan 30, 2011 at 5:26 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> The parallel pg_dump portion of this patch (i.e. the still-uncommitted
>> part) no longer applies.  Please rebase.
>
> Here is a rebased version with some minor changes as well. I haven't
> tested it on Windows now but will do so as soon as the Unix part has
> been reviewed.
>

code review:

something i found, and is a very simple one, is this warning (there's
a similar issue in _StartMasterParallel with the buf variable)
"""
pg_backup_directory.c: In function ‘_EndMasterParallel’:
pg_backup_directory.c:856: warning: ‘status’ may be used uninitialized
in this function
"""

i guess the huge amount of info is showing the patch is just for
debugging and will be removed before commit, right?

functional review:

it works good most of the time, just a few points:
- if i interrupt the process the connections stay, i guess it could
catch the signal and finish the connections
- if i have an exclusive lock on a table and a worker starts dumping
it, it fails because it can't take the lock but it just say "it was
ok" and would prefer an error

--
Jaime Casanova         www.2ndQuadrant.com
Professional PostgreSQL: Soporte y capacitación de PostgreSQL