Saturday, July 19, 2008

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

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 -0000 1.81
--- src/backend/postmaster/autovacuum.c 19 Jul 2008 07:58:33 -0000
***************
*** 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 -0000 1.71.2.4
--- src/backend/postmaster/autovacuum.c 19 Jul 2008 07:58:43 -0000
***************
*** 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");
On Fri, 2008-07-18 at 01:44 -0400, Tom Lane wrote:
> Simon Riggs <simon@2ndquadrant.com> 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

No comments: