Re: [previously on HACKERS] "Compacting" a relation - Mailing list pgsql-patches

From Bruce Momjian
Subject Re: [previously on HACKERS] "Compacting" a relation
Date
Msg-id 200702212216.l1LMG2a06555@momjian.us
Whole thread Raw
In response to Re: [previously on HACKERS] "Compacting" a relation  ("Simon Riggs" <simon@2ndquadrant.com>)
Responses Re: [previously on HACKERS] "Compacting" a relation  (Alvaro Herrera <alvherre@commandprompt.com>)
List pgsql-patches
I applied the optional VACUUM FULL version, but modified to code to say
20% rather than a factor of 5, attached.

---------------------------------------------------------------------------

Simon Riggs wrote:
> On Mon, 2007-02-05 at 11:55 +0000, Simon Riggs wrote:
> > On Sat, 2007-02-03 at 22:11 -0500, Bruce Momjian wrote:
> > > Tom Lane wrote:
> > > > Peter Eisentraut <peter_e@gmx.net> writes:
> > > > > vacuumlazy.c contains a hint "Consider compacting this relation" but AFAICT,
> > > > > there is no indication anywhere how "compacting" is supposed to be achieved.
> > > > > I guess this means VACUUM FULL or CLUSTER, but I don't think the hint can be
> > > > > processed effectively by a user.
> > > >
> > > > So change it ...
> > >
> > > New message is:
> > >
> > >   errhint("Consider using VACUUM FULL on this relation or increasing the configuration parameter
\"max_fsm_pages\".")));
> > >
> >
> > The change of wording may be appropriate, but it is triggered when
> >
> >     if (vacrelstats->tot_free_pages > MaxFSMPages)
> >
> > So if you VACUUM a 15+GB table and it has only 1% freespace then it will
> > still generate this message. Hopefully you'd agree that the message
> > would be inappropriate in that case.
> >
> > It's also inappropriate because this message is generated *prior* to
> > doing lazy_truncate_heap(), which could easily remove lots of empty
> > pages anyhow. That might reduce it to less than MaxFSMPages anyhow, so
> > it can currently be triggered in wholly inappropriate situations.
> >
> > So I suggest that we move this wording after lazy_truncate_heap() in
> > lazy_vacuum_rel() *and* we alter the hint so that it only suggests
> > VACUUM FULL if the table has 20% fragmentation, whatever its size.
> >
> > Happy to drop a patch for this, if people agree.
>
> Enclose 2 versions:
> v1 - move test and WARNING
> v2 - move test and WARNING, plus adjust hint according to relation size
>
> --
>   Simon Riggs
>   EnterpriseDB   http://www.enterprisedb.com
>

[ Attachment, skipping... ]

[ Attachment, skipping... ]

--
  Bruce Momjian  <bruce@momjian.us>          http://momjian.us
  EnterpriseDB                               http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +
Index: src/backend/commands/vacuumlazy.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v
retrieving revision 1.83
diff -c -c -r1.83 vacuumlazy.c
*** src/backend/commands/vacuumlazy.c    4 Feb 2007 03:10:55 -0000    1.83
--- src/backend/commands/vacuumlazy.c    21 Feb 2007 22:13:59 -0000
***************
*** 180,185 ****
--- 180,195 ----
      /* Update shared free space map with final free space info */
      lazy_update_fsm(onerel, vacrelstats);

+     if (vacrelstats->tot_free_pages > MaxFSMPages)
+         ereport(WARNING,
+                 (errmsg("relation \"%s.%s\" contains more than \"max_fsm_pages\" pages with useful free space",
+                         get_namespace_name(RelationGetNamespace(onerel)),
+                         RelationGetRelationName(onerel)),
+                  errhint("Consider%sincreasing the configuration parameter \"max_fsm_pages\".",
+                         /* Only suggest VACUUM FULL if 20% free */
+                         (vacrelstats->tot_free_pages > vacrelstats->rel_pages * 0.20
+                             ? " using VACUUM FULL on this relation or ": " "))));
+
      /* Update statistics in pg_class */
      vac_update_relstats(RelationGetRelid(onerel),
                          vacrelstats->rel_pages,
***************
*** 507,519 ****
                         vacrelstats->tot_free_pages,
                         empty_pages,
                         pg_rusage_show(&ru0))));
-
-     if (vacrelstats->tot_free_pages > MaxFSMPages)
-         ereport(WARNING,
-                 (errmsg("relation \"%s.%s\" contains more than \"max_fsm_pages\" pages with useful free space",
-                         get_namespace_name(RelationGetNamespace(onerel)),
-                         relname),
-                  errhint("Consider using VACUUM FULL on this relation or increasing the configuration parameter
\"max_fsm_pages\".")));
  }


--- 517,522 ----

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Dead code in _bt_split?
Next
From: Alvaro Herrera
Date:
Subject: Re: [previously on HACKERS] "Compacting" a relation