Date: Wed, 11 Jan 2017 11:48:25 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: pg_restore accepts -j -1
Message-ID: <20170111164825.GL18360@tamriel.snowman.net>
References: <20170110044815.GC18360@tamriel.snowman.net>
 <CAFjFpRdSP_N8ZL=sXGvmZR1hF=PuieLyK3Xsvh=jr1v2Kf-70w@mail.gmail.com>
 <20170111143643.GK18360@tamriel.snowman.net>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="JXgPQ6VbGllIp4i6"
Content-Disposition: inline
In-Reply-To: <20170111143643.GK18360@tamriel.snowman.net>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


--JXgPQ6VbGllIp4i6
Content-Type: multipart/mixed; boundary="0BR6GWi1HhdMNcLr"
Content-Disposition: inline


--0BR6GWi1HhdMNcLr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Ashutosh,

* Stephen Frost (sfrost@snowman.net) wrote:
> 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.

After playing with this, it seems entirely wrong to wait until after we
try to open the archive before we finish checking the validity of the
options, so I moved these checks up to a more sensible location.

> 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.

The more I think about it, the more it seems like this is something we
should have back-patched when we added that check, so I'll go ahead and
add that check to 9.2 also.

Updated patch attached.

I'll plan to push these changes later on this afternoon.

Thanks!

Stephen

--0BR6GWi1HhdMNcLr
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="check_numworkers_pg_restore_master_v2.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
new file mode 100644
index b21fd26..239b0d8
*** a/src/bin/pg_dump/pg_restore.c
--- b/src/bin/pg_dump/pg_restore.c
*************** main(int argc, char **argv)
*** 330,335 ****
--- 330,351 ----
  		exit_nicely(1);
  	}
 =20
+ 	if (numWorkers <=3D 0)
+ 	{
+ 		fprintf(stderr, _("%s: invalid number of parallel jobs\n"), progname);
+ 		exit(1);
+ 	}
+=20
+ 	/* See comments in pg_dump.c */
+ #ifdef WIN32
+ 	if (numWorkers > MAXIMUM_WAIT_OBJECTS)
+ 	{
+ 		fprintf(stderr, _("%s: maximum number of parallel jobs is %d\n"),
+ 				progname, MAXIMUM_WAIT_OBJECTS);
+ 		exit(1);
+ 	}
+ #endif
+=20
  	/* Can't do single-txn mode with multiple connections */
  	if (opts->single_txn && numWorkers > 1)
  	{
*************** main(int argc, char **argv)
*** 402,417 ****
  	if (opts->tocFile)
  		SortTocFromFile(AH);
 =20
- 	/* See comments in pg_dump.c */
- #ifdef WIN32
- 	if (numWorkers > MAXIMUM_WAIT_OBJECTS)
- 	{
- 		fprintf(stderr, _("%s: maximum number of parallel jobs is %d\n"),
- 				progname, MAXIMUM_WAIT_OBJECTS);
- 		exit(1);
- 	}
- #endif
-=20
  	AH->numWorkers =3D numWorkers;
 =20
  	if (opts->tocSummary)
--- 418,423 ----

--0BR6GWi1HhdMNcLr--

--JXgPQ6VbGllIp4i6
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: Digital signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIcBAEBCgAGBQJYdmHZAAoJEO1sijiDR2RVtWkP/3NRvoKhooDuRQMj+2KK+2nw
UNKdxECBGdGwSX/Zhj24a2Hmwvcv6iME3j4gON1wTsMDiH1ccw29DbaGJc5R99+R
dppRh5+7v4C4bBpG5lECWSWJ4FmVR0CoZTsq+w+21Kn5aG5ZhqUL17KBhDznpHAA
hESb5nKx+qav6gDj4tZXO8K5ALZK78OoMN5hziZ7a9wizMi6FjYFN4pALk5mSc1u
YwAi+t5+UIbWITPXAHLd3AR6MfV6fyMLb7qj6XbpOMK7CO7gjzEjGJ80gnUkXAIm
8riBsiSIgzc5fvEKxW+8YJrLPZYDywzXkmno/0R9autaimwGpIknxUEpi+Qm7+Ys
eXVLXduXdsCkbJ/PPsRvsfPBhcStCdOpkyAdPBCHQAkn/ClxyBV/phwUZe/QYild
6goFft9CN00DgnljrlD+08bn6zvxDWnihaaD7Jeks4LBZUOV7Ko0CISIlCE/2cDI
HQszmvzB0GLBoh4ou2zAvSYbMdWpqVprFNN/u6bhDxlFdk2ooupgMlM0r130O7GK
DOWINtvrjs6lHSMNceDdPECb53pyXbSJBHMCqGXi3Sv+KiHXJW7WYSKlgLnSyOzi
tmb8NSj50Rj7sPGdf1TtTtNAZY75FrJlg3xMwAFFar6kFANihBOhu6yAGLpAR/2s
0fFQA/QT++j1+MxQhPYX
=XrD+
-----END PGP SIGNATURE-----

--JXgPQ6VbGllIp4i6--