Thread: pgsql: Fix two bugs induced in VACUUM FULL by async-commit patch.
pgsql: Fix two bugs induced in VACUUM FULL by async-commit patch.
From
tgl@postgresql.org (Tom Lane)
Date:
Log Message:
-----------
Fix two bugs induced in VACUUM FULL by async-commit patch.
First, we cannot assume that XLogAsyncCommitFlush guarantees hint bits will be
settable, because clog.c's inexact LSN bookkeeping results in windows where a
previously flushed transaction is considered unhintable because it shares an
LSN slot with a later unflushed transaction. But repair_frag requires
XMIN_COMMITTED to be correct so that it can distinguish tuples moved by the
current vacuum. Since not being able to set the bit is an uncommon corner
case, the most practical way of dealing with it seems to be to abandon
shrinking (ie, don't invoke repair_frag) when we find a non-dead tuple whose
XMIN_COMMITTED bit couldn't be set.
Second, it is possible for the same reason that a RECENTLY_DEAD tuple does not
get its XMAX_COMMITTED bit set during scan_heap. But by the time repair_frag
examines the tuple it might be possible to set the bit. We therefore must
take buffer content lock when calling HeapTupleSatisfiesVacuum a second time,
else we can get an Assert failure in SetBufferCommitInfoNeedsSave. This
latter bug is latent in existing releases, but I think it cannot actually
occur without async commit, since the first HeapTupleSatisfiesVacuum call
should always have set the bit. So I'm not going to back-patch it.
In passing, reduce the existing "cannot shrink relation" messages from NOTICE
to LOG level. The new message must be no higher than LOG if we don't want
unpredictable regression test failures, and consistency seems like a good
idea. Also arrange that only one such message is reported per VACUUM FULL;
in typical scenarios you could get spammed with many such messages, which
seems a bit useless.
Modified Files:
--------------
pgsql/src/backend/access/transam:
xlog.c (r1.277 -> r1.278)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xlog.c?r1=1.277&r2=1.278)
pgsql/src/backend/commands:
vacuum.c (r1.354 -> r1.355)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/vacuum.c?r1=1.354&r2=1.355)
On Mon, 2007-08-13 at 19:08 +0000, Tom Lane wrote: > Log Message: > ----------- > Fix two bugs induced in VACUUM FULL by async-commit patch. > > First, we cannot assume that XLogAsyncCommitFlush guarantees hint bits will be > settable, because clog.c's inexact LSN bookkeeping results in windows where a > previously flushed transaction is considered unhintable because it shares an > LSN slot with a later unflushed transaction. But repair_frag requires > XMIN_COMMITTED to be correct so that it can distinguish tuples moved by the > current vacuum. Since not being able to set the bit is an uncommon corner > case, the most practical way of dealing with it seems to be to abandon > shrinking (ie, don't invoke repair_frag) when we find a non-dead tuple whose > XMIN_COMMITTED bit couldn't be set. > > Second, it is possible for the same reason that a RECENTLY_DEAD tuple does not > get its XMAX_COMMITTED bit set during scan_heap. But by the time repair_frag > examines the tuple it might be possible to set the bit. We therefore must > take buffer content lock when calling HeapTupleSatisfiesVacuum a second time, > else we can get an Assert failure in SetBufferCommitInfoNeedsSave. This > latter bug is latent in existing releases, but I think it cannot actually > occur without async commit, since the first HeapTupleSatisfiesVacuum call > should always have set the bit. So I'm not going to back-patch it. > > In passing, reduce the existing "cannot shrink relation" messages from NOTICE > to LOG level. The new message must be no higher than LOG if we don't want > unpredictable regression test failures, and consistency seems like a good > idea. Also arrange that only one such message is reported per VACUUM FULL; > in typical scenarios you could get spammed with many such messages, which > seems a bit useless. This is good; I see what you meant now. Thanks. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com