Message-ID: <tencent_B1AFD127DC83DA816AD7596CAE77BCDF0009@qq.com>
From: huyajun <hu_yajun@qq.com>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_DE308090-EA49-4885-89EC-A5DBA506D6AF"
Mime-Version: 1.0 (Mac OS X Mail 15.0 \(3693.20.0.1.32\))
Subject: Re: BUG #18087: pg_visibility: false positives in pg_check_visible
Date: Tue, 5 Sep 2023 21:12:28 +0800
References: <18087-3434b3aa735413ed@postgresql.org>
To: =?utf-8?B?5oiR6Ieq5bex55qE6YKu566x?= <1026592243@qq.com>,
 pgsql-bugs@lists.postgresql.org
In-Reply-To: <18087-3434b3aa735413ed@postgresql.org>
Archived-At: 
 <https://www.postgresql.org/message-id/tencent_B1AFD127DC83DA816AD7596CAE77BCDF0009%40qq.com>
Precedence: bulk


--Apple-Mail=_DE308090-EA49-4885-89EC-A5DBA506D6AF
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8


> 2023=E5=B9=B49=E6=9C=885=E6=97=A5 =E4=B8=8B=E5=8D=889:08=EF=BC=8CPG =
Bug reporting form <noreply@postgresql.org> =E5=86=99=E9=81=93=EF=BC=9A
>=20
> The following bug has been logged on the website:
>=20
> Bug reference:      18087
> Logged by:          yajun Hu
> Email address:      1026592243@qq.com
> PostgreSQL version: 11.21
> Operating system:   CentOS7 with kernel version 5.10
> Description:       =20
>=20
> hi, community partners.=20
>    I found a false positives when use pg_check_visible, I build =
postgresql
> with REL_11_STABLE, run this SQL
> select  pg_create_logical_replication_slot  ('test','pgoutput');
> create table t( a int);
> insert into t select 1;
> insert into t select 1;
> insert into t select 1;
> vacuum t;
> select * from pg_check_visible('t'); -- output ctid, which is false
> positives
>=20
> I analyze this problem, guess in collect_corrupt_items, GetOldestXmin =
use
> rel =3D NULL
> to get alldbs oldestXmin which will influenced by replication slot and =
other
> database
> long transaction. But Vacuum get relation's database oldestXmin.
>=20
> I will put my fix patch behind
>=20
> Regrads, Yajun Hu
>=20
Here are my fix patch, thinks.=

--Apple-Mail=_DE308090-EA49-4885-89EC-A5DBA506D6AF
Content-Disposition: attachment;
	filename=0001-pg_visibility-Fix-wrong-calculation-of-OldestXmin.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="0001-pg_visibility-Fix-wrong-calculation-of-OldestXmin.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20715aa13f123860e1b9cdc353966ff489dfd3594d=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20huyajun=20<hu_yajun@qq.com>=0ADate:=20Tue,=205=20=
