Re: pg_restore accepts -j -1

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_restore accepts -j -1
Date: 2017-01-11 14:36:43
Message-ID: 20170111143643.GK18360@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Ashutosh,

* Ashutosh Bapat (ashutosh(dot)bapat(at)enterprisedb(dot)com) wrote:
> On Tue, Jan 10, 2017 at 10:18 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > For reasons which seem likely to be entirely unintentional, pg_restore
> > will accept a '-1' for -j:
> >
> > pg_restore -j -1
> >
> > This seems to result in the parallel state being NULL and so things
> > don't outright break, but it hardly seems likely to be what the user was
> > asking for- my guess is that they actually wanted "parallel, single
> > transaction", which we don't actually support:
> >
> > -> pg_restore -j 2 -1
> > pg_restore: cannot specify both --single-transaction and multiple jobs
> >
> > We also don't accept -1 for pg_dump:
> >
> > -> pg_dump -j -1
> > pg_dump: invalid number of parallel jobs
> >
> > If I'm missing something, please let me know, otherwise I'll plan to put
> > the same check into pg_restore which exists in pg_dump.
>
> Both the code blocks were added by 9e257a18, but I don't see any
> description of why they are different in pg_dump.c and pg_restore.c.

Right.

> In fact per comments in pg_restore.c, that condition should be same as
> pg_dump.c. I am not sure whether it's just for windows specific
> condition or the whole block. But I don't see any reason not to
> replicate the same conditions in pg_restore.c

I'm pretty sure that comment is about the Windows-specific check of
MAXIMUM_WAIT_OBJECTS, but I don't think there's any reason to accept a
zero or negative value for numWorkers.

Attached patch adds the same check to pg_restore that's in pg_dump
already. Looks like it should back-patch to 9.3 pretty cleanly and I'll
add a similar check for 9.2.

Any thoughts about adding the Windows-specific MAXIMUM_WAIT_OBJECTS
check to 9.2 pg_restore..? It certainly looks entirely straight-forward
to do, and though I don't recall hearing anyone complaining about trying
to run pg_restore on Windows with lots of jobs and having it fall over,
it might avoid a bit of frustration for anyone who does try.

Thanks!

Stephen

Attachment Content-Type Size
check_numworkers_pg_restore.patch text/x-diff 532 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Merlin Moncure 2017-01-11 14:37:13 Re: merging some features from plpgsql2 project
Previous Message Pavel Stehule 2017-01-11 14:27:48 Re: plpgsql - additional extra checks