Обсуждение: VACUUM FULL/CLUSTER doesn't update pg_class's pg_class.relfrozenxid

Hi,

As Robert previously complained a database wide VACUUM FULL now (as of
3cff1879f8d03) reliably increases the relfrozenxid for all tables but
pg_class itself. That's a bit sad because it means doing a VACUUM FULL
won't help in a anti-wraparound scenario.

The reason for that is explained by the following comment:
    /*
     * Update the tuples in pg_class --- unless the target relation of the
     * swap is pg_class itself.  In that case, there is zero point in making
     * changes because we'd be updating the old data that we're about to throw
     * away.  Because the real work being done here for a mapped relation is
     * just to change the relation map settings, it's all right to not update
     * the pg_class rows in this case.
     */

I think the easiest fix for that is to update pg_class' relfrozenxid in
finish_heap_swap() after the indexes have been rebuilt, that's just a
couple of lines. There's more complex solutions that'd avoid the need
for that special case, but I it's sufficient. A patch doing that is
attached.

Note that VACUUM FULL will still require more xids than a plain VACUUM,
but it scales linearly with the number of relations, so I have a hard
time seing that as problematic.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Thu, Feb 27, 2014 at 1:06 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> As Robert previously complained a database wide VACUUM FULL now (as of
> 3cff1879f8d03) reliably increases the relfrozenxid for all tables but
> pg_class itself. That's a bit sad because it means doing a VACUUM FULL
> won't help in a anti-wraparound scenario.
>
> The reason for that is explained by the following comment:
>         /*
>          * Update the tuples in pg_class --- unless the target relation of the
>          * swap is pg_class itself.  In that case, there is zero point in making
>          * changes because we'd be updating the old data that we're about to throw
>          * away.  Because the real work being done here for a mapped relation is
>          * just to change the relation map settings, it's all right to not update
>          * the pg_class rows in this case.
>          */
>
> I think the easiest fix for that is to update pg_class' relfrozenxid in
> finish_heap_swap() after the indexes have been rebuilt, that's just a
> couple of lines. There's more complex solutions that'd avoid the need
> for that special case, but I it's sufficient. A patch doing that is
> attached.

So, this patch is obviously after the final CommitFest deadline, but
I'd like to commit it to 9.4 anyway on admittedly-arguable theory that
it's tying up a loose end introduced by
3cff1879f8d03cb729368722ca823a4bf74c0cac.  Prior to that commit,
VACUUM FULL and CLUSTER *never* updated relfrozenxid; beginning with
that commit, they do so for all relations except pg_class.  This
tidies up that omission.

And I think that's a pretty worthwhile thing to do, because we get
periodic reports from people who have run VACUUM FULL on a database in
danger of wraparound and then wondered why it did not fix the problem.The previously-mentioned commit did most of the
heavylifting as far
 
as tidying that up, but without this adjustment it won't quite get us
over the hump.

But all that having been said, a deadline is a deadline, so if anyone
wishes to declare this untimely please speak up.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2014-03-03 07:52:23 -0500, Robert Haas wrote:
> And I think that's a pretty worthwhile thing to do, because we get
> periodic reports from people who have run VACUUM FULL on a database in
> danger of wraparound and then wondered why it did not fix the problem.
>  The previously-mentioned commit did most of the heavy lifting as far
> as tidying that up, but without this adjustment it won't quite get us
> over the hump.
> 
> But all that having been said, a deadline is a deadline, so if anyone
> wishes to declare this untimely please speak up.

Now, I am obviously not impartial here, but I think it doesn't make
sense to whack this code around in two releases if not necessary.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On Mon, Mar 3, 2014 at 7:52 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> But all that having been said, a deadline is a deadline, so if anyone
> wishes to declare this untimely please speak up.

Hearing only crickets, committed.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company