Received: from localhost (unknown [200.46.204.183])
	by postgresql.org (Postfix) with ESMTP id 72CB964FD01
	for <pgsql-patches-postgresql.org@postgresql.org>;
	Wed, 16 Jul 2008 23:44:15 -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 58839-02 for
	<pgsql-patches-postgresql.org@postgresql.org>;
	Wed, 16 Jul 2008 23:44:05 -0300 (ADT)
X-Greylist: from auto-whitelisted by SQLgrey-1.7.6
Received: from a.mail.sonic.net (a.mail.sonic.net [64.142.16.245])
	by postgresql.org (Postfix) with ESMTP id 659EC64FCE4
	for <pgsql-patches@postgresql.org>;
	Wed, 16 Jul 2008 23:44:11 -0300 (ADT)
Received: from newshell.sonic.net (bolt.sonic.net [208.201.242.19])
	by a.mail.sonic.net (8.13.8.Beta0-Sonic/8.13.7) with ESMTP id
	m6H2i7wo017643
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO);
	Wed, 16 Jul 2008 19:44:07 -0700
Received: from bolt.sonic.net
	(IDENT:9Lm+BEJa6wg34oOA7tmBMDPZNFa8e4wV@localhost.localdomain
	[127.0.0.1])
	by newshell.sonic.net (8.13.6/8.12.1) with ESMTP id m6H2i7wS032216;
	Wed, 16 Jul 2008 19:44:07 -0700
Received: (from daveg@localhost)
	by bolt.sonic.net (8.13.6/8.12.1/Submit) id m6H2i7gA032215;
	Wed, 16 Jul 2008 19:44:07 -0700
Date: Wed, 16 Jul 2008 19:44:07 -0700
From: daveg <daveg@sonic.net>
To: pgsql-patches@postgresql.org
Cc: daveg@sonic.net, hari@efrontier.com
Subject: Re: pg_dump lock timeout
Message-ID: <20080717024407.GL11471@sonic.net>
References: <20080511113047.GV5673@sonic.net>
 <20080511130035.GW5673@sonic.net>
	<20080702155626.GC30328@fetter.org>
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="LQksG6bCIzRHxTLp"
Content-Disposition: inline
In-Reply-To: <20080702155626.GC30328@fetter.org>
User-Agent: Mutt/1.4.2.1i
X-Virus-Scanned: Maia Mailguard 1.0.1
X-Spam-Status: No, hits=0 tagged_above=0 required=5 tests=none
X-Spam-Level: 
X-Archive-Number: 200807/170
X-Sequence-Number: 6864


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


Here is an updated version of this patch against head. It builds, runs and
functions as expected. I did not build the sgml.

I've made changes based on various comments as follows:

  - use WAIT_TIME in description consistantly. Reworded for clarity.
    (Stephan Frost)

  - Use a separate query buffer in getTables() (Stephan Frost)
 
  - sets statement_timeout=0 afterwards per new policy (Tom Lane, Marko Kreen)

  - has only --long-option to conserve short option letters (Marko Kreen)

Regards

-dg

-- 
David Gould       daveg@sonic.net      510 536 1443    510 282 0869
If simplicity worked, the world would be overrun with insects.

--LQksG6bCIzRHxTLp
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="pg-dump-timeout.patch"

*** a/doc/src/sgml/ref/pg_dump.sgml
--- b/doc/src/sgml/ref/pg_dump.sgml
***************
*** 557,564 **** PostgreSQL documentation
          This option disables the use of dollar quoting for function bodies,
          and forces them to be quoted using SQL standard string syntax.
         </para>
!      </listitem>
!     </varlistentry>
  
       <varlistentry>
        <term><option>--disable-triggers</></term>
--- 557,564 ----
          This option disables the use of dollar quoting for function bodies,
          and forces them to be quoted using SQL standard string syntax.
         </para>
!       </listitem>
!      </varlistentry>
  
       <varlistentry>
        <term><option>--disable-triggers</></term>
***************
*** 588,593 **** PostgreSQL documentation
--- 588,605 ----
       </varlistentry>
  
       <varlistentry>
+       <term><option>--lock-wait-timeout=<replaceable class="parameter">wait_time</replaceable></option></term>
+       <listitem>
+        <para>
+         Do not wait forever for table locks at the start of the dump. Instead
+         time out and abandon the dump if unable to lock a table within the
+         specified wait time. The wait time may be specified with the same
+         formats as accepted by <command>SET statement_timeout</>.
+        </para>
+       </listitem>
+      </varlistentry>
+ 
+      <varlistentry>
        <term><option>--use-set-session-authorization</></term>
        <listitem>
         <para>
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***************
*** 71,76 **** bool		attrNames;			/* put attr names into insert strings */
--- 71,77 ----
  bool		schemaOnly;
  bool		dataOnly;
  bool		aclsSkip;
