Received: from localhost (unknown [200.46.204.183]) by postgresql.org (Postfix) with ESMTP id 986336501C9; Wed, 16 Jul 2008 09:37:47 -0300 (ADT) Received: from postgresql.org ([200.46.204.86]) by localhost (mx1.hub.org [200.46.204.183]) (amavisd-maia, port 10024) with ESMTP id 99080-08; Wed, 16 Jul 2008 09:37:38 -0300 (ADT) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from b.mail.sonic.net (b.mail.sonic.net [64.142.19.5]) by postgresql.org (Postfix) with ESMTP id 834F26501B0; Wed, 16 Jul 2008 09:37:43 -0300 (ADT) Received: from newshell.sonic.net (bolt.sonic.net [208.201.242.19]) by b.mail.sonic.net (8.13.8.Beta0-Sonic/8.13.7) with ESMTP id m6GCbaIW019905 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 16 Jul 2008 05:37:36 -0700 Received: from bolt.sonic.net (IDENT:obx1yuq7IfFvt1atQa7PO8bbcyp2uSnd@localhost.localdomain [127.0.0.1]) by newshell.sonic.net (8.13.6/8.12.1) with ESMTP id m6GCbabu004133; Wed, 16 Jul 2008 05:37:36 -0700 Received: (from daveg@localhost) by bolt.sonic.net (8.13.6/8.12.1/Submit) id m6GCbaJP004132; Wed, 16 Jul 2008 05:37:36 -0700 Date: Wed, 16 Jul 2008 05:37:36 -0700 From: daveg To: Marko Kreen Cc: pgsql-patches@postgresql.org, hari@efrontier.com, Postgres Hackers Subject: Re: [PATCHES] pg_dump lock timeout Message-ID: <20080716123736.GJ11471@sonic.net> References: <20080511113047.GV5673@sonic.net> <20080703125501.GS866@sonic.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20080703125501.GS866@sonic.net> User-Agent: Mutt/1.4.2.1i X-Virus-Scanned: Maia Mailguard 1.0.1 X-Archive-Number: 200807/769 X-Sequence-Number: 121029 On Thu, Jul 03, 2008 at 05:55:01AM -0700, daveg wrote: > On Thu, Jul 03, 2008 at 11:15:10AM +0300, Marko Kreen wrote: > > On 5/11/08, daveg wrote: > > > Attached is a patch to add a commandline option to pg_dump to limit how long > > > pg_dump will wait for locks during startup. > > > > My quick review: > > > > - It does not seem important enough to waste a short option on. > > Having only long option should be enough. > > Agreed. I'll change it. > > > - It would be more polite to do SET LOCAL instead SET. > > (Eg. it makes safer to use pg_dump through pooler.) > > Also agreed. Thanks. On second glance, pg_dump sets lots of variables without using SET LOCAL. I think fixing that must be the subject of a separate patch as fixing just one of many will only cause confusion. > > - The statement_timeout is set back with "statement_timeout = default" > > Maybe it would be better to do "= 0" here? Although such decision > > would go outside the scope of the patch, I see no sense having > > any other statement_timeout for actual dumping. > > I'd prefer to leave whatever policy is otherwise in place alone. I can see > use cases for either having or not having a timeout for pg_dump, but it does > seem outside the scope of this patch. As it happens, another patch has set the policy to "statement_timeout = 0", so I will follow that. I'm sending in the revised patch today. -dg -- David Gould daveg@sonic.net 510 536 1443 510 282 0869 If simplicity worked, the world would be overrun with insects.