pg_visibility's pg_check_visible() yields false positive when working in parallel with autovacuum
Hello everyone!
I've been examining a floating bug in pg_visibility. To reproduce it,
you have to:
1) apply a patch to pg_visibility's tests (you can find it attached)
2) run test-icx.sh (attached) from the root of the postgresql's source tree
pg_vis_clean.patch adds one additional test to pg_visibility test suite
and normally it is expected to pass. However, when (I assume so) it
happens to run in parallel with autovacuum, the test is failed.
test-icx.sh runs several parallel installchecks on contrib/pg_visibility
tests and there is a chance that some of them will catch this error.
This is the set-up for a bug reproduction:
*create table vacuum_test as select 42 i;**
**vacuum (disable_page_skipping) vacuum_test;**
*
This is the actual test result:
*diff -U3
/tmp/icx/ic01-1/contrib/pg_visibility/expected/pg_visibility.out
/tmp/icx/ic01-1/contrib/pg_visibility/results/pg_visibility.out**
**--- /tmp/icx/ic01-1/contrib/pg_visibility/expected/pg_visibility.out
2022-02-17 23:46:26.005500630 +0300**
**+++ /tmp/icx/ic01-1/contrib/pg_visibility/results/pg_visibility.out
2022-02-17 23:47:36.066838872 +0300**
**@@ -247,7 +247,7 @@**
** select count(*) > 0 from pg_check_visible('vacuum_test');**
** ?column?**
** ----------**
**- f**
**+ t**
** (1 row)**
**
** -- cleanup*
The thing is that the underlying file is one page long which should
contain the only all-visible tuple and pg_check_visible should return
nothing. Is this a real bug or do I miss something? Thank you in advance
for the response
P. S.: SELECT version(); result: *PostgreSQL 14.2 on
x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.3.0-17ubuntu1~20.04)
9.3.0, 64-bit*
Hi,
On 2022-02-18 00:09:48 +0300, Daniel Shelepanov wrote:
I've been examining a floating bug in pg_visibility. To reproduce it, you
have to:1) apply a patch to pg_visibility's tests (you can find it attached)
2) run test-icx.sh (attached) from the root of the postgresql's source tree
Could you try to minimize the script? A 300 line reproducer is quite long. And
it looks like it won't even work in non postgres-pro tree.
One thing to do would be to modify pg_visibility to elog(PANIC, "something")
when it encounters corruption. Then you would have a chance of inspecting the
state of the tuple/page in that moment.
Greetings,
Andres Freund
Hi,
On 2022-02-18 08:56:37 -0800, Andres Freund wrote:
Could you try to minimize the script? A 300 line reproducer is quite long. And
it looks like it won't even work in non postgres-pro tree.One thing to do would be to modify pg_visibility to elog(PANIC, "something")
when it encounters corruption. Then you would have a chance of inspecting the
state of the tuple/page in that moment.
Oh, I have been able to reliably reproduce this on HEAD. I modified
record_corrupt_item() to PANIC and then:
psql regression:
BEGIN ;SELECT txid_current();
<leave open>
psql postgres
DROP TABLE IF EXISTS vacuum_test_0;
create table vacuum_test_0 as select 42 i;
vacuum (disable_page_skipping) vacuum_test_0;
select * from pg_check_visible('vacuum_test_0');
At which point there immediately is a crash.
This reproduces in earlier versions too, at least back to 10.
I *think* this is a false positive:
- PGPROC->xmin is computed without regard for the database in which the other
sessions are running. Due to the the txid_current() session this includes an
older xid.
- During the VACUUM in vis.sql the only connection to the database pgbench
connects to is VACUUM and thus ignored when determining horizons (due to
PROC_IN_VACUUM). Therefore the horizon is computed to latestCompletedXid +
1.
- But during pg_check_visible(), the current session is *not* marked as
PROC_IN_VACUUM. So the horizon is the xid from the txid_current().
Boom.
Greetings,
Andres Freund