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

2008-07-21 Thread Alvaro Herrera
Simon Riggs wrote:

 On Fri, 2008-07-18 at 01:44 -0400, Tom Lane wrote:
  I agree, this is important for visibility into what's happening.
  The string isn't getting translated so I don't see any big downside
  to applying the patch in back branches.
 
 Patches for 8.3 and CVS HEAD.

Applied, thanks.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

-- 
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] Is autovacuum doing a wraparound-avoiding VACUUM?

2008-07-19 Thread Simon Riggs

On Fri, 2008-07-18 at 01:44 -0400, Tom Lane wrote:
 Simon Riggs [EMAIL PROTECTED] writes:
  On Thu, 2008-07-17 at 17:10 -0400, Alvaro Herrera wrote:
  I don't like your wording though; it feels too verbose (and you're
  losing the ANALYZE in case it's doing both things).  How about 
  
  snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
  autovacuum: VACUUM%s%s, vac
  tab-at_doanalyze ?  ANALYZE : ,
  tab-at_wraparound ?  (wraparound) : );
 
  Yes, looks good.
 
 May I suggest (to prevent wraparound) or something like that?
 Otherwise, +1.
 
  You're not proposing it for 8.3 right?
 
  I think I am. It's an important diagnostic for your other fix.
 
 I agree, this is important for visibility into what's happening.
 The string isn't getting translated so I don't see any big downside
 to applying the patch in back branches.

Patches for 8.3 and CVS HEAD.

-- 
 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.81
diff -c -r1.81 autovacuum.c
*** src/backend/postmaster/autovacuum.c	17 Jul 2008 21:02:31 -	1.81
--- src/backend/postmaster/autovacuum.c	19 Jul 2008 07:58:33 -
***
*** 2657,2664 
  	/* Report the command and possible options */
  	if (tab-at_dovacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
!  autovacuum: VACUUM%s,
!  tab-at_doanalyze ?  ANALYZE : );
  	else
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: ANALYZE);
--- 2657,2665 
  	/* Report the command and possible options */
  	if (tab-at_dovacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
!  autovacuum: VACUUM%s%s,
!  tab-at_doanalyze ?  ANALYZE : ,
!  tab-at_wraparound ?  (to prevent wraparound) : );
  	else
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: ANALYZE);
Index: src/backend/postmaster/autovacuum.c
===
RCS file: /home/sriggs/pg/REPOSITORY/pgsql/src/backend/postmaster/autovacuum.c,v
retrieving revision 1.71.2.4
diff -c -r1.71.2.4 autovacuum.c
*** src/backend/postmaster/autovacuum.c	17 Jul 2008 21:02:41 -	1.71.2.4
--- src/backend/postmaster/autovacuum.c	19 Jul 2008 07:58:43 -
***
*** 291,297 
  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);
--- 291,297 
  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);
***
*** 2633,2639 
  	MemoryContextSwitchTo(old_cxt);
  
  	/* Let pgstat know what we're doing */
! 	autovac_report_activity(vacstmt, relid);
  
  	vacuum(vacstmt, relids, bstrategy, for_wraparound, true);
  }
--- 2633,2639 
  	MemoryContextSwitchTo(old_cxt);
  
  	/* Let pgstat know what we're doing */
! 	autovac_report_activity(vacstmt, relid, for_wraparound);
  
  	vacuum(vacstmt, relids, bstrategy, for_wraparound, true);
  }
***
*** 2650,2656 
   * 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));
--- 2650,2656 
   * 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));
***
*** 2661,2668 
  	/* Report the command and possible options */
  	if (vacstmt-vacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
!  autovacuum: VACUUM%s,
!  vacstmt-analyze ?  ANALYZE : );
  	else
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: ANALYZE);
--- 2661,2669 
  	/* Report the command and possible options */
  	if (vacstmt-vacuum)
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
!  autovacuum: VACUUM%s%s,
!  vacstmt-analyze ?  ANALYZE : ,
!  for_wraparound ?  (to prevent wraparound) : );
  	else
  		snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: ANALYZE);

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

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

2008-07-17 Thread Alvaro Herrera
Simon Riggs wrote:
 
 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.

I just obsoleted this patch.  The new patch should be easier to do
though -- just a one line change I think.

I don't like your wording though; it feels too verbose (and you're
losing the ANALYZE in case it's doing both things).  How about 

snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
autovacuum: VACUUM%s%s, vac
tab-at_doanalyze ?  ANALYZE : ,
tab-at_wraparound ?  (wraparound) : );

You're not proposing it for 8.3 right?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

-- 
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] Is autovacuum doing a wraparound-avoiding VACUUM?

2008-07-17 Thread Simon Riggs

On Thu, 2008-07-17 at 17:10 -0400, Alvaro Herrera wrote:
 Simon Riggs wrote:
  
  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.
 
 I just obsoleted this patch.  The new patch should be easier to do
 though -- just a one line change I think.
 
 I don't like your wording though; it feels too verbose (and you're
 losing the ANALYZE in case it's doing both things).  How about 
 
   snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
   autovacuum: VACUUM%s%s, vac
   tab-at_doanalyze ?  ANALYZE : ,
   tab-at_wraparound ?  (wraparound) : );

Yes, looks good.

Losing the ANALYZE was conscious, but in retrospect is something we
might live to regret. Yours is better.

 You're not proposing it for 8.3 right?

I think I am. It's an important diagnostic for your other fix.

We need to be able to tell the difference between a wraparound and other
weird situations.

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Training, Services and Support


-- 
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] Is autovacuum doing a wraparound-avoiding VACUUM?

2008-07-17 Thread Tom Lane
Simon Riggs [EMAIL PROTECTED] writes:
 On Thu, 2008-07-17 at 17:10 -0400, Alvaro Herrera wrote:
 I don't like your wording though; it feels too verbose (and you're
 losing the ANALYZE in case it's doing both things).  How about 
 
 snprintf(activity, MAX_AUTOVAC_ACTIV_LEN,
 autovacuum: VACUUM%s%s, vac
 tab-at_doanalyze ?  ANALYZE : ,
 tab-at_wraparound ?  (wraparound) : );

 Yes, looks good.

May I suggest (to prevent wraparound) or something like that?
Otherwise, +1.

 You're not proposing it for 8.3 right?

 I think I am. It's an important diagnostic for your other fix.

I agree, this is important for visibility into what's happening.
The string isn't getting translated so I don't see any big downside
to applying the patch in back branches.

regards, tom lane

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