Sep=202023=2017:25:33=20+0800=0ASubject:=20[PATCH]=20pg_visibility:=20=
Fix=20wrong=20calculation=20of=20OldestXmin=0A=0AVacuum=20get=20rel's=20=
OldestXmin=20but=20pg_visibility=20not.=0A1.=20other=20db=20procArray=20=
will=20impact=20calculation=0A2.=20replication=20slot=20will=20impact=20=
calculation=0ASo=20OldestXmin=20may=20be=20smaller=20than=20vacuum=20=
calculation=0Awhich=20cound=20lead=20to=20false=20positives=0A---=0A=20=
contrib/pg_visibility/Makefile=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20|=20=204=20++++=0A=20=
contrib/pg_visibility/expected/pg_visibility.out=20|=2012=20++++++++++++=0A=
=20contrib/pg_visibility/pg_visibility.c=20=20=20=20=20=20=20=20=20=20=20=
=20|=2015=20+++++++++++----=0A=20=
contrib/pg_visibility/pg_visibility.conf=20=20=20=20=20=20=20=20=20|=20=20=
1=20+=0A=20contrib/pg_visibility/sql/pg_visibility.sql=20=20=20=20=20=20=
|=20=203=20+++=0A=205=20files=20changed,=2031=20insertions(+),=204=20=
deletions(-)=0A=20create=20mode=20100644=20=
contrib/pg_visibility/pg_visibility.conf=0A=0Adiff=20--git=20=
a/contrib/pg_visibility/Makefile=20b/contrib/pg_visibility/Makefile=0A=
index=2021d787ddf7..366160a40e=20100644=0A---=20=
a/contrib/pg_visibility/Makefile=0A+++=20=
b/contrib/pg_visibility/Makefile=0A@@=20-8,7=20+8,11=20@@=20DATA=20=3D=20=
pg_visibility--1.1.sql=20pg_visibility--1.1--1.2.sql=20\=0A=20=09=
pg_visibility--1.0--1.1.sql=0A=20PGFILEDESC=20=3D=20"pg_visibility=20-=20=
page=20visibility=20information"=0A=20=0A+REGRESS_OPTS=20=3D=20=
--temp-config=20$(top_srcdir)/contrib/pg_visibility/pg_visibility.conf=0A=
=20REGRESS=20=3D=20pg_visibility=0A+#=20Disabled=20because=20these=20=
tests=20require=20"wal_level=3Dlogical",=0A+#=20which=20typical=20=
installcheck=20users=20do=20not=20have.=0A+NO_INSTALLCHECK=20=3D=201=0A=20=
=0A=20ifdef=20USE_PGXS=0A=20PG_CONFIG=20=3D=20pg_config=0Adiff=20--git=20=
a/contrib/pg_visibility/expected/pg_visibility.out=20=
b/contrib/pg_visibility/expected/pg_visibility.out=0Aindex=20=
cc1edeba73..4acec4e004=20100644=0A---=20=
a/contrib/pg_visibility/expected/pg_visibility.out=0A+++=20=
b/contrib/pg_visibility/expected/pg_visibility.out=0A@@=20-66,6=20+66,13=20=
@@=20select=20pg_check_frozen('test_foreign_table');=0A=20ERROR:=20=20=
"test_foreign_table"=20is=20not=20a=20table,=20materialized=20view,=20or=20=
TOAST=20table=0A=20select=20=
pg_truncate_visibility_map('test_foreign_table');=0A=20ERROR:=20=20=
"test_foreign_table"=20is=20not=20a=20table,=20materialized=20view,=20or=20=
TOAST=20table=0A+--=20create=20logical=20slot=20to=20check=20get=20=
correct=20OldestXmin.=0A+SELECT=20'create_logical_slot'=20FROM=20=
pg_create_logical_replication_slot('pg_visibility_test','pgoutput');=0A+=20=
=20=20=20=20=20?column?=20=20=20=20=20=20=20=0A+---------------------=0A=
+=20create_logical_slot=0A+(1=20row)=0A+=0A=20--=20check=20some=20of=20=
the=20allowed=20relkinds=0A=20create=20table=20regular_table=20(a=20=
int);=0A=20insert=20into=20regular_table=20values=20(1),=20(2);=0A@@=20=
-125,6=20+132,11=20@@=20select=20*=20from=20=
pg_check_frozen('test_partition');=20--=20hopefully=20none=0A=20--------=0A=
=20(0=20rows)=0A=20=0A+select=20*=20from=20=
pg_check_visible('test_partition');=20--=20hopefully=20none=0A+=20t_ctid=20=
=0A+--------=0A+(0=20rows)=0A+=0A=20select=20=
pg_truncate_visibility_map('test_partition');=0A=20=20=
pg_truncate_visibility_map=20=0A=20----------------------------=0Adiff=20=
--git=20a/contrib/pg_visibility/pg_visibility.c=20=
b/contrib/pg_visibility/pg_visibility.c=0Aindex=208009bb381c..0add768bcc=20=
100644=0A---=20a/contrib/pg_visibility/pg_visibility.c=0A+++=20=
b/contrib/pg_visibility/pg_visibility.c=0A@@=20-554,13=20+554,17=20@@=20=
collect_corrupt_items(Oid=20relid,=20bool=20all_visible,=20bool=20=
all_frozen)=0A=20=09BufferAccessStrategy=20bstrategy=20=3D=20=
GetAccessStrategy(BAS_BULKREAD);=0A=20=09TransactionId=20OldestXmin=20=3D=20=
InvalidTransactionId;=0A=20=0A+=09rel=20=3D=20relation_open(relid,=20=
AccessShareLock);=0A=20=09if=20(all_visible)=0A=20=09{=0A-=09=09/*=20=
Don't=20pass=20rel;=20that=20will=20fail=20in=20recovery.=20*/=0A-=09=09=
OldestXmin=20=3D=20GetOldestXmin(NULL,=20PROCARRAY_FLAGS_VACUUM);=0A+=09=09=
/*=20Don't=20pass=20rel=20when=20in=20recovery,=20that=20will=20fail.=20=
*/=0A+=09=09if=20(RecoveryInProgress())=0A+=09=09=09OldestXmin=20=3D=20=
GetOldestXmin(NULL,=20PROCARRAY_FLAGS_VACUUM);=0A+=09=09/*=20Pass=20rel=20=
when=20in=20primary=20to=20avoid=20returning=20spurious=20results.=20*/=20=
=0A+=09=09else=0A+=09=09=09OldestXmin=20=3D=20GetOldestXmin(rel,=20=
PROCARRAY_FLAGS_VACUUM);=0A=20=09}=0A=20=0A-=09rel=20=3D=20=
relation_open(relid,=20AccessShareLock);=0A=20=0A=20=09/*=20Only=20some=20=
relkinds=20have=20a=20visibility=20map=20*/=0A=20=09=
check_relation_relkind(rel);=0A@@=20-674,7=20+678,10=20@@=20=
collect_corrupt_items(Oid=20relid,=20bool=20all_visible,=20bool=20=
all_frozen)=0A=20=09=09=09=09=20*=20a=20buffer=20lock.=20And=20this=20=
shouldn't=20happen=20often,=20so=20it's=0A=20=09=09=09=09=20*=20worth=20=
being=20careful=20so=20as=20to=20avoid=20false=20positives.=0A=20=09=09=09=
=09=20*/=0A-=09=09=09=09RecomputedOldestXmin=20=3D=20GetOldestXmin(NULL,=20=
PROCARRAY_FLAGS_VACUUM);=0A+=09=09=09=09if=20(RecoveryInProgress())=0A+=09=
=09=09=09=09RecomputedOldestXmin=20=3D=20GetOldestXmin(NULL,=20=
PROCARRAY_FLAGS_VACUUM);=0A+=09=09=09=09else=0A+=09=09=09=09=09=
RecomputedOldestXmin=20=3D=20GetOldestXmin(rel,=20=
PROCARRAY_FLAGS_VACUUM);=0A=20=0A=20=09=09=09=09if=20=
(!TransactionIdPrecedes(OldestXmin,=20RecomputedOldestXmin))=0A=20=09=09=09=
=09=09record_corrupt_item(items,=20&tuple.t_self);=0Adiff=20--git=20=
a/contrib/pg_visibility/pg_visibility.conf=20=
b/contrib/pg_visibility/pg_visibility.conf=0Anew=20file=20mode=20100644=0A=
index=200000000000..e3d257315f=0A---=20/dev/null=0A+++=20=
b/contrib/pg_visibility/pg_visibility.conf=0A@@=20-0,0=20+1=20@@=0A=
+wal_level=20=3D=20logical=0Adiff=20--git=20=
a/contrib/pg_visibility/sql/pg_visibility.sql=20=
b/contrib/pg_visibility/sql/pg_visibility.sql=0Aindex=20=
6f4c9a42f9..a32c438f9d=20100644=0A---=20=
a/contrib/pg_visibility/sql/pg_visibility.sql=0A+++=20=
b/contrib/pg_visibility/sql/pg_visibility.sql=0A@@=20-48,6=20+48,8=20@@=20=
select=20pg_visibility_map_summary('test_foreign_table');=0A=20select=20=
pg_check_frozen('test_foreign_table');=0A=20select=20=
pg_truncate_visibility_map('test_foreign_table');=0A=20=0A+--=20create=20=
logical=20slot=20to=20check=20get=20correct=20OldestXmin.=0A+SELECT=20=
'create_logical_slot'=20FROM=20=
pg_create_logical_replication_slot('pg_visibility_test','pgoutput');=0A=20=
--=20check=20some=20of=20the=20allowed=20relkinds=0A=20create=20table=20=
regular_table=20(a=20int);=0A=20insert=20into=20regular_table=20values=20=
(1),=20(2);=0A@@=20-70,6=20+72,7=20@@=20select=20count(*)=20>=200=20from=20=
pg_visibility('test_partition',=200);=0A=20select=20count(*)=20>=200=20=
from=20pg_visibility_map('test_partition');=0A=20select=20count(*)=20>=20=
0=20from=20pg_visibility_map_summary('test_partition');=0A=20select=20*=20=
from=20pg_check_frozen('test_partition');=20--=20hopefully=20none=0A=
+select=20*=20from=20pg_check_visible('test_partition');=20--=20=
hopefully=20none=0A=20select=20=
pg_truncate_visibility_map('test_partition');=0A=20=0A=20--=20cleanup=0A=
--=20=0A2.41.0=0A=0A=

--Apple-Mail=_DE308090-EA49-4885-89EC-A5DBA506D6AF--