+ const char	*lockWaitTimeout;
  
  /* subquery used to convert user ID (eg, datdba) to user name */
  static const char *username_subquery;
***************
*** 265,270 **** main(int argc, char **argv)
--- 266,275 ----
  		{"disable-triggers", no_argument, &disable_triggers, 1},
  		{"no-tablespaces", no_argument, &outputNoTablespaces, 1},
  		{"use-set-session-authorization", no_argument, &use_setsessauth, 1},
+ 		/*
+ 		* long options with arguments and no short option letter
+ 		*/
+ 		{"lock-wait-timeout", required_argument, NULL, 1},
  
  		{NULL, 0, NULL, 0}
  	};
***************
*** 278,283 **** main(int argc, char **argv)
--- 283,289 ----
  	strcpy(g_opaque_type, "opaque");
  
  	dataOnly = schemaOnly = dumpInserts = attrNames = false;
+ 	lockWaitTimeout = NULL;
  
  	progname = get_progname(argv[0]);
  
***************
*** 436,441 **** main(int argc, char **argv)
--- 442,452 ----
  				/* This covers the long options equivalent to -X xxx. */
  				break;
  
+ 			case 1:
+ 				/* lock-wait-timeout */
+ 				lockWaitTimeout = optarg;
+ 				break;
+ 
  			default:
  				fprintf(stderr, _("Try \"%s --help\" for more information.\n"), progname);
  				exit(1);
***************
*** 757,762 **** help(const char *progname)
--- 768,776 ----
  	printf(_("  -F, --format=c|t|p       output file format (custom, tar, plain text)\n"));
  	printf(_("  -v, --verbose            verbose mode\n"));
  	printf(_("  -Z, --compress=0-9       compression level for compressed formats\n"));
+ 	printf(_("  --lock-wait-timeout=WAIT_TIME\n"
+ 			 "                           timeout and fail after waiting WAIT_TIME\n"
+ 			 "                           for a table lock during startup\n"));
  	printf(_("  --help                   show this help, then exit\n"));
  	printf(_("  --version                output version information, then exit\n"));
  
***************
*** 2956,2962 **** getTables(int *numTables)
  	int			ntups;
  	int			i;
  	PQExpBuffer query = createPQExpBuffer();
! 	PQExpBuffer delqry = createPQExpBuffer();
  	PQExpBuffer lockquery = createPQExpBuffer();
  	TableInfo  *tblinfo;
  	int			i_reltableoid;
--- 2970,2976 ----
  	int			ntups;
  	int			i;
  	PQExpBuffer query = createPQExpBuffer();
! 	PQExpBuffer waitquery = createPQExpBuffer();
  	PQExpBuffer lockquery = createPQExpBuffer();
  	TableInfo  *tblinfo;
  	int			i_reltableoid;
***************
*** 3191,3196 **** getTables(int *numTables)
--- 3205,3218 ----
  	i_reltablespace = PQfnumber(res, "reltablespace");
  	i_reloptions = PQfnumber(res, "reloptions");
  
+ 	if (lockWaitTimeout)
+ 	{
+ 		/* Abandon the dump instead of waiting forever for a table lock */
+ 		resetPQExpBuffer(waitquery);
+ 		appendPQExpBuffer(waitquery, "SET statement_timeout = ");
+ 		appendStringLiteralConn(waitquery, lockWaitTimeout, g_conn);
+ 		do_sql_command(g_conn, waitquery->data);
+ 	}
  	for (i = 0; i < ntups; i++)
  	{
  		tblinfo[i].dobj.objType = DO_TABLE;
***************
*** 3259,3264 **** getTables(int *numTables)
--- 3281,3290 ----
  					  tblinfo[i].dobj.name);
  	}
  
+ 	if (lockWaitTimeout)
+ 	{
+ 		do_sql_command(g_conn, "SET statement_timeout = 0");
+ 	}
  	PQclear(res);
  
  	/*
***************
*** 3291,3297 **** getTables(int *numTables)
  	}
  
  	destroyPQExpBuffer(query);
! 	destroyPQExpBuffer(delqry);
  	destroyPQExpBuffer(lockquery);
  
  	return tblinfo;
--- 3317,3323 ----
  	}
  
  	destroyPQExpBuffer(query);
! 	destroyPQExpBuffer(waitquery);
  	destroyPQExpBuffer(lockquery);
  
  	return tblinfo;

--LQksG6bCIzRHxTLp--