Re: [PATCHES] pg_dump lock timeout

2008-07-16 Thread daveg
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 [EMAIL PROTECTED] 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   [EMAIL PROTECTED]  510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


[PATCHES] Is autovacuum doing a wraparound-avoiding VACUUM?

2008-07-16 Thread Simon Riggs

Is autovacuum doing a wraparound-avoiding VACUUM?
Currently, no easy way to tell.

Patch to change message of autovac in pg_stat_activity when we are
performing an anti-wraparound VACUUM.

We will then be able to explain why an autovacuum process doesn't get
cancelled, like we might otherwise hope it would be.

That way we can tell difference between hung and just important.

Perhaps message should say non-automatically cancelled VACUUM
tablename, but that sounded more obscure than the phrase I selected.

Discuss...

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support
Index: src/backend/postmaster/autovacuum.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.80
diff -c -r1.80 autovacuum.c
*** src/backend/postmaster/autovacuum.c	1 Jul 2008 02:09:34 -	1.80
--- src/backend/postmaster/autovacuum.c	16 Jul 2008 16:23:36 -
***
*** 290,296 
  static PgStat_StatTabEntry *get_pgstat_tabentry_relid(Oid relid, bool isshared,
  		  PgStat_StatDBEntry *shared,
  		  PgStat_StatDBEntry *dbentry);
! static void autovac_report_activity(VacuumStmt *vacstmt, Oid relid);
  static void avl_sighup_handler(SIGNAL_ARGS);
  static void avl_sigusr1_handler(SIGNAL_ARGS);
  static void avl_sigterm_handler(SIGNAL_ARGS);
--- 290,296 
  static PgStat_StatTabEntry *get_pgstat_tabentry_relid(Oid relid, bool isshared,
  		  PgStat_StatDBEntry *shared,
  		  PgStat_StatDBEntry *dbentry);
! static void autovac_report_activity(VacuumStmt *vacstmt, Oid relid, bool for_wraparound);
  static void avl_sighup_handler(SIGNAL_ARGS);
  static void avl_sigusr1_handler(SIGNAL_ARGS);
  static void avl_sigterm_handler(SIGNAL_ARGS);
***
*** 2626,2632 
  	vacstmt.va_cols = NIL;
  
  	/* Let pgstat know what we're doing */
! 	autovac_report_activity(vacstmt, relid);
  
  	vacuum(vacstmt, relid, bstrategy, for_wraparound, true);
  }
--- 2626,2632 
  	vacstmt.va_cols = NIL;
  
  	/* Let pgstat know what we're doing */
! 	autovac_report_activity(vacstmt, relid, for_wraparound);
  
  	vacuum(vacstmt, relid, bstrategy, for_wraparound, true);
  }
***
*** 2643,2649 
   * bother to report IDLE or some such.
   */
  static void
! autovac_report_activity(VacuumStmt *vacstmt, Oid relid)
  {
  	char	   *relname = get_rel_name(relid);
  	char	   *nspname = get_namespace_name(get_rel_namespace(relid));
--- 2643,2649 
   * bother to report IDLE or some such.
   */
  static void
! autovac_report_activity(VacuumStmt *vacstmt, Oid relid, bool for_wraparound)
  {
  	char	   *relname = get_rel_name(relid);
  	char	   *nspname = get_namespace_name(get_rel_namespace(relid));
***
*** 2652,2658 
  	char		activity[MAX_AUTOVAC_ACTIV_LEN];
  
  	/* Report the command and possible options */
! 	if (vacstmt-vacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: VACUUM%s,
   vacstmt-analyze ?  ANALYZE : );
--- 2652,2661 
  	char		activity[MAX_AUTOVAC_ACTIV_LEN];
  
  	/* Report the command and possible options */
! 	if (for_wraparound)
! 		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
!  autovacuum: VACUUM to avoid wraparound of );
! 	else if (vacstmt-vacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: VACUUM%s,
   vacstmt-analyze ?  ANALYZE : );

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] pg_dump lock timeout

2008-07-16 Thread daveg

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   [EMAIL PROTECTED]  510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.
*** 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
termoption--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
termoption--disable-triggers//term
***
*** 588,593  PostgreSQL documentation
--- 588,605 
   /varlistentry
  
   varlistentry
+   termoption--lock-wait-timeout=replaceable 
class=parameterwait_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 commandSET statement_timeout/.
+/para
+   /listitem
+  /varlistentry
+ 
+  varlistentry
termoption--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, --verboseverbose 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(_(  --versionoutput 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