From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
---|---|
To: | Simon Riggs <simon(at)2ndQuadrant(dot)com> |
Cc: | Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: New VACUUM FULL |
Date: | 2009-12-04 17:56:52 |
Message-ID: | 1259949412.19545.245.camel@jdavis |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, 2009-12-04 at 09:20 +0000, Simon Riggs wrote:
> On Tue, 2009-12-01 at 01:43 -0800, Jeff Davis wrote:
> > Marking as ready.
>
> You're saying its ready, yet there are 3 additional suggestions patches
> attached here. Please can you resolve these and re-submit a single final
> patch from author and reviewer?
My apologies. At the time, I thought a couple days might matter, and the
changes are in areas that committers tend to editorialize anyway: docs
and a style issue. The only substantial patch was to vacuumdb.c.
Complete patch attached including my edits.
> * What happens if you issue VACUUM FULL; which we would expect to use
> the new method of vacuum on all tables in the database. Won't that just
> fail with an error when it comes to catalog tables? Sounds to me like we
> should avoid the error and just silently do an INPLACE on catalog
> tables.
That's how it works.
> * Such a pivotal change to Postgres needs more test coverage than a
> single line in regression tests. It might have been OK before, but I
> think we need a few more combinations here, at least in this release:
> with, without indexes, empty table, clustered, non-clustered etc and of
> course a full database VACUUM so that we have the catalog table case
> covered, plus an explicit catalog table vacuum.
It was my impression that the regression tests aren't meant to be
exhaustive, but merely exercise a good portion of the code to help
detect simple breakage. Also, pg_regress isn't good for detecting a lot
of the problems that vacuum might have (how do you even know whether the
vacuum happened in-place or not?).
We could put a VACUUM FULL; and a VACUUM (FULL INPLACE); somewhere,
which will cover a lot of the cases you're talking about. However, that
may be a performance consideration especially for people who develop on
laptops.
In general, though, I think the right place for this is a longer test
suite that is meant to be run less frequently.
Regards,
Jeff Davis
Attachment | Content-Type | Size |
---|---|---|
vacuum-full-20091204.context.patch | text/x-patch | 36.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2009-12-04 17:57:41 | Re: operator exclusion constraints |
Previous Message | Kern Sibbald | 2009-12-04 17:46:37 | Re: Catastrophic changes to PostgreSQL 8.4 |