Thread: Wrong check in pg_visibility?
Hi hackers! Due to the error in PG-ProEE we have added the following test to pg_visibility: create table vacuum_test as select 42 i; vacuum vacuum_test; select count(*) > 0 from pg_check_visible('vacuum_test'); drop table vacuum_test; Sometime (very rarely) this test failed: pg_visibility reports "corrupted" tuples. The same error can be reproduced not only with PG-Pro but also with vanilla REL_11_STABLE, REL_12_STABLE and REL_13_STABLE. It is not reproduced with master after Andres snapshot optimization - commit dc7420c2. It is not so easy to reproduce this error: it is necessary to repeat this tests many times. Probability increased with more aggressive autovacuum settings. But even with such settings and thousands of iterations I was not able to reproduce this error at my notebook - only at virtual machine. The error is reported here: /* * If we're checking whether the page is all-visible, we expect * the tuple to be all-visible. */ if (check_visible && !tuple_all_visible(&tuple, OldestXmin, buffer)) { TransactionId RecomputedOldestXmin; /* * Time has passed since we computed OldestXmin, so it's * possible that this tuple is all-visible in reality even * though it doesn't appear so based on our * previously-computed value. Let's compute a new value so we * can be certain whether there is a problem. * * From a concurrency point of view, it sort of sucks to * retake ProcArrayLock here while we're holding the buffer * exclusively locked, but it should be safe against * deadlocks, because surely GetOldestXmin() should never take * a buffer lock. And this shouldn't happen often, so it's * worth being careful so as to avoid false positives. */ RecomputedOldestXmin = GetOldestXmin(NULL, PROCARRAY_FLAGS_VACUUM); if (!TransactionIdPrecedes(OldestXmin, RecomputedOldestXmin)) record_corrupt_item(items, &tuple.t_self); I debugger I have checked that OldestXmin = RecomputedOldestXmin = tuple.t_data->xmin I wonder if this check in pg_visibility is really correct and it can not happen that OldestXmin=tuple.t_data->xmin? Please notice that tuple_all_visible returns false if !TransactionIdPrecedes(xmin, OldestXmin) Thanks in advance, Konstantin
On 06.12.2020 23:50, Konstantin Knizhnik wrote:
Hi hackers!
Due to the error in PG-ProEE we have added the following test to pg_visibility:
create table vacuum_test as select 42 i;
vacuum vacuum_test;
select count(*) > 0 from pg_check_visible('vacuum_test');
drop table vacuum_test;
Sometime (very rarely) this test failed: pg_visibility reports "corrupted" tuples.
The same error can be reproduced not only with PG-Pro but also with vanilla REL_11_STABLE, REL_12_STABLE and REL_13_STABLE.
It is not reproduced with master after Andres snapshot optimization - commit dc7420c2.
It is not so easy to reproduce this error: it is necessary to repeat this tests many times.
Probability increased with more aggressive autovacuum settings.
But even with such settings and thousands of iterations I was not able to reproduce this error at my notebook - only at virtual machine.
The error is reported here:
/*
* If we're checking whether the page is all-visible, we expect
* the tuple to be all-visible.
*/
if (check_visible &&
!tuple_all_visible(&tuple, OldestXmin, buffer))
{
TransactionId RecomputedOldestXmin;
/*
* Time has passed since we computed OldestXmin, so it's
* possible that this tuple is all-visible in reality even
* though it doesn't appear so based on our
* previously-computed value. Let's compute a new value so we
* can be certain whether there is a problem.
*
* From a concurrency point of view, it sort of sucks to
* retake ProcArrayLock here while we're holding the buffer
* exclusively locked, but it should be safe against
* deadlocks, because surely GetOldestXmin() should never take
* a buffer lock. And this shouldn't happen often, so it's
* worth being careful so as to avoid false positives.
*/
RecomputedOldestXmin = GetOldestXmin(NULL, PROCARRAY_FLAGS_VACUUM);
if (!TransactionIdPrecedes(OldestXmin, RecomputedOldestXmin))
record_corrupt_item(items, &tuple.t_self);
I debugger I have checked that OldestXmin = RecomputedOldestXmin = tuple.t_data->xmin
I wonder if this check in pg_visibility is really correct and it can not happen that OldestXmin=tuple.t_data->xmin?
Please notice that tuple_all_visible returns false if !TransactionIdPrecedes(xmin, OldestXmin)
I did more investigations and have to say that this check in pg_visibility.c is really not correct.
The process which is keeping oldest xmin is autovacuum.
It should be ignored by GetOldestXmin because of PROCARRAY_FLAGS_VACUUM flags, but it is not actually skipped
because PROC_IN_VACUUM flag is not set yet. There is yet another flag - PROC_IS_AUTOVACUUM
which is always set in autovacuum, but it can not be passed to GetOldestXmin? because is cleared by PROCARRAY_PROC_FLAGS_MASK.
If we just repeat RecomputedOldestXmin = GetOldestXmin(NULL, PROCARRAY_FLAGS_VACUUM);
several times, then finally we will get right xmin.
I wonder if such check should be excluded from pg_visibility or made in more correct way?
Because nothing in documentation of pg_check_visible says that it may return false positives:
pg_check_visible(relation regclass, t_ctid OUT tid) returns setof tid
Returns the TIDs of non-all-visible tuples stored in pages marked all-visible in the visibility map. If this function returns a non-empty set of TIDs, the visibility map is corrupt.
And comment to this function is even more frightening:
/*
* Return the TIDs of not-all-visible tuples in pages marked all-visible
* in the visibility map. We hope no one will ever find any, but there could
* be bugs, database corruption, etc.
*/-- Konstantin Knizhnik Postgres Professional: http://www.postgrespro.com The Russian Postgres Company