pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17257: (auto)vacuum hangs within lazy_scan_prune()
43+ messages / 9 participants
[nested] [flat]
Commits from this thread:
2024-07-19 83c39a1f7f fd4f12df5e Ensure vacuum removes all visibly dead tuples older than OldestXmin
* BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-10-29 13:00 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 43+ messages in thread
From: PG Bug reporting form @ 2021-10-29 13:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
The following bug has been logged on the website:
Bug reference: 17257
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 14.0
Operating system: Ubuntu 20.04
Description:
When running concurrent installchecks (x100) I've observed the autovacuum
process hanging:
tester 328713 328706 0 12:19 ? 00:02:03 postgres: logical
replication launcher
tester 3301788 328706 0 16:33 ? 00:00:02 postgres: postgres
regress030 127.0.0.1(44534) CREATE TABLE
tester 3302076 328706 99 16:33 ? 05:17:10 postgres: autovacuum
worker regress030
tester 3311424 328706 0 16:34 ? 00:03:09 postgres: postgres
regress090 127.0.0.1(48744) ALTER TABLE
tester 3311453 328706 1 16:34 ? 00:03:12 postgres: postgres
regress048 127.0.0.1(48768) ALTER TABLE
tester 3311461 328706 1 16:34 ? 00:03:11 postgres: postgres
regress017 127.0.0.1(48774) DROP TABLE
tester 3311490 328706 0 16:34 ? 00:03:09 postgres: postgres
regress006 127.0.0.1(48804) ALTER TABLE
tester 3311493 328706 0 16:34 ? 00:03:08 postgres: postgres
regress008 127.0.0.1(48806) ALTER TABLE
gdb showed the following stacktrace:
0x0000564d6c93cd1f in GetPrivateRefCountEntry (buffer=<optimized out>,
do_move=<optimized out>) at bufmgr.c:312
312 Assert(BufferIsValid(buffer));
(gdb) bt
#0 0x0000564d6c93cd1f in GetPrivateRefCountEntry (buffer=<optimized out>,
do_move=<optimized out>) at bufmgr.c:312
#1 0x0000564d6c93fb79 in GetPrivateRefCount (buffer=111422) at
bufmgr.c:398
#2 BufferGetBlockNumber (buffer=buffer@entry=111422) at bufmgr.c:2771
#3 0x0000564d6b83519d in heap_prune_chain (prstate=0x7fff07f03e10,
rootoffnum=7, buffer=111422) at pruneheap.c:625
#4 heap_page_prune (relation=relation@entry=0x7fa60a9cff80,
buffer=buffer@entry=111422,
vistest=vistest@entry=0x564d710816a0 <GlobalVisCatalogRels>,
old_snap_xmin=old_snap_xmin@entry=0,
old_snap_ts=old_snap_ts@entry=0, report_stats=report_stats@entry=false,
off_loc=<optimized out>) at pruneheap.c:278
#5 0x0000564d6b84a7d4 in lazy_scan_prune (vacrel=0x625000044190,
buf=<optimized out>, blkno=<optimized out>,
page=0x7fa642bdd580 <incomplete sequence \334>, vistest=<optimized out>,
prunestate=<optimized out>)
at vacuumlazy.c:1741
#6 0x0000564d6b8597cd in lazy_scan_heap (aggressive=<optimized out>,
params=<optimized out>, vacrel=<optimized out>)
at vacuumlazy.c:1384
#7 heap_vacuum_rel (rel=<optimized out>, params=<optimized out>,
bstrategy=<optimized out>) at vacuumlazy.c:638
#8 0x0000564d6bf9b6b6 in table_relation_vacuum (bstrategy=<optimized out>,
params=0x6250000272c4, rel=0x7fa60a9cff80)
at ../../../src/include/access/tableam.h:1678
#9 vacuum_rel (relid=<optimized out>, relation=<optimized out>,
params=0x6250000272c4) at vacuum.c:2034
#10 0x0000564d6bfa11f0 in vacuum (relations=0x625000041278,
relations@entry=0x6250000552b0,
params=params@entry=0x6250000272c4, bstrategy=<optimized out>,
bstrategy@entry=0x625000053048,
isTopLevel=isTopLevel@entry=true) at vacuum.c:475
#11 0x0000564d6c6e3a37 in autovacuum_do_vac_analyze
(bstrategy=0x625000053048, tab=0x6250000272c0) at autovacuum.c:3242
#12 do_autovacuum () at autovacuum.c:2490
#13 0x0000564d6c6e6681 in AutoVacWorkerMain (argv=0x0, argc=0) at
autovacuum.c:1714
#14 0x0000564d6c6e6a95 in StartAutoVacWorker () at autovacuum.c:1499
#15 0x0000564d6c7272d4 in StartAutovacuumWorker () at postmaster.c:5561
#16 sigusr1_handler (postgres_signal_arg=<optimized out>) at
postmaster.c:5266
#17 <signal handler called>
#18 __GI___sigprocmask (how=2, set=<optimized out>, oset=0x0) at
../sysdeps/unix/sysv/linux/sigprocmask.c:39
#19 0x0000564d6b52efc4 in __interceptor_sigprocmask.part.0 ()
#20 0x0000564d6c727d9e in ServerLoop () at postmaster.c:1707
#21 0x0000564d6c72c309 in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0x603000000250) at postmaster.c:1417
#22 0x0000564d6b4f6995 in main (argc=3, argv=0x603000000250) at main.c:209
(gdb) b 1820
Breakpoint 1 at 0x564d6b84b378: file vacuumlazy.c, line 1820.
(gdb) cont
Continuing.
Breakpoint 1, lazy_scan_prune (vacrel=0x625000044190, buf=<optimized out>,
blkno=<optimized out>,
page=0x7fa642bdd580 <incomplete sequence \334>, vistest=<optimized out>,
prunestate=<optimized out>)
at vacuumlazy.c:1820
1820 goto retry;
(gdb) print *vacrel
$15 = {rel = 0x7fa60a9cff80, indrels = 0x625000043b70, nindexes = 5,
failsafe_active = false,
consider_bypass_optimization = true, do_index_vacuuming = true,
do_index_cleanup = true, do_rel_truncate = true,
bstrategy = 0x625000053048, lps = 0x0, old_rel_pages = 6, old_live_tuples
= 133, relfrozenxid = 726, relminmxid = 1,
OldestXmin = 48155004, FreezeLimit = 4293122300, MultiXactCutoff =
4289973311,
relnamespace = 0x625000044140 "pg_catalog", relname = 0x625000044168
"pg_constraint", indname = 0x0, blkno = 11,
offnum = 32, phase = VACUUM_ERRCB_PHASE_SCAN_HEAP, dead_tuples =
0x62a0000a8240, rel_pages = 13, scanned_pages = 12,
pinskipped_pages = 0, frozenskipped_pages = 0, tupcount_pages = 12,
pages_removed = 0, lpdead_item_pages = 9,
nonempty_pages = 11, new_rel_tuples = 0, new_live_tuples = 0, indstats =
0x6250000444d8, num_index_scans = 0,
tuples_deleted = 9, lpdead_items = 226, new_dead_tuples = 0, num_tuples =
237, live_tuples = 237}
I've also caught the similar hang on (auto)vacuuming pg_attrdef,
pg_shdepend, pg_class.
It seems that this eternal retry is caused by the HEAPTUPLE_RECENTLY_DEAD
state of a tuple processed in the lazy_scan_prune loop.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-10-29 13:30 Alexander Lakhin <exclusion@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 3 replies; 43+ messages in thread
From: Alexander Lakhin @ 2021-10-29 13:30 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
29.10.2021 16:00, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17257
> Logged by: Alexander Lakhin
> Email address: exclusion@gmail.com
> PostgreSQL version: 14.0
> Operating system: Ubuntu 20.04
> Description:
>
> When running concurrent installchecks (x100) I've observed the autovacuum
> process hanging:
>
I can propose the debugging patch to reproduce the issue that replaces
the hang with the assert and modifies a pair of crash-causing test
scripts to simplify the reproducing. (Sorry, I have no time now to prune
down the scripts further as I have to leave for a week.)
The reproducing script is:
###
export PGDATABASE=regression
createdb regression
echo "
vacuum (verbose, skip_locked, index_cleanup off) pg_catalog.pg_class;
select pg_sleep(random()/50);
" >/tmp/pseudo-autovacuum.sql
( timeout 10m bash -c "while true; do psql -f
src/test/regress/sql/inherit.sql >>psql1.log 2>&1; coredumpctl
--no-pager >/dev/null 2>&1 && break; done" ) &
( timeout 10m bash -c "while true; do psql -f
src/test/regress/sql/vacuum.sql >>psql2.log 2>&1; coredumpctl --no-pager
>/dev/null 2>&1 && break; done" ) &
pgbench -n -f /tmp/pseudo-autovacuum.sql -C -c 40 -T 600 >pgbench.log 2>&1 &
wait
###
(I've set:
autovacuum=off
fsync=off
in postgresql.conf)
It leads to:
TIME PID UID GID SIG COREFILE EXE
Fri 2021-10-29 16:15:59 MSK 2337121 1000 1000 6 present
.../usr/local/pgsql/bin/postgres
real 2m19,425s
user 4m26,078s
sys 0m31,658s
Core was generated by `postgres: law regression [local]
VACUUM '.
Program terminated with signal SIGABRT, Aborted.
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007f3d17e88859 in __GI_abort () at abort.c:79
#2 0x0000557358fbecb8 in ExceptionalCondition
(conditionName=conditionName@entry=0x557359027544 "numretries < 100",
errorType=errorType@entry=0x55735901800b "FailedAssertion",
fileName=0x7ffef5bbfa70 "\231\354\373XsU",
fileName@entry=0x557359027520 "vacuumlazy.c",
lineNumber=lineNumber@entry=1726) at assert.c:69
#3 0x0000557358ba0bdb in lazy_scan_prune (vacrel=0x55735a508d60,
buf=16184, blkno=270, page=0x7f3d16ef6f80 "",
vistest=0x5573592dd540 <GlobalVisCatalogRels>,
prunestate=0x7ffef5bc0fb0) at vacuumlazy.c:1823
#4 0x0000557358ba38c5 in lazy_scan_heap (aggressive=false,
params=0x7ffef5bc13a0, vacrel=<optimized out>)
at vacuumlazy.c:1384
#5 heap_vacuum_rel (rel=0x7f3d183ebec8, params=0x7ffef5bc13a0,
bstrategy=<optimized out>) at vacuumlazy.c:638
#6 0x0000557358cec785 in table_relation_vacuum (bstrategy=<optimized
out>, params=0x7ffef5bc13a0, rel=0x7f3d183ebec8)
at ../../../src/include/access/tableam.h:1678
#7 vacuum_rel (relid=1259, relation=<optimized out>,
params=params@entry=0x7ffef5bc13a0) at vacuum.c:2034
#8 0x0000557358cedf9e in vacuum (relations=0x55735a57bea8,
params=0x7ffef5bc13a0, bstrategy=<optimized out>,
isTopLevel=<optimized out>) at vacuum.c:475
#9 0x0000557358cee4f5 in ExecVacuum
(pstate=pstate@entry=0x55735a4ab730, vacstmt=vacstmt@entry=0x55735a48a408,
isTopLevel=isTopLevel@entry=true) at vacuum.c:268
#10 0x0000557358e9de4e in standard_ProcessUtility (pstmt=0x55735a48a758,
queryString=0x55735a4895e0 "vacuum (verbose, skip_locked,
index_cleanup off) pg_catalog.pg_class;",
readOnlyTree=<optimized out>, context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x55735a48a848,
qc=0x7ffef5bc1700) at utility.c:858
#11 0x0000557358e9c411 in PortalRunUtility
(portal=portal@entry=0x55735a4ed040, pstmt=pstmt@entry=0x55735a48a758,
isTopLevel=isTopLevel@entry=true,
setHoldSnapshot=setHoldSnapshot@entry=false,
dest=dest@entry=0x55735a48a848,
qc=qc@entry=0x7ffef5bc1700) at pquery.c:1155
#12 0x0000557358e9c54d in PortalRunMulti
(portal=portal@entry=0x55735a4ed040, isTopLevel=isTopLevel@entry=true,
setHoldSnapshot=setHoldSnapshot@entry=false,
dest=dest@entry=0x55735a48a848, altdest=altdest@entry=0x55735a48a848,
qc=qc@entry=0x7ffef5bc1700) at pquery.c:1312
#13 0x0000557358e9cbe9 in PortalRun (portal=portal@entry=0x55735a4ed040,
count=count@entry=9223372036854775807,
isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true,
dest=dest@entry=0x55735a48a848,
altdest=altdest@entry=0x55735a48a848, qc=0x7ffef5bc1700) at pquery.c:788
#14 0x0000557358e9895b in exec_simple_query (
query_string=0x55735a4895e0 "vacuum (verbose, skip_locked,
index_cleanup off) pg_catalog.pg_class;")
at postgres.c:1214
#15 0x0000557358e9a561 in PostgresMain (argc=argc@entry=1,
argv=argv@entry=0x7ffef5bc1b70, dbname=<optimized out>,
username=<optimized out>) at postgres.c:4486
#16 0x0000557358e064dd in BackendRun (port=0x55735a4aeb00,
port=0x55735a4aeb00) at postmaster.c:4506
#17 BackendStartup (port=0x55735a4aeb00) at postmaster.c:4228
#18 ServerLoop () at postmaster.c:1745
#19 0x0000557358e07481 in PostmasterMain (argc=<optimized out>,
argv=<optimized out>) at postmaster.c:1417
#20 0x0000557358b352e2 in main (argc=3, argv=0x55735a4838a0) at main.c:209
Sometimes the server crashes other way as described in bug #17255.
And I've also seen the following crash (maybe it's yet another bug, but
I can't explore it now):
Core was generated by `postgres: law regression [local]
EXPLAIN '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 RelationBuildPartitionDesc (omit_detached=true, rel=0x7f09b87a27e8)
at partdesc.c:230
230 datum = heap_getattr(tuple,
Anum_pg_class_relpartbound,
(gdb) bt
#0 RelationBuildPartitionDesc (omit_detached=true, rel=0x7f09b87a27e8)
at partdesc.c:230
#1 RelationGetPartitionDesc (rel=0x7f09b87a27e8,
omit_detached=<optimized out>) at partdesc.c:110
#2 0x000055ec12169032 in PartitionDirectoryLookup (pdir=0x55ec12ee7660,
rel=rel@entry=0x7f09b87a27e8)
at partdesc.c:425
#3 0x000055ec12158b35 in set_relation_partition_info
(relation=0x7f09b87a27e8, rel=0x55ec12f337c0,
root=0x55ec12effae0) at plancat.c:2197
#4 get_relation_info (root=<optimized out>, relationObjectId=<optimized
out>, inhparent=<optimized out>,
rel=<optimized out>) at plancat.c:472
#5 0x000055ec1215d26f in build_simple_rel
(root=root@entry=0x55ec12effae0, relid=5,
parent=parent@entry=0x55ec12ee7330) at relnode.c:307
#6 0x000055ec1214f495 in expand_partitioned_rtentry
(root=0x55ec12effae0, relinfo=0x55ec12ee7330,
parentrte=0x55ec12e006d0, parentRTindex=1, parentrel=0x7f09b87a0630,
top_parentrc=0x0, lockmode=1) at inherit.c:398
#7 0x000055ec1214f67e in expand_inherited_rtentry
(root=root@entry=0x55ec12effae0, rel=0x55ec12ee7330,
rte=0x55ec12e006d0, rti=rti@entry=1) at inherit.c:143
#8 0x000055ec1212eb8d in add_other_rels_to_query
(root=root@entry=0x55ec12effae0) at initsplan.c:163
#9 0x000055ec12131e3f in query_planner (root=root@entry=0x55ec12effae0,
qp_callback=qp_callback@entry=0x55ec12132b20 <standard_qp_callback>,
qp_extra=qp_extra@entry=0x7ffcffe4e890)
at planmain.c:264
#10 0x000055ec121371b7 in grouping_planner (root=<optimized out>,
tuple_fraction=<optimized out>) at planner.c:1442
#11 0x000055ec12139d0e in subquery_planner
(glob=glob@entry=0x55ec12ef3eb8, parse=parse@entry=0x55ec12e005b8,
parent_root=parent_root@entry=0x0,
hasRecursion=hasRecursion@entry=false,
tuple_fraction=tuple_fraction@entry=0)
at planner.c:1019
#12 0x000055ec1213a383 in standard_planner (parse=0x55ec12e005b8,
query_string=<optimized out>, cursorOptions=2048,
boundParams=<optimized out>) at planner.c:400
#13 0x000055ec1220d7f8 in pg_plan_query (querytree=0x55ec12e005b8,
querytree@entry=0x7ffcffe4ead0,
query_string=query_string@entry=0x55ec12dff520 "explain (costs off)
select * from range_list_parted;",
cursorOptions=cursorOptions@entry=2048,
boundParams=boundParams@entry=0x0) at postgres.c:847
#14 0x000055ec12025fef in ExplainOneQuery (query=0x7ffcffe4ead0,
cursorOptions=2048, into=0x0, es=0x55ec12ef3bd0,
queryString=0x55ec12dff520 "explain (costs off) select * from
range_list_parted;", params=0x0, queryEnv=0x0)
at explain.c:397
#15 0x000055ec1202679f in ExplainQuery (pstate=0x55ec12ef3a70,
stmt=0x55ec12e003d8, params=0x0, dest=0x55ec12ef39d8)
at ../../../src/include/nodes/nodes.h:604
#16 0x000055ec122132c9 in standard_ProcessUtility (pstmt=0x55ec12e01058,
queryString=0x55ec12dff520 "explain (costs off) select * from
range_list_parted;", readOnlyTree=<optimized out>,
context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
dest=0x55ec12ef39d8, qc=0x7ffcffe4ed60)
at utility.c:862
#17 0x000055ec122116cf in PortalRunUtility
(portal=portal@entry=0x55ec12e63060, pstmt=0x55ec12e01058,
isTopLevel=<optimized out>,
setHoldSnapshot=setHoldSnapshot@entry=true, dest=dest@entry=0x55ec12ef39d8,
qc=qc@entry=0x7ffcffe4ed60) at pquery.c:1155
#18 0x000055ec12211bb0 in FillPortalStore (portal=0x55ec12e63060,
isTopLevel=<optimized out>)
at ../../../src/include/nodes/nodes.h:604
#19 0x000055ec12211ebd in PortalRun (portal=portal@entry=0x55ec12e63060,
count=count@entry=9223372036854775807,
isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true,
dest=dest@entry=0x55ec12e01148,
altdest=altdest@entry=0x55ec12e01148, qc=0x7ffcffe4ef60) at pquery.c:760
#20 0x000055ec1220dcf5 in exec_simple_query (
query_string=0x55ec12dff520 "explain (costs off) select * from
range_list_parted;") at postgres.c:1214
#21 0x000055ec1220f886 in PostgresMain (argc=argc@entry=1,
argv=argv@entry=0x7ffcffe4f4b0, dbname=<optimized out>,
username=<optimized out>) at postgres.c:4486
#22 0x000055ec1218071a in BackendRun (port=<optimized out>,
port=<optimized out>) at postmaster.c:4506
#23 BackendStartup (port=<optimized out>) at postmaster.c:4228
#24 ServerLoop () at postmaster.c:1745
#25 0x000055ec12181682 in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0x55ec12df9ce0) at postmaster.c:1417
#26 0x000055ec11eca3a0 in main (argc=3, argv=0x55ec12df9ce0) at main.c:209
Best regards,
Alexander
Attachments:
vacuum-hang-demo.patch (text/x-patch, 28.5K) download
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d2c735eedae..79cec947f4d 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1717,10 +1717,13 @@ lazy_scan_prune(LVRelState *vacrel,
int nfrozen;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
xl_heap_freeze_tuple frozen[MaxHeapTuplesPerPage];
+int numretries = 0;
maxoff = PageGetMaxOffsetNumber(page);
retry:
+numretries++;
+Assert(numretries < 100);
/* Initialize (or reset) page-level counters */
tuples_deleted = 0;
diff --git a/src/test/regress/sql/inherit.sql b/src/test/regress/sql/inherit.sql
index 64173a8738c..165427d9d7e 100644
--- a/src/test/regress/sql/inherit.sql
+++ b/src/test/regress/sql/inherit.sql
@@ -1,497 +1,5 @@
---
--- Test inheritance features
---
-CREATE TABLE a (aa TEXT);
-CREATE TABLE b (bb TEXT) INHERITS (a);
-CREATE TABLE c (cc TEXT) INHERITS (a);
-CREATE TABLE d (dd TEXT) INHERITS (b,c,a);
-
-INSERT INTO a(aa) VALUES('aaa');
-INSERT INTO a(aa) VALUES('aaaa');
-INSERT INTO a(aa) VALUES('aaaaa');
-INSERT INTO a(aa) VALUES('aaaaaa');
-INSERT INTO a(aa) VALUES('aaaaaaa');
-INSERT INTO a(aa) VALUES('aaaaaaaa');
-
-INSERT INTO b(aa) VALUES('bbb');
-INSERT INTO b(aa) VALUES('bbbb');
-INSERT INTO b(aa) VALUES('bbbbb');
-INSERT INTO b(aa) VALUES('bbbbbb');
-INSERT INTO b(aa) VALUES('bbbbbbb');
-INSERT INTO b(aa) VALUES('bbbbbbbb');
-
-INSERT INTO c(aa) VALUES('ccc');
-INSERT INTO c(aa) VALUES('cccc');
-INSERT INTO c(aa) VALUES('ccccc');
-INSERT INTO c(aa) VALUES('cccccc');
-INSERT INTO c(aa) VALUES('ccccccc');
-INSERT INTO c(aa) VALUES('cccccccc');
-
-INSERT INTO d(aa) VALUES('ddd');
-INSERT INTO d(aa) VALUES('dddd');
-INSERT INTO d(aa) VALUES('ddddd');
-INSERT INTO d(aa) VALUES('dddddd');
-INSERT INTO d(aa) VALUES('ddddddd');
-INSERT INTO d(aa) VALUES('dddddddd');
-
-SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM d, pg_class where d.tableoid = pg_class.oid;
-SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
-
-UPDATE a SET aa='zzzz' WHERE aa='aaaa';
-UPDATE ONLY a SET aa='zzzzz' WHERE aa='aaaaa';
-UPDATE b SET aa='zzz' WHERE aa='aaa';
-UPDATE ONLY b SET aa='zzz' WHERE aa='aaa';
-UPDATE a SET aa='zzzzzz' WHERE aa LIKE 'aaa%';
-
-SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM d, pg_class where d.tableoid = pg_class.oid;
-SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
-
-UPDATE b SET aa='new';
-
-SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM d, pg_class where d.tableoid = pg_class.oid;
-SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
-
-UPDATE a SET aa='new';
-
-DELETE FROM ONLY c WHERE aa='new';
-
-SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM d, pg_class where d.tableoid = pg_class.oid;
-SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
-
-DELETE FROM a;
-
-SELECT relname, a.* FROM a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM d, pg_class where d.tableoid = pg_class.oid;
-SELECT relname, a.* FROM ONLY a, pg_class where a.tableoid = pg_class.oid;
-SELECT relname, b.* FROM ONLY b, pg_class where b.tableoid = pg_class.oid;
-SELECT relname, c.* FROM ONLY c, pg_class where c.tableoid = pg_class.oid;
-SELECT relname, d.* FROM ONLY d, pg_class where d.tableoid = pg_class.oid;
-
--- Confirm PRIMARY KEY adds NOT NULL constraint to child table
-CREATE TEMP TABLE z (b TEXT, PRIMARY KEY(aa, b)) inherits (a);
-INSERT INTO z VALUES (NULL, 'text'); -- should fail
-
--- Check inherited UPDATE with all children excluded
-create table some_tab (a int, b int);
-create table some_tab_child () inherits (some_tab);
-insert into some_tab_child values(1,2);
-
-explain (verbose, costs off)
-update some_tab set a = a + 1 where false;
-update some_tab set a = a + 1 where false;
-explain (verbose, costs off)
-update some_tab set a = a + 1 where false returning b, a;
-update some_tab set a = a + 1 where false returning b, a;
-table some_tab;
-
-drop table some_tab cascade;
-
--- Check UPDATE with inherited target and an inherited source table
-create temp table foo(f1 int, f2 int);
-create temp table foo2(f3 int) inherits (foo);
-create temp table bar(f1 int, f2 int);
-create temp table bar2(f3 int) inherits (bar);
-
-insert into foo values(1,1);
-insert into foo values(3,3);
-insert into foo2 values(2,2,2);
-insert into foo2 values(3,3,3);
-insert into bar values(1,1);
-insert into bar values(2,2);
-insert into bar values(3,3);
-insert into bar values(4,4);
-insert into bar2 values(1,1,1);
-insert into bar2 values(2,2,2);
-insert into bar2 values(3,3,3);
-insert into bar2 values(4,4,4);
-
-update bar set f2 = f2 + 100 where f1 in (select f1 from foo);
-
-select tableoid::regclass::text as relname, bar.* from bar order by 1,2;
-
--- Check UPDATE with inherited target and an appendrel subquery
-update bar set f2 = f2 + 100
-from
- ( select f1 from foo union all select f1+3 from foo ) ss
-where bar.f1 = ss.f1;
-
-select tableoid::regclass::text as relname, bar.* from bar order by 1,2;
-
--- Check UPDATE with *partitioned* inherited target and an appendrel subquery
-create table some_tab (a int);
-insert into some_tab values (0);
-create table some_tab_child () inherits (some_tab);
-insert into some_tab_child values (1);
-create table parted_tab (a int, b char) partition by list (a);
-create table parted_tab_part1 partition of parted_tab for values in (1);
-create table parted_tab_part2 partition of parted_tab for values in (2);
-create table parted_tab_part3 partition of parted_tab for values in (3);
-insert into parted_tab values (1, 'a'), (2, 'a'), (3, 'a');
-
-update parted_tab set b = 'b'
-from
- (select a from some_tab union all select a+1 from some_tab) ss (a)
-where parted_tab.a = ss.a;
-select tableoid::regclass::text as relname, parted_tab.* from parted_tab order by 1,2;
-
-truncate parted_tab;
-insert into parted_tab values (1, 'a'), (2, 'a'), (3, 'a');
-update parted_tab set b = 'b'
-from
- (select 0 from parted_tab union all select 1 from parted_tab) ss (a)
-where parted_tab.a = ss.a;
-select tableoid::regclass::text as relname, parted_tab.* from parted_tab order by 1,2;
-
--- modifies partition key, but no rows will actually be updated
-explain update parted_tab set a = 2 where false;
-
-drop table parted_tab;
-
--- Check UPDATE with multi-level partitioned inherited target
-create table mlparted_tab (a int, b char, c text) partition by list (a);
-create table mlparted_tab_part1 partition of mlparted_tab for values in (1);
-create table mlparted_tab_part2 partition of mlparted_tab for values in (2) partition by list (b);
-create table mlparted_tab_part3 partition of mlparted_tab for values in (3);
-create table mlparted_tab_part2a partition of mlparted_tab_part2 for values in ('a');
-create table mlparted_tab_part2b partition of mlparted_tab_part2 for values in ('b');
-insert into mlparted_tab values (1, 'a'), (2, 'a'), (2, 'b'), (3, 'a');
-
-update mlparted_tab mlp set c = 'xxx'
-from
- (select a from some_tab union all select a+1 from some_tab) ss (a)
-where (mlp.a = ss.a and mlp.b = 'b') or mlp.a = 3;
-select tableoid::regclass::text as relname, mlparted_tab.* from mlparted_tab order by 1,2;
-
-drop table mlparted_tab;
-drop table some_tab cascade;
-
-/* Test multiple inheritance of column defaults */
-
-CREATE TABLE firstparent (tomorrow date default now()::date + 1);
-CREATE TABLE secondparent (tomorrow date default now() :: date + 1);
-CREATE TABLE jointchild () INHERITS (firstparent, secondparent); -- ok
-CREATE TABLE thirdparent (tomorrow date default now()::date - 1);
-CREATE TABLE otherchild () INHERITS (firstparent, thirdparent); -- not ok
-CREATE TABLE otherchild (tomorrow date default now())
- INHERITS (firstparent, thirdparent); -- ok, child resolves ambiguous default
-
-DROP TABLE firstparent, secondparent, jointchild, thirdparent, otherchild;
-
--- Test changing the type of inherited columns
-insert into d values('test','one','two','three');
-alter table a alter column aa type integer using bit_length(aa);
-select * from d;
-
--- The above verified that we can change the type of a multiply-inherited
--- column; but we should reject that if any definition was inherited from
--- an unrelated parent.
-create temp table parent1(f1 int, f2 int);
-create temp table parent2(f1 int, f3 bigint);
-create temp table childtab(f4 int) inherits(parent1, parent2);
-alter table parent1 alter column f1 type bigint; -- fail, conflict w/parent2
-alter table parent1 alter column f2 type bigint; -- ok
-
--- Test non-inheritable parent constraints
-create table p1(ff1 int);
-alter table p1 add constraint p1chk check (ff1 > 0) no inherit;
-alter table p1 add constraint p2chk check (ff1 > 10);
--- connoinherit should be true for NO INHERIT constraint
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pgc.connoinherit from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname = 'p1' order by 1,2;
-
--- Test that child does not inherit NO INHERIT constraints
-create table c1 () inherits (p1);
-\d p1
-\d c1
-
--- Test that child does not override inheritable constraints of the parent
-create table c2 (constraint p2chk check (ff1 > 10) no inherit) inherits (p1); --fails
-
-drop table p1 cascade;
-
--- Tests for casting between the rowtypes of parent and child
--- tables. See the pgsql-hackers thread beginning Dec. 4/04
-create table base (i integer);
-create table derived () inherits (base);
-create table more_derived (like derived, b int) inherits (derived);
-insert into derived (i) values (0);
-select derived::base from derived;
-select NULL::derived::base;
--- remove redundant conversions.
-explain (verbose on, costs off) select row(i, b)::more_derived::derived::base from more_derived;
-explain (verbose on, costs off) select (1, 2)::more_derived::derived::base;
-drop table more_derived;
-drop table derived;
-drop table base;
-
-create table p1(ff1 int);
-create table p2(f1 text);
-create function p2text(p2) returns text as 'select $1.f1' language sql;
-create table c1(f3 int) inherits(p1,p2);
-insert into c1 values(123456789, 'hi', 42);
-select p2text(c1.*) from c1;
-drop function p2text(p2);
-drop table c1;
-drop table p2;
-drop table p1;
-
-CREATE TABLE ac (aa TEXT);
-alter table ac add constraint ac_check check (aa is not null);
-CREATE TABLE bc (bb TEXT) INHERITS (ac);
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-
-insert into ac (aa) values (NULL);
-insert into bc (aa) values (NULL);
-
-alter table bc drop constraint ac_check; -- fail, disallowed
-alter table ac drop constraint ac_check;
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-
--- try the unnamed-constraint case
-alter table ac add check (aa is not null);
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-
-insert into ac (aa) values (NULL);
-insert into bc (aa) values (NULL);
-
-alter table bc drop constraint ac_aa_check; -- fail, disallowed
-alter table ac drop constraint ac_aa_check;
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-
-alter table ac add constraint ac_check check (aa is not null);
-alter table bc no inherit ac;
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-alter table bc drop constraint ac_check;
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-alter table ac drop constraint ac_check;
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-
-drop table bc;
-drop table ac;
-
-create table ac (a int constraint check_a check (a <> 0));
-create table bc (a int constraint check_a check (a <> 0), b int constraint check_b check (b <> 0)) inherits (ac);
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc') order by 1,2;
-
-drop table bc;
-drop table ac;
-
-create table ac (a int constraint check_a check (a <> 0));
-create table bc (b int constraint check_b check (b <> 0));
-create table cc (c int constraint check_c check (c <> 0)) inherits (ac, bc);
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc', 'cc') order by 1,2;
-
-alter table cc no inherit bc;
-select pc.relname, pgc.conname, pgc.contype, pgc.conislocal, pgc.coninhcount, pg_get_expr(pgc.conbin, pc.oid) as consrc from pg_class as pc inner join pg_constraint as pgc on (pgc.conrelid = pc.oid) where pc.relname in ('ac', 'bc', 'cc') order by 1,2;
-
-drop table cc;
-drop table bc;
-drop table ac;
-
-create table p1(f1 int);
-create table p2(f2 int);
-create table c1(f3 int) inherits(p1,p2);
-insert into c1 values(1,-1,2);
-alter table p2 add constraint cc check (f2>0); -- fail
-alter table p2 add check (f2>0); -- check it without a name, too
-delete from c1;
-insert into c1 values(1,1,2);
-alter table p2 add check (f2>0);
-insert into c1 values(1,-1,2); -- fail
-create table c2(f3 int) inherits(p1,p2);
-\d c2
-create table c3 (f4 int) inherits(c1,c2);
-\d c3
-drop table p1 cascade;
-drop table p2 cascade;
-
-create table pp1 (f1 int);
-create table cc1 (f2 text, f3 int) inherits (pp1);
-alter table pp1 add column a1 int check (a1 > 0);
-\d cc1
-create table cc2(f4 float) inherits(pp1,cc1);
-\d cc2
-alter table pp1 add column a2 int check (a2 > 0);
-\d cc2
-drop table pp1 cascade;
-
--- Test for renaming in simple multiple inheritance
-CREATE TABLE inht1 (a int, b int);
-CREATE TABLE inhs1 (b int, c int);
-CREATE TABLE inhts (d int) INHERITS (inht1, inhs1);
-
-ALTER TABLE inht1 RENAME a TO aa;
-ALTER TABLE inht1 RENAME b TO bb; -- to be failed
-ALTER TABLE inhts RENAME aa TO aaa; -- to be failed
-ALTER TABLE inhts RENAME d TO dd;
-\d+ inhts
-
-DROP TABLE inhts;
-
--- Test for renaming in diamond inheritance
-CREATE TABLE inht2 (x int) INHERITS (inht1);
-CREATE TABLE inht3 (y int) INHERITS (inht1);
-CREATE TABLE inht4 (z int) INHERITS (inht2, inht3);
-
-ALTER TABLE inht1 RENAME aa TO aaa;
-\d+ inht4
-
-CREATE TABLE inhts (d int) INHERITS (inht2, inhs1);
-ALTER TABLE inht1 RENAME aaa TO aaaa;
-ALTER TABLE inht1 RENAME b TO bb; -- to be failed
-\d+ inhts
-
-WITH RECURSIVE r AS (
- SELECT 'inht1'::regclass AS inhrelid
-UNION ALL
- SELECT c.inhrelid FROM pg_inherits c, r WHERE r.inhrelid = c.inhparent
-)
-SELECT a.attrelid::regclass, a.attname, a.attinhcount, e.expected
- FROM (SELECT inhrelid, count(*) AS expected FROM pg_inherits
- WHERE inhparent IN (SELECT inhrelid FROM r) GROUP BY inhrelid) e
- JOIN pg_attribute a ON e.inhrelid = a.attrelid WHERE NOT attislocal
- ORDER BY a.attrelid::regclass::name, a.attnum;
-
-DROP TABLE inht1, inhs1 CASCADE;
-
-
--- Test non-inheritable indices [UNIQUE, EXCLUDE] constraints
-CREATE TABLE test_constraints (id int, val1 varchar, val2 int, UNIQUE(val1, val2));
-CREATE TABLE test_constraints_inh () INHERITS (test_constraints);
-\d+ test_constraints
-ALTER TABLE ONLY test_constraints DROP CONSTRAINT test_constraints_val1_val2_key;
-\d+ test_constraints
-\d+ test_constraints_inh
-DROP TABLE test_constraints_inh;
-DROP TABLE test_constraints;
-
-CREATE TABLE test_ex_constraints (
- c circle,
- EXCLUDE USING gist (c WITH &&)
-);
-CREATE TABLE test_ex_constraints_inh () INHERITS (test_ex_constraints);
-\d+ test_ex_constraints
-ALTER TABLE test_ex_constraints DROP CONSTRAINT test_ex_constraints_c_excl;
-\d+ test_ex_constraints
-\d+ test_ex_constraints_inh
-DROP TABLE test_ex_constraints_inh;
-DROP TABLE test_ex_constraints;
-
--- Test non-inheritable foreign key constraints
-CREATE TABLE test_primary_constraints(id int PRIMARY KEY);
-CREATE TABLE test_foreign_constraints(id1 int REFERENCES test_primary_constraints(id));
-CREATE TABLE test_foreign_constraints_inh () INHERITS (test_foreign_constraints);
-\d+ test_primary_constraints
-\d+ test_foreign_constraints
-ALTER TABLE test_foreign_constraints DROP CONSTRAINT test_foreign_constraints_id1_fkey;
-\d+ test_foreign_constraints
-\d+ test_foreign_constraints_inh
-DROP TABLE test_foreign_constraints_inh;
-DROP TABLE test_foreign_constraints;
-DROP TABLE test_primary_constraints;
-
--- Test foreign key behavior
-create table inh_fk_1 (a int primary key);
-insert into inh_fk_1 values (1), (2), (3);
-create table inh_fk_2 (x int primary key, y int references inh_fk_1 on delete cascade);
-insert into inh_fk_2 values (11, 1), (22, 2), (33, 3);
-create table inh_fk_2_child () inherits (inh_fk_2);
-insert into inh_fk_2_child values (111, 1), (222, 2);
-delete from inh_fk_1 where a = 1;
-select * from inh_fk_1 order by 1;
-select * from inh_fk_2 order by 1, 2;
-drop table inh_fk_1, inh_fk_2, inh_fk_2_child;
-
--- Test that parent and child CHECK constraints can be created in either order
-create table p1(f1 int);
-create table p1_c1() inherits(p1);
-
-alter table p1 add constraint inh_check_constraint1 check (f1 > 0);
-alter table p1_c1 add constraint inh_check_constraint1 check (f1 > 0);
-
-alter table p1_c1 add constraint inh_check_constraint2 check (f1 < 10);
-alter table p1 add constraint inh_check_constraint2 check (f1 < 10);
-
-select conrelid::regclass::text as relname, conname, conislocal, coninhcount
-from pg_constraint where conname like 'inh\_check\_constraint%'
-order by 1, 2;
-
-drop table p1 cascade;
-
--- Test that a valid child can have not-valid parent, but not vice versa
-create table invalid_check_con(f1 int);
-create table invalid_check_con_child() inherits(invalid_check_con);
-
-alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0) not valid;
-alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0); -- fail
-alter table invalid_check_con_child drop constraint inh_check_constraint;
-
-insert into invalid_check_con values(0);
-
-alter table invalid_check_con_child add constraint inh_check_constraint check(f1 > 0);
-alter table invalid_check_con add constraint inh_check_constraint check(f1 > 0) not valid;
-
-insert into invalid_check_con values(0); -- fail
-insert into invalid_check_con_child values(0); -- fail
-
-select conrelid::regclass::text as relname, conname,
- convalidated, conislocal, coninhcount, connoinherit
-from pg_constraint where conname like 'inh\_check\_constraint%'
-order by 1, 2;
-
--- We don't drop the invalid_check_con* tables, to test dump/reload with
-
---
--- Test parameterized append plans for inheritance trees
---
+SELECT pg_sleep(random()/10);
-create temp table patest0 (id, x) as
- select x, x from generate_series(0,1000) x;
-create temp table patest1() inherits (patest0);
-insert into patest1
- select x, x from generate_series(0,1000) x;
-create temp table patest2() inherits (patest0);
-insert into patest2
- select x, x from generate_series(0,1000) x;
-create index patest0i on patest0(id);
-create index patest1i on patest1(id);
-create index patest2i on patest2(id);
-analyze patest0;
-analyze patest1;
-analyze patest2;
-
-explain (costs off)
-select * from patest0 join (select f1 from int4_tbl limit 1) ss on id = f1;
-select * from patest0 join (select f1 from int4_tbl limit 1) ss on id = f1;
-
-drop index patest2i;
-
-explain (costs off)
select * from patest0 join (select f1 from int4_tbl limit 1) ss on id = f1;
select * from patest0 join (select f1 from int4_tbl limit 1) ss on id = f1;
diff --git a/src/test/regress/sql/vacuum.sql b/src/test/regress/sql/vacuum.sql
index 18cb7fd08ac..e9232356b6e 100644
--- a/src/test/regress/sql/vacuum.sql
+++ b/src/test/regress/sql/vacuum.sql
@@ -3,78 +3,11 @@
--
CREATE TABLE vactst (i INT);
-INSERT INTO vactst VALUES (1);
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst VALUES (0);
-SELECT count(*) FROM vactst;
-DELETE FROM vactst WHERE i != 0;
-SELECT * FROM vactst;
-VACUUM FULL vactst;
-UPDATE vactst SET i = i + 1;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst SELECT * FROM vactst;
-INSERT INTO vactst VALUES (0);
-SELECT count(*) FROM vactst;
-DELETE FROM vactst WHERE i != 0;
-VACUUM (FULL) vactst;
-DELETE FROM vactst;
-SELECT * FROM vactst;
-
-VACUUM (FULL, FREEZE) vactst;
-VACUUM (ANALYZE, FULL) vactst;
CREATE TABLE vaccluster (i INT PRIMARY KEY);
ALTER TABLE vaccluster CLUSTER ON vaccluster_pkey;
CLUSTER vaccluster;
-CREATE FUNCTION do_analyze() RETURNS VOID VOLATILE LANGUAGE SQL
- AS 'ANALYZE pg_am';
-CREATE FUNCTION wrap_do_analyze(c INT) RETURNS INT IMMUTABLE LANGUAGE SQL
- AS 'SELECT $1 FROM do_analyze()';
-CREATE INDEX ON vaccluster(wrap_do_analyze(i));
-INSERT INTO vaccluster VALUES (1), (2);
-ANALYZE vaccluster;
-
--- Test ANALYZE in transaction, where the transaction surrounding
--- analyze performed modifications. This tests for the bug at
--- https://postgr.es/m/c7988239-d42c-ddc4-41db-171b23b35e4f%40ssinger.info
--- (which hopefully is unlikely to be reintroduced), but also seems
--- independently worthwhile to cover.
-INSERT INTO vactst SELECT generate_series(1, 300);
-DELETE FROM vactst WHERE i % 7 = 0; -- delete a few rows outside
-BEGIN;
-INSERT INTO vactst SELECT generate_series(301, 400);
-DELETE FROM vactst WHERE i % 5 <> 0; -- delete a few rows inside
-ANALYZE vactst;
-COMMIT;
-
-VACUUM FULL pg_am;
-VACUUM FULL pg_class;
-VACUUM FULL pg_database;
-VACUUM FULL vaccluster;
-VACUUM FULL vactst;
-
-VACUUM (DISABLE_PAGE_SKIPPING) vaccluster;
-
-- PARALLEL option
CREATE TABLE pvactst (i INT, a INT[], p POINT) with (autovacuum_enabled = off);
INSERT INTO pvactst SELECT i, array[1,2,3], point(i, i+1) FROM generate_series(1,1000) i;
@@ -144,97 +77,7 @@ VACUUM no_index_cleanup;
-- Test some extra relations.
VACUUM (INDEX_CLEANUP FALSE) vaccluster;
VACUUM (INDEX_CLEANUP AUTO) vactst; -- index cleanup option is ignored if no indexes
-VACUUM (INDEX_CLEANUP FALSE, FREEZE TRUE) vaccluster;
-
--- TRUNCATE option
-CREATE TABLE vac_truncate_test(i INT NOT NULL, j text)
- WITH (vacuum_truncate=true, autovacuum_enabled=false);
-INSERT INTO vac_truncate_test VALUES (1, NULL), (NULL, NULL);
-VACUUM (TRUNCATE FALSE) vac_truncate_test;
-SELECT pg_relation_size('vac_truncate_test') > 0;
-VACUUM vac_truncate_test;
-SELECT pg_relation_size('vac_truncate_test') = 0;
-VACUUM (TRUNCATE FALSE, FULL TRUE) vac_truncate_test;
-DROP TABLE vac_truncate_test;
-
--- partitioned table
-CREATE TABLE vacparted (a int, b char) PARTITION BY LIST (a);
-CREATE TABLE vacparted1 PARTITION OF vacparted FOR VALUES IN (1);
-INSERT INTO vacparted VALUES (1, 'a');
-UPDATE vacparted SET b = 'b';
-VACUUM (ANALYZE) vacparted;
-VACUUM (FULL) vacparted;
-VACUUM (FREEZE) vacparted;
-
--- check behavior with duplicate column mentions
-VACUUM ANALYZE vacparted(a,b,a);
-ANALYZE vacparted(a,b,b);
-
--- partitioned table with index
-CREATE TABLE vacparted_i (a int primary key, b varchar(100))
- PARTITION BY HASH (a);
-CREATE TABLE vacparted_i1 PARTITION OF vacparted_i
- FOR VALUES WITH (MODULUS 2, REMAINDER 0);
-CREATE TABLE vacparted_i2 PARTITION OF vacparted_i
- FOR VALUES WITH (MODULUS 2, REMAINDER 1);
-INSERT INTO vacparted_i SELECT i, 'test_'|| i from generate_series(1,10) i;
-VACUUM (ANALYZE) vacparted_i;
-VACUUM (FULL) vacparted_i;
-VACUUM (FREEZE) vacparted_i;
-SELECT relname, relhasindex FROM pg_class
- WHERE relname LIKE 'vacparted_i%' AND relkind IN ('p','r')
- ORDER BY relname;
-DROP TABLE vacparted_i;
-
--- multiple tables specified
-VACUUM vaccluster, vactst;
-VACUUM vacparted, does_not_exist;
-VACUUM (FREEZE) vacparted, vaccluster, vactst;
-VACUUM (FREEZE) does_not_exist, vaccluster;
-VACUUM ANALYZE vactst, vacparted (a);
-VACUUM ANALYZE vactst (does_not_exist), vacparted (b);
-VACUUM FULL vacparted, vactst;
-VACUUM FULL vactst, vacparted (a, b), vaccluster (i);
-ANALYZE vactst, vacparted;
-ANALYZE vacparted (b), vactst;
-ANALYZE vactst, does_not_exist, vacparted;
-ANALYZE vactst (i), vacparted (does_not_exist);
-ANALYZE vactst, vactst;
-BEGIN; -- ANALYZE behaves differently inside a transaction block
-ANALYZE vactst, vactst;
-COMMIT;
-
--- parenthesized syntax for ANALYZE
-ANALYZE (VERBOSE) does_not_exist;
-ANALYZE (nonexistent-arg) does_not_exist;
-ANALYZE (nonexistentarg) does_not_exit;
-
--- ensure argument order independence, and that SKIP_LOCKED on non-existing
--- relation still errors out. Suppress WARNING messages caused by concurrent
--- autovacuums.
-SET client_min_messages TO 'ERROR';
-ANALYZE (SKIP_LOCKED, VERBOSE) does_not_exist;
-ANALYZE (VERBOSE, SKIP_LOCKED) does_not_exist;
-
--- SKIP_LOCKED option
-VACUUM (SKIP_LOCKED) vactst;
-VACUUM (SKIP_LOCKED, FULL) vactst;
-ANALYZE (SKIP_LOCKED) vactst;
-RESET client_min_messages;
-
--- ensure VACUUM and ANALYZE don't have a problem with serializable
-SET default_transaction_isolation = serializable;
-VACUUM vactst;
-ANALYZE vactst;
-RESET default_transaction_isolation;
-BEGIN TRANSACTION ISOLATION LEVEL SERIALIZABLE;
-ANALYZE vactst;
-COMMIT;
-
--- PROCESS_TOAST option
-ALTER TABLE vactst ADD COLUMN t TEXT;
-ALTER TABLE vactst ALTER COLUMN t SET STORAGE EXTERNAL;
-VACUUM (PROCESS_TOAST FALSE) vactst;
+SELECT pg_sleep(random()/2);
VACUUM (PROCESS_TOAST FALSE, FULL) vactst;
DROP TABLE vaccluster;
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-10-29 18:17 Peter Geoghegan <pg@bowt.ie>
parent: Alexander Lakhin <exclusion@gmail.com>
2 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2021-10-29 18:17 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>; Matthias van de Meent <boekewurm+postgres@gmail.com>
On Fri, Oct 29, 2021 at 6:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I can propose the debugging patch to reproduce the issue that replaces
> the hang with the assert and modifies a pair of crash-causing test
> scripts to simplify the reproducing. (Sorry, I have no time now to prune
> down the scripts further as I have to leave for a week.)
This bug is similar to the one fixed in commit d9d8aa9b. And so I
wonder if code like GlobalVisTestFor() is missing something that it
needs for partitioned tables.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-01 15:15 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Matthias van de Meent @ 2021-11-01 15:15 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>; Matthias van de Meent <boekewurm+postgres@gmail.com>
On Fri, 29 Oct 2021 at 20:17, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Fri, Oct 29, 2021 at 6:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> > I can propose the debugging patch to reproduce the issue that replaces
> > the hang with the assert and modifies a pair of crash-causing test
> > scripts to simplify the reproducing. (Sorry, I have no time now to prune
> > down the scripts further as I have to leave for a week.)
>
> This bug is similar to the one fixed in commit d9d8aa9b. And so I
> wonder if code like GlobalVisTestFor() is missing something that it
> needs for partitioned tables.
Without `autovacuum = off; fsync = off` I could not replicate the
issue in the configured 10m time window; with those options I did get
the reported trace in minutes.
I think that I also have found the culprit, which is something we
talked about in [0]: GlobalVisState->maybe_needed was not guaranteed
to never move backwards when recalculated, and because vacuum can
update its snapshot bounds (heap_prune_satisfies_vacuum ->
GlobalVisTestIsRemovableFullXid -> GlobalVisUpdate) this maybe_needed
could move backwards, resulting in the observed behaviour.
It was my understanding based on the mail conversation that Andres
would fix this observed issue too while fixing [0] (whose fix was
included with beta 2), but apparently I was wrong; I can't find the
code for 'maybe_needed'-won't-move-backwards-in-a-backend.
I (again) propose the attached patch, which ensures that this
maybe_needed field will not move backwards for a backend. It is
based on 14, but should be applied on head as well, because it's
lacking there as well.
Another alternative would be to replace the use of vacrel->OldestXmin
with `vacrel->vistest->maybe_needed` in lazy_scan_prune, but I believe
that is not legal in how vacuum works (we cannot unilaterally decide
that we want to retain tuples < OldestXmin).
Note: After fixing the issue with retreating maybe_needed I also hit
your segfault, and I'm still trying to find out what the source of
that issue might be. I do think it is an issue seperate from stuck
vacuum, though.
Kind regards,
Matthias van de Meent
[0] https://www.postgresql.org/message-id/flat/20210609184506.rqm5rikoikm47csf%40alap3.anarazel.de#e9d55...
Attachments:
0001-Fix-stuck-vacuum-due-to-retreating-GlobalVisState-ma.patch (application/octet-stream, 2.0K) download
From d7bad690ec67a5cec3e229cf5a282e83019cede8 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Mon, 1 Nov 2021 15:51:30 +0100
Subject: [PATCH] Fix stuck vacuum due to retreating
GlobalVisState->maybe_needed
The issue is solved by guaranteeing that a backend's maybe_needed
will never go backwards.
---
src/backend/storage/ipc/procarray.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 9fcd12020a..374b7a2633 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -4087,17 +4087,25 @@ static void
GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons)
{
GlobalVisSharedRels.maybe_needed =
- FullXidRelativeTo(horizons->latest_completed,
- horizons->shared_oldest_nonremovable);
+ FullTransactionIdNewer(
+ GlobalVisSharedRels.maybe_needed,
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->shared_oldest_nonremovable));
GlobalVisCatalogRels.maybe_needed =
- FullXidRelativeTo(horizons->latest_completed,
- horizons->catalog_oldest_nonremovable);
+ FullTransactionIdNewer(
+ GlobalVisCatalogRels.maybe_needed,
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->catalog_oldest_nonremovable));
GlobalVisDataRels.maybe_needed =
- FullXidRelativeTo(horizons->latest_completed,
- horizons->data_oldest_nonremovable);
+ FullTransactionIdNewer(
+ GlobalVisDataRels.maybe_needed,
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->data_oldest_nonremovable));
GlobalVisTempRels.maybe_needed =
- FullXidRelativeTo(horizons->latest_completed,
- horizons->temp_oldest_nonremovable);
+ FullTransactionIdNewer(
+ GlobalVisTempRels.maybe_needed,
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->temp_oldest_nonremovable));
/*
* In longer running transactions it's possible that transactions we
--
2.30.2
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-03 15:45 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Matthias van de Meent @ 2021-11-03 15:45 UTC (permalink / raw)
To: ; +Cc: Peter Geoghegan <pg@bowt.ie>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Mon, 1 Nov 2021 at 16:15, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
>
> On Fri, 29 Oct 2021 at 20:17, Peter Geoghegan <pg@bowt.ie> wrote:
> >
> > On Fri, Oct 29, 2021 at 6:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> > > I can propose the debugging patch to reproduce the issue that replaces
> > > the hang with the assert and modifies a pair of crash-causing test
> > > scripts to simplify the reproducing. (Sorry, I have no time now to prune
> > > down the scripts further as I have to leave for a week.)
> >
> > This bug is similar to the one fixed in commit d9d8aa9b. And so I
> > wonder if code like GlobalVisTestFor() is missing something that it
> > needs for partitioned tables.
>
> Without `autovacuum = off; fsync = off` I could not replicate the
> issue in the configured 10m time window; with those options I did get
> the reported trace in minutes.
>
> I think that I also have found the culprit, which is something we
> talked about in [0]: GlobalVisState->maybe_needed was not guaranteed
> to never move backwards when recalculated, and because vacuum can
> update its snapshot bounds (heap_prune_satisfies_vacuum ->
> GlobalVisTestIsRemovableFullXid -> GlobalVisUpdate) this maybe_needed
> could move backwards, resulting in the observed behaviour.
>
> It was my understanding based on the mail conversation that Andres
> would fix this observed issue too while fixing [0] (whose fix was
> included with beta 2), but apparently I was wrong; I can't find the
> code for 'maybe_needed'-won't-move-backwards-in-a-backend.
>
> I (again) propose the attached patch, which ensures that this
> maybe_needed field will not move backwards for a backend. It is
> based on 14, but should be applied on head as well, because it's
> lacking there as well.
>
> Another alternative would be to replace the use of vacrel->OldestXmin
> with `vacrel->vistest->maybe_needed` in lazy_scan_prune, but I believe
> that is not legal in how vacuum works (we cannot unilaterally decide
> that we want to retain tuples < OldestXmin).
>
> Note: After fixing the issue with retreating maybe_needed I also hit
> your segfault, and I'm still trying to find out what the source of
> that issue might be. I do think it is an issue seperate from stuck
> vacuum, though.
After further debugging, I think these both might be caused by the
same issue, due to xmin horizon confusion as a result from restored
snapshots:
I seem to repeatedly get backends of which the xmin is set from
InvalidTransactionId to some value < min(ProcGlobal->xids), which then
result in shared_oldest_nonremovable (and others) being less than the
value of their previous iteration. This leads to the infinite loop in
lazy_scan_prune (it stores and uses one value of
*_oldest_nonremovable, whereas heap_page_prune uses a more up-to-date
variant). Ergo, this issue is not really solved by my previous patch,
because apparently at this point we have snapshots wih an xmin that is
only registered in the backend's procarray entry when the xmin is
already out of scope, which makes it generally impossible to determine
what tuples may or may not yet be vacuumed.
I noticed that when this happens, generally a parallel vacuum worker
is involved. I also think that this is intimately related to [0], and
how snapshots are restored in parallel workers: A vacuum worker is
generally ignored, but if its snapshot has the oldest xmin available,
then a parallel worker launched from that vacuum worker will move the
visible xmin backwards. Same for concurrent index creation jobs.
Kind regards,
Matthias van de Meent
[0] https://www.postgresql.org/message-id/flat/202110191807.5svc3kmm32tl%40alvherre.pgsql
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-03 16:21 Peter Geoghegan <pg@bowt.ie>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2021-11-03 16:21 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Wed, Nov 3, 2021 at 8:46 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> I seem to repeatedly get backends of which the xmin is set from
> InvalidTransactionId to some value < min(ProcGlobal->xids), which then
> result in shared_oldest_nonremovable (and others) being less than the
> value of their previous iteration. This leads to the infinite loop in
> lazy_scan_prune (it stores and uses one value of
> *_oldest_nonremovable, whereas heap_page_prune uses a more up-to-date
> variant).
> I noticed that when this happens, generally a parallel vacuum worker
> is involved.
Hmm. That is plausible. The way that VACUUM (and concurrent index
builds) avoid being seen via the PROC_IN_VACUUM thing is pretty
delicate. Wouldn't surprise me if the parallel VACUUM issue subtly
broke lazy_scan_prune in the way that we see here.
What about testing? Can we find a simple way of reducing this
complicated repro to a less complicated repro with a failing
assertion? Maybe an assertion that we get to keep after the bug is
fixed?
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-05 11:43 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 2 replies; 43+ messages in thread
From: Matthias van de Meent @ 2021-11-05 11:43 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Wed, 3 Nov 2021 at 17:21, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Wed, Nov 3, 2021 at 8:46 AM Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> > I seem to repeatedly get backends of which the xmin is set from
> > InvalidTransactionId to some value < min(ProcGlobal->xids), which then
> > result in shared_oldest_nonremovable (and others) being less than the
> > value of their previous iteration. This leads to the infinite loop in
> > lazy_scan_prune (it stores and uses one value of
> > *_oldest_nonremovable, whereas heap_page_prune uses a more up-to-date
> > variant).
>
> > I noticed that when this happens, generally a parallel vacuum worker
> > is involved.
>
> Hmm. That is plausible. The way that VACUUM (and concurrent index
> builds) avoid being seen via the PROC_IN_VACUUM thing is pretty
> delicate. Wouldn't surprise me if the parallel VACUUM issue subtly
> broke lazy_scan_prune in the way that we see here.
>
> What about testing? Can we find a simple way of reducing this
> complicated repro to a less complicated repro with a failing
> assertion? Maybe an assertion that we get to keep after the bug is
> fixed?
I added the attached instrumentation for checking xmin validity, which
asserts what I believe are correct claims about the proc
infrastructure:
- It is always safe to set ->xmin to InvalidTransactionId: This
removes any claim that we have a snapshot anyone should worry about.
- If we have a valid ->xmin set, it is always safe to increase its value.
- Otherwise, the xmin must not lower the overall xmin of the database
it is connected to, plus some potential conditions for status flags.
It also may not be set without first taking the ProcArrayLock:
without synchronised access to the proc array, you cannot guarantee
you can set your xmin to a globally correct value.
It worked well with the bgworker flags patch [0], until I added this
instrumentation to SnapshotResetXmin and ran the regression tests: I
stumbled upon the following issue with aborting transactions, and I
don't know what the correct solution is to solve it:
AbortTransaction (see xact.c) calls ProcArrayEndTransaction, which can
reset MyProc->xmin to InvalidTransactionId (both directly and through
ProcArrayEndTransactionInternal). So far, this is safe.
However, later in AbortTransaction we call ResourceOwnerRelease(...,
RESOURCE_RELEASE_AFTER_LOCKS...), which will clean up the snapshots
stored in its owner->snapshotarr array using UnregisterSnapshot.
Then, if UnregisterSnapshot determines that a snapshot is now not
referenced anymore, and that snapshot has no active count, then it
will call SnapshotResetXmin().
Finally, when SnapshotResetXmin() is called, the oldest still
registered snapshot in RegisteredSnapshots will be pulled and
MyProc->xmin will be set to that snapshot's xmin.
Similarly, in AbortTransaction we call AtEOXact_Inval, which calls
ProcessInvalidationMessages -> LocalExecuteInvalidationMessage ->
InvalidateCatalogSnapshot -> SnapshotResetXmin, also setting
MyProc->xmin back to a non-InvalidXid value.
Note that from a third-party observer's standpoint we've just moved
our horizons backwards, and the regression tests (correctly) fail when
assertions are enabled.
I don't know what the expected behaviour is, but I do know that this
is a violation of the expected invariant of xmin never goes backwards
(for any of the cluster, database or data level).
Kind regards,
Matthias van de Meent
[0] https://www.postgresql.org/message-id/CAD21AoDkERUJkGEuQRiyGKmVRt2duU378UgnwBpqXQjA%2BEY3Lg%40mail.g...
Attachments:
v2-0001-Add-instrumentation-for-xmin-horizon-validation.patch (application/x-patch, 13.3K) download
From 9bd76028a10a1bb9a55136a95344d72ca3e11b84 Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Fri, 5 Nov 2021 12:23:59 +0100
Subject: [PATCH v2] Add instrumentation for xmin horizon validation
We have an unwritten rule that a backend's xmin may not go back, but
this is not enforced. The added check is for assertion checking only, and
allows one to check before writing this xmin to the global procarray that
this xmin is actually valid and may be written with the current state.
Checks include consistency at the cluster- and database level, locking,
and status flags.
---
src/backend/replication/logical/snapbuild.c | 13 +-
src/backend/replication/walsender.c | 6 +
src/backend/storage/ipc/procarray.c | 162 +++++++++++++++++++-
src/backend/utils/time/snapmgr.c | 3 +
src/include/storage/procarray.h | 6 +
5 files changed, 171 insertions(+), 19 deletions(-)
diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c
index 6df602485b..5fd126748b 100644
--- a/src/backend/replication/logical/snapbuild.c
+++ b/src/backend/replication/logical/snapbuild.c
@@ -545,18 +545,7 @@ SnapBuildInitialSnapshot(SnapBuild *builder)
* mechanism. Due to that we can do this without locks, we're only
* changing our own value.
*/
-#ifdef USE_ASSERT_CHECKING
- {
- TransactionId safeXid;
-
- LWLockAcquire(ProcArrayLock, LW_SHARED);
- safeXid = GetOldestSafeDecodingTransactionId(false);
- LWLockRelease(ProcArrayLock);
-
- Assert(TransactionIdPrecedesOrEquals(safeXid, snap->xmin));
- }
-#endif
-
+ CheckXminIsConsistent(snap->xmin, false);
MyProc->xmin = snap->xmin;
/* allocate in transaction context */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 3224536356..212c455eaf 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2171,9 +2171,15 @@ ProcessStandbyHSFeedbackMessage(void)
{
if (TransactionIdIsNormal(feedbackCatalogXmin)
&& TransactionIdPrecedes(feedbackCatalogXmin, feedbackXmin))
+ {
+ CheckXminIsConsistent(feedbackCatalogXmin, false);
MyProc->xmin = feedbackCatalogXmin;
+ }
else
+ {
+ CheckXminIsConsistent(feedbackXmin, false);
MyProc->xmin = feedbackXmin;
+ }
}
}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 9fcd12020a..2395ee856d 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -355,6 +355,7 @@ static void MaintainLatestCompletedXidRecovery(TransactionId latestXid);
static inline FullTransactionId FullXidRelativeTo(FullTransactionId rel,
TransactionId xid);
static void GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons);
+static void ComputeXidHorizons(ComputeXidHorizonsResult *h, bool already_locked);
/*
* Report shared-memory space needed by CreateSharedProcArray.
@@ -1698,14 +1699,15 @@ TransactionIdIsActive(TransactionId xid)
* doesn't expect (breaking HOT).
*/
static void
-ComputeXidHorizons(ComputeXidHorizonsResult *h)
+ComputeXidHorizons(ComputeXidHorizonsResult *h, bool already_locked)
{
ProcArrayStruct *arrayP = procArray;
TransactionId kaxmin;
bool in_recovery = RecoveryInProgress();
TransactionId *other_xids = ProcGlobal->xids;
- LWLockAcquire(ProcArrayLock, LW_SHARED);
+ if (!already_locked)
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
h->latest_completed = ShmemVariableCache->latestCompletedXid;
@@ -1743,6 +1745,21 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
h->temp_oldest_nonremovable = MyProc->xid;
else
h->temp_oldest_nonremovable = initial;
+
+ /* Sanity checks: values are always >= our previous maybe_needed */
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed, initial),
+ GlobalVisSharedRels.maybe_needed));
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed, initial),
+ GlobalVisCatalogRels.maybe_needed));
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed, initial),
+ GlobalVisDataRels.maybe_needed));
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed,
+ h->temp_oldest_nonremovable),
+ GlobalVisTempRels.maybe_needed));
}
/*
@@ -1795,6 +1812,11 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
if (statusFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING))
continue;
+ /* Sanity check: xmin can not be lower than maybe_needed */
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed, xmin),
+ GlobalVisSharedRels.maybe_needed));
+
/* shared tables need to take backends in all databases into account */
h->shared_oldest_nonremovable =
TransactionIdOlder(h->shared_oldest_nonremovable, xmin);
@@ -1822,8 +1844,19 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
* only on vacuums of user-defined tables.
*/
if (!(statusFlags & PROC_IN_SAFE_IC))
+ {
+ /* Sanity check: xmin can not be lower than maybe_needed */
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed, xmin),
+ GlobalVisDataRels.maybe_needed));
h->data_oldest_nonremovable =
TransactionIdOlder(h->data_oldest_nonremovable, xmin);
+ }
+
+ /* Sanity check: xmin can not be lower than maybe_needed */
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(h->latest_completed, xmin),
+ GlobalVisCatalogRels.maybe_needed));
/* Catalog tables need to consider all backends in this db */
h->catalog_oldest_nonremovable =
@@ -1847,7 +1880,8 @@ ComputeXidHorizons(ComputeXidHorizonsResult *h)
* No other information from shared state is needed, release the lock
* immediately. The rest of the computations can be done without a lock.
*/
- LWLockRelease(ProcArrayLock);
+ if (!already_locked)
+ LWLockRelease(ProcArrayLock);
if (in_recovery)
{
@@ -2006,7 +2040,7 @@ GetOldestNonRemovableTransactionId(Relation rel)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
switch (GlobalVisHorizonKindForRel(rel))
{
@@ -2034,7 +2068,7 @@ GetOldestTransactionIdConsideredRunning(void)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
return horizons.oldest_considered_running;
}
@@ -2047,7 +2081,7 @@ GetReplicationHorizons(TransactionId *xmin, TransactionId *catalog_xmin)
{
ComputeXidHorizonsResult horizons;
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
/*
* Don't want to use shared_oldest_nonremovable here, as that contains the
@@ -2153,7 +2187,10 @@ GetSnapshotDataReuse(Snapshot snapshot)
* xmin.
*/
if (!TransactionIdIsValid(MyProc->xmin))
+ {
+ CheckXminIsConsistent(snapshot->xmin, true);
MyProc->xmin = TransactionXmin = snapshot->xmin;
+ }
RecentXmin = snapshot->xmin;
Assert(TransactionIdPrecedesOrEquals(TransactionXmin, RecentXmin));
@@ -2441,7 +2478,10 @@ GetSnapshotData(Snapshot snapshot)
replication_slot_catalog_xmin = procArray->replication_slot_catalog_xmin;
if (!TransactionIdIsValid(MyProc->xmin))
+ {
+ CheckXminIsConsistent(xmin, true);
MyProc->xmin = TransactionXmin = xmin;
+ }
LWLockRelease(ProcArrayLock);
@@ -2619,6 +2659,7 @@ ProcArrayInstallImportedXmin(TransactionId xmin,
* GetSnapshotData first, we'll be overwriting a valid xmin here, so
* we don't check that.)
*/
+ CheckXminIsConsistent(xmin, true);
MyProc->xmin = TransactionXmin = xmin;
result = true;
@@ -2662,6 +2703,8 @@ ProcArrayInstallRestoredXmin(TransactionId xmin, PGPROC *proc)
TransactionIdIsNormal(xid) &&
TransactionIdPrecedesOrEquals(xid, xmin))
{
+ /* restore xmin */
+ CheckXminIsConsistent(xmin, true);
MyProc->xmin = TransactionXmin = xmin;
result = true;
}
@@ -4086,6 +4129,23 @@ GlobalVisTestShouldUpdate(GlobalVisState *state)
static void
GlobalVisUpdateApply(ComputeXidHorizonsResult *horizons)
{
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->shared_oldest_nonremovable),
+ GlobalVisSharedRels.maybe_needed));
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->catalog_oldest_nonremovable),
+ GlobalVisCatalogRels.maybe_needed));
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->data_oldest_nonremovable),
+ GlobalVisDataRels.maybe_needed));
+ Assert(FullTransactionIdFollowsOrEquals(
+ FullXidRelativeTo(horizons->latest_completed,
+ horizons->temp_oldest_nonremovable),
+ GlobalVisTempRels.maybe_needed));
+
GlobalVisSharedRels.maybe_needed =
FullXidRelativeTo(horizons->latest_completed,
horizons->shared_oldest_nonremovable);
@@ -4128,7 +4188,7 @@ GlobalVisUpdate(void)
ComputeXidHorizonsResult horizons;
/* updates the horizons as a side-effect */
- ComputeXidHorizons(&horizons);
+ ComputeXidHorizons(&horizons, false);
}
/*
@@ -5132,3 +5192,91 @@ KnownAssignedXidsReset(void)
LWLockRelease(ProcArrayLock);
}
+
+#ifdef USE_ASSERT_CHECKING
+/*
+ * CheckXminIsConsistent
+ * Checks that an xmin is consistent with the current state of
+ * the PGPROC infrastructure.
+ *
+ * Any xmin that is set on MyProc (or any other proc, really) must be
+ * consistent with the current state of the pgproc infrastructure, i.e. it may
+ * not move the globally minimum considered running xid backwards. Results
+ * from transactions older than the oldest considered running transaction may
+ * be removed or otherwise modified through e.g. vacuum, whereas results from
+ * transactions that are considered running may not yet be removed and are
+ * thus safe from removal.
+ *
+ * Basically, we assert that with this xmin we would not move the GlobalVis*
+ * horizons backwards.
+ */
+void
+CheckXminIsConsistent(TransactionId xmin, bool already_locked)
+{
+ ComputeXidHorizonsResult result;
+
+ /*
+ * There can be reasons for xmins to be incorrectly registered under
+ * non-postmaster environments, whilst still retaining overall
+ * consistency. However, when under postmaster, data consistency cannot
+ * be guaranteed unless all xmins are correctly registered while
+ * that xmin is still inside the oldest_considered_running window.
+ */
+ if (!IsPostmasterEnvironment)
+ return;
+
+ /* Resetting xmin is generally safe */
+ if (!TransactionIdIsValid(xmin))
+ {
+ /* We cannot set unset xmin without also having no xid */
+ Assert(!TransactionIdIsValid(MyProc->xid));
+ return;
+ }
+ /* Moving our own xmin forward is always safe */
+ if (TransactionIdIsValid(MyProc->xmin) &&
+ TransactionIdFollowsOrEquals(xmin, MyProc->xmin))
+ return;
+
+ /*
+ * For the following conditions, we can only update MyProc->xmin
+ * while we hold the ProcArrayLock: we have to be certain that the
+ * xmin that we're installing is still inside the visible range
+ * for this cluster. For any caller, it is impossible to guarantee
+ * that the origin of their xmin is still valid when they call
+ * this function unless they hold a lock on the Proc array.
+ *
+ * Note that the lock held may be at both LW_SHARED and LW_EXCLUSIVE
+ * granularity: The xmin we're installing will not lower the visibility
+ * horizon for any running backend, and therefore shouldn't impact
+ * concurrently generated results.
+ */
+ Assert(already_locked);
+ Assert(LWLockHeldByMe(ProcArrayLock));
+
+ ComputeXidHorizons(&result, already_locked);
+
+ /*
+ * xmin may not be older than the current oldest transaction
+ */
+ Assert(TransactionIdFollowsOrEquals(xmin, result.oldest_considered_running));
+
+ /*
+ * If we don't have a special status, xmin must also not be older
+ * than the oldest removable xid for the database that this
+ * backend is connected to.
+ */
+ if (MyProc->databaseId == InvalidOid)
+ return;
+
+ if (MyProc->statusFlags & PROC_IN_VACUUM)
+ {
+ Assert(TransactionIdFollowsOrEquals(xmin, result.catalog_oldest_nonremovable));
+ }
+ else if (MyProc->statusFlags & PROC_IN_SAFE_IC)
+ {
+ Assert(TransactionIdFollowsOrEquals(xmin, result.catalog_oldest_nonremovable));
+ }
+ else
+ Assert(TransactionIdFollowsOrEquals(xmin, result.data_oldest_nonremovable));
+}
+#endif /* USE_ASSERT_CHECKING */
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index dca1bc8afc..3f3641298f 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -954,7 +954,10 @@ SnapshotResetXmin(void)
pairingheap_first(&RegisteredSnapshots));
if (TransactionIdPrecedes(MyProc->xmin, minSnapshot->xmin))
+ {
+ CheckXminIsConsistent(minSnapshot->xmin, false);
MyProc->xmin = minSnapshot->xmin;
+ }
}
/*
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index b01fa52139..cca407c859 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -94,4 +94,10 @@ extern void ProcArraySetReplicationSlotXmin(TransactionId xmin,
extern void ProcArrayGetReplicationSlotXmin(TransactionId *xmin,
TransactionId *catalog_xmin);
+#ifdef USE_ASSERT_CHECKING
+extern void CheckXminIsConsistent(TransactionId xmin, bool already_locked);
+#else /* USE_ASSERT_CHECKING */
+#define CheckXminIsConsistent(xmin, already_locked) ((void) 0)
+#endif /* USE_ASSERT_CHECKING */
+
#endif /* PROCARRAY_H */
--
2.30.2
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-05 21:25 Peter Geoghegan <pg@bowt.ie>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2021-11-05 21:25 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Nov 5, 2021 at 4:43 AM Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> I added the attached instrumentation for checking xmin validity, which
> asserts what I believe are correct claims about the proc
> infrastructure:
This test case involves partitioning, but also pruning, which is very
particular about heap tuple headers being a certain way following
updates. I wonder if we're missing a
HeapTupleHeaderIndicatesMovedPartitions() test somewhere. Could be in
heapam/VACUUM/pruning code, or could be somewhere else.
Take a look at commit f16241bef7 to get some idea of what I mean.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-06 01:20 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 0 replies; 43+ messages in thread
From: Matthias van de Meent @ 2021-11-06 01:20 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; Andres Freund <andres@anarazel.de>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, 5 Nov 2021 at 22:25, Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Fri, Nov 5, 2021 at 4:43 AM Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> > I added the attached instrumentation for checking xmin validity, which
> > asserts what I believe are correct claims about the proc
> > infrastructure:
>
> This test case involves partitioning, but also pruning, which is very
> particular about heap tuple headers being a certain way following
> updates. I wonder if we're missing a
> HeapTupleHeaderIndicatesMovedPartitions() test somewhere. Could be in
> heapam/VACUUM/pruning code, or could be somewhere else.
If you watch closely, the second backtrace in [0] (the segfault)
originates from the code that builds the partition bounds based on
relcaches / catalog tables, which are never partitioned. Although it
is indeed in the partition infrastructure, if we'd have a tuple with
HeapTupleHeaderIndicatesMovedPartitions() at that point, then that'd
be a bug (we do not partition catalogs).
But I hit this same segfault earlier while testing, and I deduced that
problem that I hit at that point was that there was that an index
entry could not resolve to a heap tuple (or the scan at partdesc.c:227
otherwise returned NULL where one result was expected); so that tuple
is NULL at partdesc.c:230, and heap_getattr subsequently segfaults
when it dereferences the null tuple pointer to access it's fields.
Due to the blatant visibility horizon confusion, the failing scan
being on the pg_class table, and the test case including aggressive
manual vacuuming of the pg_class table, I assume that the error was
caused by vacuum having removed tuples from pg_class, while other
backends still required / expected access to these tuples.
Kind regards,
Matthias
[0] https://www.postgresql.org/message-id/d5d5af5d-ba46-aff3-9f91-776c70246cc3%40gmail.com
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-06 01:59 Andres Freund <andres@anarazel.de>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
1 sibling, 0 replies; 43+ messages in thread
From: Andres Freund @ 2021-11-06 01:59 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 2021-11-05 12:43:00 +0100, Matthias van de Meent wrote:
> I added the attached instrumentation for checking xmin validity, which
> asserts what I believe are correct claims about the proc
> infrastructure:
>
> - It is always safe to set ->xmin to InvalidTransactionId: This
> removes any claim that we have a snapshot anyone should worry about.
> - If we have a valid ->xmin set, it is always safe to increase its value.
I think I know what you mean, but of course you cannot just increase xmin if
there are existing snapshots requiring that xmin.
> - Otherwise, the xmin must not lower the overall xmin of the database
> it is connected to, plus some potential conditions for status flags.
walsenders can end up doing this IIRC.
> It also may not be set without first taking the ProcArrayLock:
> without synchronised access to the proc array, you cannot guarantee
> you can set your xmin to a globally correct value.
There's possibly one exception around this, which is snapshot import. But
that's rare enough that an unnecessary acquisition is fine.
> It worked well with the bgworker flags patch [0], until I added this
> instrumentation to SnapshotResetXmin and ran the regression tests: I
> stumbled upon the following issue with aborting transactions, and I
> don't know what the correct solution is to solve it:
>
> AbortTransaction (see xact.c) calls ProcArrayEndTransaction, which can
> reset MyProc->xmin to InvalidTransactionId (both directly and through
> ProcArrayEndTransactionInternal). So far, this is safe.
>
> However, later in AbortTransaction we call ResourceOwnerRelease(...,
> RESOURCE_RELEASE_AFTER_LOCKS...), which will clean up the snapshots
> stored in its owner->snapshotarr array using UnregisterSnapshot.
> Then, if UnregisterSnapshot determines that a snapshot is now not
> referenced anymore, and that snapshot has no active count, then it
> will call SnapshotResetXmin().
> Finally, when SnapshotResetXmin() is called, the oldest still
> registered snapshot in RegisteredSnapshots will be pulled and
> MyProc->xmin will be set to that snapshot's xmin.
Yea, that's not great. This is a pretty old behaviour, IIRC?
> We have an unwritten rule that a backend's xmin may not go back, but
> this is not enforced.
I don't think we can make any of this hard assertions. There's e.g. the
following comment:
* Note: despite the above, it's possible for the calculated values to move
* backwards on repeated calls. The calculated values are conservative, so
* that anything older is definitely not considered as running by anyone
* anymore, but the exact values calculated depend on a number of things. For
* example, if there are no transactions running in the current database, the
* horizon for normal tables will be latestCompletedXid. If a transaction
* begins after that, its xmin will include in-progress transactions in other
* databases that started earlier, so another call will return a lower value.
* Nonetheless it is safe to vacuum a table in the current database with the
* first result. There are also replication-related effects: a walsender
* process can set its xmin based on transactions that are no longer running
* on the primary but are still being replayed on the standby, thus possibly
* making the values go backwards. In this case there is a possibility that
* we lose data that the standby would like to have, but unless the standby
* uses a replication slot to make its xmin persistent there is little we can
* do about that --- data is only protected if the walsender runs continuously
* while queries are executed on the standby. (The Hot Standby code deals
* with such cases by failing standby queries that needed to access
* already-removed data, so there's no integrity bug.) The computed values
* are also adjusted with vacuum_defer_cleanup_age, so increasing that setting
* on the fly is another easy way to make horizons move backwards, with no
* consequences for data integrity.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2021-11-10 20:28 Peter Geoghegan <pg@bowt.ie>
parent: Alexander Lakhin <exclusion@gmail.com>
2 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2021-11-10 20:28 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Oct 29, 2021 at 6:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I can propose the debugging patch to reproduce the issue that replaces
> the hang with the assert and modifies a pair of crash-causing test
> scripts to simplify the reproducing. (Sorry, I have no time now to prune
> down the scripts further as I have to leave for a week.)
>
> The reproducing script is:
I cannot reproduce this bug by following your steps, even when the
assertion is made to fail after only 5 retries (5 is still ludicrously
excessive, 100 might be overkill). And even when I don't use a debug
build (and make the assertion into an equivalent PANIC). I wonder why
that is. I didn't have much trouble following your similar repro for
bug #17255.
My immediate goal in trying to follow your reproducer was to determine
what effect (if any) the pending bugfix for #17255 [1] has on this
bug. It seems more than possible that this bug is in fact a different
manifestation of the same underlying problem we see in #17255. And so
that should be the next thing we check here.
[1] https://postgr.es/m/CAH2-WzkpG9KLQF5sYHaOO_dSVdOjM+dv=nTEn85oNfMUTk836Q@mail.gmail.com
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2023-12-25 02:44 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Noah Misch @ 2023-12-25 02:44 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; Peter Geoghegan <pg@bowt.ie>; +Cc: robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Mon, Nov 01, 2021 at 04:15:27PM +0100, Matthias van de Meent wrote:
> Another alternative would be to replace the use of vacrel->OldestXmin
> with `vacrel->vistest->maybe_needed` in lazy_scan_prune, but I believe
v17 commit 1ccc1e05ae essentially did that.
> that is not legal in how vacuum works (we cannot unilaterally decide
> that we want to retain tuples < OldestXmin).
Do you think commit 1ccc1e05ae creates problems in that respect? It does have
the effect of retaining tuples for which GlobalVisState rules "retain" but
HeapTupleSatisfiesVacuum() would have ruled "delete". If that doesn't create
problems, then back-patching commit 1ccc1e05ae could be a fix for remaining
infinite-retries scenarios, if any.
On Wed, Nov 10, 2021 at 12:28:43PM -0800, Peter Geoghegan wrote:
> On Fri, Oct 29, 2021 at 6:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> > I can propose the debugging patch to reproduce the issue that replaces
> > the hang with the assert and modifies a pair of crash-causing test
> > scripts to simplify the reproducing. (Sorry, I have no time now to prune
> > down the scripts further as I have to leave for a week.)
> >
> > The reproducing script is:
>
> I cannot reproduce this bug by following your steps, even when the
> assertion is made to fail after only 5 retries (5 is still ludicrously
> excessive, 100 might be overkill). And even when I don't use a debug
> build (and make the assertion into an equivalent PANIC). I wonder why
> that is. I didn't have much trouble following your similar repro for
> bug #17255.
For what it's worth, I needed "-X" on the script's psql command lines to keep
my ~/.psqlrc from harming things. I also wondered if the regression database
needed to be populated with a "make installcheck" run. The script had a
"createdb regression" without a "make installcheck", so I assumed an empty
regression database was intended.
> My immediate goal in trying to follow your reproducer was to determine
> what effect (if any) the pending bugfix for #17255 [1] has on this
> bug. It seems more than possible that this bug is in fact a different
> manifestation of the same underlying problem we see in #17255. And so
> that should be the next thing we check here.
>
> [1] https://postgr.es/m/CAH2-WzkpG9KLQF5sYHaOO_dSVdOjM+dv=nTEn85oNfMUTk836Q@mail.gmail.com
Using the https://postgr.es/m/d5d5af5d-ba46-aff3-9f91-776c70246cc3@gmail.com
procedure, I see these results:
- A commit from the day of that email, 2021-10-29, (5ccceb2946) reproduced the
"numretries" assertion failure in each of five 10m runs.
- At the 2022-01-13 commit (18b87b201f^) just before the fix for #17255, the
script instead gets: FailedAssertion("HeapTupleHeaderIsHeapOnly(htup)",
File: "pruneheap.c", Line: 964. That happened once in two 10m runs, so it
was harder to reach than the numretries failure.
- At 18b87b201f, a 1440m script run got no failures.
I've seen symptoms that suggest the infinite-numretries bug remains reachable,
but I don't know how to reproduce them. (Given the upthread notes about xmin
going backward during end-of-xact, I'd first try some pauses there.) If it
does remain reachable, likely some other code change between 2021-10 and
2022-01 made this particular test script no longer reach it.
Thanks,
nm
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2023-12-31 23:53 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2023-12-31 23:53 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sun, Dec 24, 2023 at 6:44 PM Noah Misch <noah@leadboat.com> wrote:
> On Mon, Nov 01, 2021 at 04:15:27PM +0100, Matthias van de Meent wrote:
> > Another alternative would be to replace the use of vacrel->OldestXmin
> > with `vacrel->vistest->maybe_needed` in lazy_scan_prune, but I believe
>
> v17 commit 1ccc1e05ae essentially did that.
Obviously, 1ccc1e05ae would fix the immediate problem of infinite
retries, since it just rips out the loop.
> > that is not legal in how vacuum works (we cannot unilaterally decide
> > that we want to retain tuples < OldestXmin).
>
> Do you think commit 1ccc1e05ae creates problems in that respect? It does have
> the effect of retaining tuples for which GlobalVisState rules "retain" but
> HeapTupleSatisfiesVacuum() would have ruled "delete". If that doesn't create
> problems, then back-patching commit 1ccc1e05ae could be a fix for remaining
> infinite-retries scenarios, if any.
My guess is that there is a decent chance that backpatching 1ccc1e05ae
would be okay, but that isn't much use. I really don't know either way
right now. And I wouldn't like to speculate too much further before
gaining a proper understanding of what's going on here. Seems to be
specific to partitioning with cross-partition updates.
> Using the https://postgr.es/m/d5d5af5d-ba46-aff3-9f91-776c70246cc3@gmail.com
> procedure, I see these results:
>
> - A commit from the day of that email, 2021-10-29, (5ccceb2946) reproduced the
> "numretries" assertion failure in each of five 10m runs.
>
> - At the 2022-01-13 commit (18b87b201f^) just before the fix for #17255, the
> script instead gets: FailedAssertion("HeapTupleHeaderIsHeapOnly(htup)",
> File: "pruneheap.c", Line: 964. That happened once in two 10m runs, so it
> was harder to reach than the numretries failure.
>
> - At 18b87b201f, a 1440m script run got no failures.
>
> I've seen symptoms that suggest the infinite-numretries bug remains reachable,
> but I don't know how to reproduce them. (Given the upthread notes about xmin
> going backward during end-of-xact, I'd first try some pauses there.) If it
> does remain reachable, likely some other code change between 2021-10 and
> 2022-01 made this particular test script no longer reach it.
I am aware of a production database that appears to run into the same
problem. Inserts and concurrent cross-partition updates are used
heavily on this instance (the table in question uses partitioning).
Perhaps you happened upon a similar problematic production database,
and found this thread when researching the issue? Maybe we've both
seen the same problem in the wild?
I have seen VACUUM get stuck like this on multiple versions, all
associated with the same application code/partitioning
scheme/workload. This includes a 15.4 instance, and various 14.* point
release instances. It seems likely that there is a bug here, and that
it affects Postgres 14, 15, and 16.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-06 20:24 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 3 replies; 43+ messages in thread
From: Noah Misch @ 2024-01-06 20:24 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sun, Dec 31, 2023 at 03:53:34PM -0800, Peter Geoghegan wrote:
> On Sun, Dec 24, 2023 at 6:44 PM Noah Misch <noah@leadboat.com> wrote:
> > On Mon, Nov 01, 2021 at 04:15:27PM +0100, Matthias van de Meent wrote:
> > > Another alternative would be to replace the use of vacrel->OldestXmin
> > > with `vacrel->vistest->maybe_needed` in lazy_scan_prune, but I believe
> >
> > v17 commit 1ccc1e05ae essentially did that.
>
> Obviously, 1ccc1e05ae would fix the immediate problem of infinite
> retries, since it just rips out the loop.
Yep.
> > > that is not legal in how vacuum works (we cannot unilaterally decide
> > > that we want to retain tuples < OldestXmin).
> >
> > Do you think commit 1ccc1e05ae creates problems in that respect? It does have
> > the effect of retaining tuples for which GlobalVisState rules "retain" but
> > HeapTupleSatisfiesVacuum() would have ruled "delete". If that doesn't create
> > problems, then back-patching commit 1ccc1e05ae could be a fix for remaining
> > infinite-retries scenarios, if any.
>
> My guess is that there is a decent chance that backpatching 1ccc1e05ae
> would be okay, but that isn't much use. I really don't know either way
> right now. And I wouldn't like to speculate too much further before
> gaining a proper understanding of what's going on here.
Fair enough. While I agree there's a decent chance back-patching would be
okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
Matthias theorized. Something like: we update relfrozenxid based on
OldestXmin, even though GlobalVisState caused us to retain a tuple older than
OldestXmin. Then relfrozenxid disagrees with table contents.
> Seems to be
> specific to partitioning with cross-partition updates.
>
> > Using the https://postgr.es/m/d5d5af5d-ba46-aff3-9f91-776c70246cc3@gmail.com
> > procedure, I see these results:
> >
> > - A commit from the day of that email, 2021-10-29, (5ccceb2946) reproduced the
> > "numretries" assertion failure in each of five 10m runs.
> >
> > - At the 2022-01-13 commit (18b87b201f^) just before the fix for #17255, the
> > script instead gets: FailedAssertion("HeapTupleHeaderIsHeapOnly(htup)",
> > File: "pruneheap.c", Line: 964. That happened once in two 10m runs, so it
> > was harder to reach than the numretries failure.
> >
> > - At 18b87b201f, a 1440m script run got no failures.
> >
> > I've seen symptoms that suggest the infinite-numretries bug remains reachable,
> > but I don't know how to reproduce them. (Given the upthread notes about xmin
> > going backward during end-of-xact, I'd first try some pauses there.) If it
> > does remain reachable, likely some other code change between 2021-10 and
> > 2022-01 made this particular test script no longer reach it.
>
> I am aware of a production database that appears to run into the same
> problem. Inserts and concurrent cross-partition updates are used
> heavily on this instance (the table in question uses partitioning).
> Perhaps you happened upon a similar problematic production database,
> and found this thread when researching the issue? Maybe we've both
> seen the same problem in the wild?
I did find this thread while researching the symptoms I was seeing. No
partitioning where I saw them.
> I have seen VACUUM get stuck like this on multiple versions, all
> associated with the same application code/partitioning
> scheme/workload. This includes a 15.4 instance, and various 14.* point
> release instances. It seems likely that there is a bug here, and that
> it affects Postgres 14, 15, and 16.
Agreed.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-06 21:30 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
2 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-06 21:30 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sat, Jan 6, 2024 at 12:24 PM Noah Misch <noah@leadboat.com> wrote:
> On Sun, Dec 31, 2023 at 03:53:34PM -0800, Peter Geoghegan wrote:
> > My guess is that there is a decent chance that backpatching 1ccc1e05ae
> > would be okay, but that isn't much use. I really don't know either way
> > right now. And I wouldn't like to speculate too much further before
> > gaining a proper understanding of what's going on here.
>
> Fair enough. While I agree there's a decent chance back-patching would be
> okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
> Matthias theorized. Something like: we update relfrozenxid based on
> OldestXmin, even though GlobalVisState caused us to retain a tuple older than
> OldestXmin. Then relfrozenxid disagrees with table contents.
Either every relevant code path has the same OldestXmin to work off
of, or the whole NewRelfrozenXid/relfrozenxid-tracking thing can't be
expected to work as designed. I find it a bit odd that
pruneheap.c/GlobalVisState has no direct understanding of this
dependency (none that I can discern, at least). Wouldn't it at least
be more natural if pruneheap.c could access OldestXmin when run inside
VACUUM? (Could just be used by defensive hardening code.)
We're also relying on vacuumlazy.c's call to vacuum_get_cutoffs()
(which itself calls GetOldestNonRemovableTransactionId) taking place
before vacuumlazy.c goes on to call GlobalVisTestFor() a few lines
further down (I think). It seems like even the code in procarray.c
might have something to say about the vacuumlazy.c dependency, too.
But offhand it doesn't look like it does, either. Why shouldn't we
expect random implementation details in code like ComputeXidHorizons()
to break the assumption/dependency within vacuumlazy.c?
I also worry about the possibility that GlobalVisTestShouldUpdate()
masks problems in this area (as opposed to causing the problems). It
seems very hard to test.
> I did find this thread while researching the symptoms I was seeing. No
> partitioning where I saw them.
If this was an isolated incident then it could perhaps have been a
symptom of corruption. Though corruption seems highly unlikely to be
involved with the cases that I've seen, since they appear
intermittently, across a variety of different contexts/versions.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-06 21:41 Peter Geoghegan <pg@bowt.ie>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-06 21:41 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sat, Jan 6, 2024 at 1:30 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Sat, Jan 6, 2024 at 12:24 PM Noah Misch <noah@leadboat.com> wrote:
> > Fair enough. While I agree there's a decent chance back-patching would be
> > okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
> > Matthias theorized. Something like: we update relfrozenxid based on
> > OldestXmin, even though GlobalVisState caused us to retain a tuple older than
> > OldestXmin. Then relfrozenxid disagrees with table contents.
>
> Either every relevant code path has the same OldestXmin to work off
> of, or the whole NewRelfrozenXid/relfrozenxid-tracking thing can't be
> expected to work as designed. I find it a bit odd that
> pruneheap.c/GlobalVisState has no direct understanding of this
> dependency (none that I can discern, at least).
What do you think of the idea of adding a defensive "can't happen"
error to lazy_scan_prune() that will catch DEAD or RECENTLY_DEAD
tuples with storage that still contain an xmax < OldestXmin? This
probably won't catch every possible problem, but I suspect it'll work
well enough.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-06 22:44 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Noah Misch @ 2024-01-06 22:44 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sat, Jan 06, 2024 at 01:30:40PM -0800, Peter Geoghegan wrote:
> On Sat, Jan 6, 2024 at 12:24 PM Noah Misch <noah@leadboat.com> wrote:
> > On Sun, Dec 31, 2023 at 03:53:34PM -0800, Peter Geoghegan wrote:
> > > My guess is that there is a decent chance that backpatching 1ccc1e05ae
> > > would be okay, but that isn't much use. I really don't know either way
> > > right now. And I wouldn't like to speculate too much further before
> > > gaining a proper understanding of what's going on here.
> >
> > Fair enough. While I agree there's a decent chance back-patching would be
> > okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
> > Matthias theorized. Something like: we update relfrozenxid based on
> > OldestXmin, even though GlobalVisState caused us to retain a tuple older than
> > OldestXmin. Then relfrozenxid disagrees with table contents.
>
> Either every relevant code path has the same OldestXmin to work off
> of, or the whole NewRelfrozenXid/relfrozenxid-tracking thing can't be
> expected to work as designed. I find it a bit odd that
> pruneheap.c/GlobalVisState has no direct understanding of this
> dependency (none that I can discern, at least). Wouldn't it at least
> be more natural if pruneheap.c could access OldestXmin when run inside
> VACUUM? (Could just be used by defensive hardening code.)
Tied to that decision is the choice of semantics when the xmin horizon moves
backward during one VACUUM, e.g. when a new walsender xmin does so. Options:
1. Continue to remove tuples based on the OldestXmin from VACUUM's start. We
could have already removed some of those tuples, so the walsender xmin
won't achieve a guarantee anyway. (VACUUM would want ratchet-like behavior
in GlobalVisState, possibly by sharing OldestXmin with pruneheap like you
say.)
2. Move OldestXmin backward, to reflect the latest xmin horizon. (Perhaps
VACUUM would just pass GlobalVisState to a function that returns the
compatible OldestXmin.)
Which way is better?
> We're also relying on vacuumlazy.c's call to vacuum_get_cutoffs()
> (which itself calls GetOldestNonRemovableTransactionId) taking place
> before vacuumlazy.c goes on to call GlobalVisTestFor() a few lines
> further down (I think). It seems like even the code in procarray.c
> might have something to say about the vacuumlazy.c dependency, too.
> But offhand it doesn't look like it does, either. Why shouldn't we
> expect random implementation details in code like ComputeXidHorizons()
> to break the assumption/dependency within vacuumlazy.c?
Makes sense.
On Sat, Jan 06, 2024 at 01:41:23PM -0800, Peter Geoghegan wrote:
> What do you think of the idea of adding a defensive "can't happen"
> error to lazy_scan_prune() that will catch DEAD or RECENTLY_DEAD
> tuples with storage that still contain an xmax < OldestXmin? This
> probably won't catch every possible problem, but I suspect it'll work
> well enough.
So before the "goto retry", ERROR if the tuple header suggests this mismatch
is happening? That, or limiting the retry count, could help.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-08 17:02 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-08 17:02 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sat, Jan 6, 2024 at 5:44 PM Noah Misch <noah@leadboat.com> wrote:
> Tied to that decision is the choice of semantics when the xmin horizon moves
> backward during one VACUUM, e.g. when a new walsender xmin does so. Options:
>
> 1. Continue to remove tuples based on the OldestXmin from VACUUM's start. We
> could have already removed some of those tuples, so the walsender xmin
> won't achieve a guarantee anyway. (VACUUM would want ratchet-like behavior
> in GlobalVisState, possibly by sharing OldestXmin with pruneheap like you
> say.)
>
> 2. Move OldestXmin backward, to reflect the latest xmin horizon. (Perhaps
> VACUUM would just pass GlobalVisState to a function that returns the
> compatible OldestXmin.)
>
> Which way is better?
I suppose that a hybrid of these two approaches makes the most sense.
A design that's a lot closer to your #1 than to your #2.
Under this scheme, pruneheap.c would be explicitly aware of
OldestXmin, and would promise to respect the exact invariant that we
need to avoid getting stuck in lazy_scan_prune's loop (or avoid
confused NewRelfrozenXid tracking on HEAD, which no longer has this
loop). But it'd be limited to that exact invariant; we'd still avoid
unduly imposing any requirements on pruning-away deleted tuples whose
xmax was >= OldestXmin. lazy_scan_prune/vacuumlazy.c shouldn't care if
we prune away any "extra" heap tuples, just because we can (or just
because it's convenient to the implementation). Andres has in the past
placed a lot of emphasis on being able to update the
GlobalVisState-wise bounds on the fly. Not sure that it's really that
important that VACUUM does that, but there is no reason to not allow
it. So we can keep that property (as well as the aforementioned
high-level OldestXmin immutability property).
More importantly (at least to me), this scheme allows vacuumlazy.c to
continue to treat OldestXmin as an immutable cutoff for both pruning
and freezing -- the high level design doesn't need any revisions. We
already "freeze away" multixact member XIDs >= OldestXmin in certain
rare cases (i.e. we remove lockers that are determined to no longer be
running in FreezeMultiXactId's "second pass" slow path), so there is
nothing fundamentally novel about the idea of removing some extra XIDs
>= OldestXmin in passing, just because it happens to be convenient to
some low-level piece of code that's external to vacuumlazy.c.
What do you think of that general approach? I see no reason why it
matters if OldestXmin goes backwards across two VACUUM operations, so
I haven't tried to avoid that.
> On Sat, Jan 06, 2024 at 01:41:23PM -0800, Peter Geoghegan wrote:
> > What do you think of the idea of adding a defensive "can't happen"
> > error to lazy_scan_prune() that will catch DEAD or RECENTLY_DEAD
> > tuples with storage that still contain an xmax < OldestXmin? This
> > probably won't catch every possible problem, but I suspect it'll work
> > well enough.
>
> So before the "goto retry", ERROR if the tuple header suggests this mismatch
> is happening? That, or limiting the retry count, could help.
When I wrote this code, my understanding was that the sole reason for
needing to loop back was a concurrently-aborted xact. In principle we
ought to be able to test the tuple to detect if it's that exact case
(the only truly valid case), and then throw an error if we somehow got
it wrong. That kind of hardening would at least be correct according
to my original understanding of things.
There is an obvious practical concern with adding such hardening now:
what if the current loop is accidentally protective, in whatever way?
That seems quite possible. I seem to recall that Andres supposed at
some point that the loop's purpose wasn't limited to the
concurrently-aborted-inserter case that I believed was the only
relevant case back when I worked on what became commit 8523492d4e
("Remove tupgone special case from vacuumlazy.c"). I don't have a
reference for that, but I'm pretty sure it was said at some point
around the release of 14.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-08 18:21 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Noah Misch @ 2024-01-08 18:21 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Mon, Jan 08, 2024 at 12:02:01PM -0500, Peter Geoghegan wrote:
> On Sat, Jan 6, 2024 at 5:44 PM Noah Misch <noah@leadboat.com> wrote:
> > Tied to that decision is the choice of semantics when the xmin horizon moves
> > backward during one VACUUM, e.g. when a new walsender xmin does so. Options:
> >
> > 1. Continue to remove tuples based on the OldestXmin from VACUUM's start. We
> > could have already removed some of those tuples, so the walsender xmin
> > won't achieve a guarantee anyway. (VACUUM would want ratchet-like behavior
> > in GlobalVisState, possibly by sharing OldestXmin with pruneheap like you
> > say.)
> >
> > 2. Move OldestXmin backward, to reflect the latest xmin horizon. (Perhaps
> > VACUUM would just pass GlobalVisState to a function that returns the
> > compatible OldestXmin.)
> >
> > Which way is better?
>
> I suppose that a hybrid of these two approaches makes the most sense.
> A design that's a lot closer to your #1 than to your #2.
>
> Under this scheme, pruneheap.c would be explicitly aware of
> OldestXmin, and would promise to respect the exact invariant that we
> need to avoid getting stuck in lazy_scan_prune's loop (or avoid
> confused NewRelfrozenXid tracking on HEAD, which no longer has this
> loop). But it'd be limited to that exact invariant; we'd still avoid
> unduly imposing any requirements on pruning-away deleted tuples whose
> xmax was >= OldestXmin. lazy_scan_prune/vacuumlazy.c shouldn't care if
> we prune away any "extra" heap tuples, just because we can (or just
> because it's convenient to the implementation). Andres has in the past
> placed a lot of emphasis on being able to update the
> GlobalVisState-wise bounds on the fly. Not sure that it's really that
> important that VACUUM does that, but there is no reason to not allow
> it. So we can keep that property (as well as the aforementioned
> high-level OldestXmin immutability property).
>
> More importantly (at least to me), this scheme allows vacuumlazy.c to
> continue to treat OldestXmin as an immutable cutoff for both pruning
> and freezing -- the high level design doesn't need any revisions. We
> already "freeze away" multixact member XIDs >= OldestXmin in certain
> rare cases (i.e. we remove lockers that are determined to no longer be
> running in FreezeMultiXactId's "second pass" slow path), so there is
> nothing fundamentally novel about the idea of removing some extra XIDs
> >= OldestXmin in passing, just because it happens to be convenient to
> some low-level piece of code that's external to vacuumlazy.c.
>
> What do you think of that general approach?
That all sounds good to me.
> I see no reason why it
> matters if OldestXmin goes backwards across two VACUUM operations, so
> I haven't tried to avoid that.
That may be fully okay, or we may want to clamp OldestXmin to be no older than
relfrozenxid. I don't feel great about the system moving relfrozenxid
backward unless it observed an older XID, and observing an older XID would be
a corruption signal. I don't have a specific way non-monotonic relfrozenxid
breaks things, though.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-08 18:36 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-08 18:36 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Mon, Jan 8, 2024 at 1:21 PM Noah Misch <noah@leadboat.com> wrote:
> > I see no reason why it
> > matters if OldestXmin goes backwards across two VACUUM operations, so
> > I haven't tried to avoid that.
>
> That may be fully okay, or we may want to clamp OldestXmin to be no older than
> relfrozenxid. I don't feel great about the system moving relfrozenxid
> backward unless it observed an older XID, and observing an older XID would be
> a corruption signal. I don't have a specific way non-monotonic relfrozenxid
> breaks things, though.
We're already prepared for this -- relfrozenxid simply cannot go
backwards, regardless of what vacuumlazy.c thinks. That is,
vac_update_relstats() won't accept a new relfrozenxid that is < its
existing value (unless it's a value "from the future", which is a way
of recovering after historical pg_upgrade-related corruption bugs).
If memory serves it doesn't take much effort to exercise the relevant
code within vac_update_relstats(). I'm pretty sure that the regression
tests will fail if you run them after removing its defensive
no-older-relfrozenxid test (though I haven't checked recently).
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-09 20:59 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
2 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-09 20:59 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Sat, Jan 6, 2024 at 3:24 PM Noah Misch <noah@leadboat.com> wrote:
> On Sun, Dec 31, 2023 at 03:53:34PM -0800, Peter Geoghegan wrote:
> > I am aware of a production database that appears to run into the same
> > problem. Inserts and concurrent cross-partition updates are used
> > heavily on this instance (the table in question uses partitioning).
> > Perhaps you happened upon a similar problematic production database,
> > and found this thread when researching the issue? Maybe we've both
> > seen the same problem in the wild?
>
> I did find this thread while researching the symptoms I was seeing. No
> partitioning where I saw them.
I'm starting to think that partitioning was just a red herring. The
instance had problems in the presence of DELETEs, not UPDATEs (or
cross-partition UPDATEs).
Did the affected system that you investigated happen to have an
atypically high number of databases? The system 15.4 system that I saw
the problem on had almost 3,000 databases.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-09 21:44 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Noah Misch @ 2024-01-09 21:44 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Tue, Jan 09, 2024 at 03:59:19PM -0500, Peter Geoghegan wrote:
> On Sat, Jan 6, 2024 at 3:24 PM Noah Misch <noah@leadboat.com> wrote:
> > On Sun, Dec 31, 2023 at 03:53:34PM -0800, Peter Geoghegan wrote:
> > > I am aware of a production database that appears to run into the same
> > > problem. Inserts and concurrent cross-partition updates are used
> > > heavily on this instance (the table in question uses partitioning).
> > > Perhaps you happened upon a similar problematic production database,
> > > and found this thread when researching the issue? Maybe we've both
> > > seen the same problem in the wild?
> >
> > I did find this thread while researching the symptoms I was seeing. No
> > partitioning where I saw them.
>
> I'm starting to think that partitioning was just a red herring. The
> instance had problems in the presence of DELETEs, not UPDATEs (or
> cross-partition UPDATEs).
>
> Did the affected system that you investigated happen to have an
> atypically high number of databases? The system 15.4 system that I saw
> the problem on had almost 3,000 databases.
No, single-digit database count here.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-09 23:16 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-09 23:16 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Tue, Jan 9, 2024 at 4:44 PM Noah Misch <noah@leadboat.com> wrote:
> On Tue, Jan 09, 2024 at 03:59:19PM -0500, Peter Geoghegan wrote:
> > Did the affected system that you investigated happen to have an
> > atypically high number of databases? The system 15.4 system that I saw
> > the problem on had almost 3,000 databases.
>
> No, single-digit database count here.
My suspicion was that this factor might increase the propensity of
calls to GetOldestNonRemovableTransactionId (used to establish
VACUUM's OldestXmin) to not agree with the GlobalVis* based state used
by pruneheap.c, in the way that we need to worry about here (i.e.
inconsistencies that lead to VACUUM getting stuck inside
lazy_scan_prune's loop).
Using gdb I was able to determine that
ComputeXidHorizonsResultLastXmin == RecentXmin at some point long
after the system gets stuck (when I actually looked). So
GlobalVisTestShouldUpdate() doesn't return true at that point. And, I
see that VACUUM's OldestXmin value is between
GlobalVisDataRels.maybe_needed and
GlobalVisDataRels.definitely_needed. The deleted tuple's xmax is
committed according to OldestXmin (i.e. it's a value < OldestXmin),
and is < GlobalVisDataRels.definitely_needed, too. The same tuple xmax
is > GlobalVisDataRels.maybe_needed. As for this tuple's xmin, it was
already frozen by a previous VACUUM operation. The tuple infomask
flags indicate that it's a pretty standard deleted tuple.
Overall, there aren't a lot of details here that seem like they might
be out of the ordinary, hinting at a specific underlying cause.
It looks more like the assumptions that we make about OldestXmin
agreeing with GlobalVis* state just aren't quite robust, in general.
Ideally I'd be able to point to some specific assumption that has been
violated -- and we might yet tie the problem to some specific detail
that I've yet to identify. As I said upthread, I'm concerned that code
in places like procarray.c is rather loose about how the horizons are
recomputed, in a way that doesn't sit well with me.
GlobalVisTestShouldUpdate() thinks that it's okay to use
ComputeXidHorizonsResultLastXmin-based heuristics to decide when to
recompute horizons. It is more or less treated as a matter of weighing
costs against benefits -- not as a potential correctness issue.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-10 19:06 Peter Geoghegan <pg@bowt.ie>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-10 19:06 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Tue, Jan 9, 2024 at 6:16 PM Peter Geoghegan <pg@bowt.ie> wrote:
> On Tue, Jan 9, 2024 at 4:44 PM Noah Misch <noah@leadboat.com> wrote:
> > On Tue, Jan 09, 2024 at 03:59:19PM -0500, Peter Geoghegan wrote:
> > > Did the affected system that you investigated happen to have an
> > > atypically high number of databases? The system 15.4 system that I saw
> > > the problem on had almost 3,000 databases.
> >
> > No, single-digit database count here.
>
> My suspicion was that this factor might increase the propensity of
> calls to GetOldestNonRemovableTransactionId (used to establish
> VACUUM's OldestXmin) to not agree with the GlobalVis* based state used
> by pruneheap.c, in the way that we need to worry about here (i.e.
> inconsistencies that lead to VACUUM getting stuck inside
> lazy_scan_prune's loop).
Another question about your database/system: does VACUUM get stuck
while scanning a page some time after it has already completed a round
of index vacuuming? And if so, does an nbtree bulk delete end up
deleting and then recycling many index leaf pages (e.g., due to bulk
range deletions)?
That's what I see here -- I don't think that pruning leaves behind
even a single live heap tuple, despite scanning thousands of pages
before reaching the page that it gets stuck on. Could be another red
herring. But it doesn't seem impossible that some of the nbtree calls
to procarray.c routines performed by code added by my commit
9dd963ae25, "Recycle nbtree pages deleted during same VACUUM", are
somehow related. That is, that code could be part of the chain of
events that cause the problem (whether or not the code itself is
technically at fault).
I'm referring to calls such as the
"GetOldestNonRemovableTransactionId(NULL)" and
"GlobalVisCheckRemovableFullXid()" calls that take place inside
_bt_pendingfsm_finalize(). It's not like we do stuff like that in very
many other places.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-10 19:38 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Noah Misch @ 2024-01-10 19:38 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Wed, Jan 10, 2024 at 02:06:42PM -0500, Peter Geoghegan wrote:
> On Tue, Jan 9, 2024 at 6:16 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > On Tue, Jan 9, 2024 at 4:44 PM Noah Misch <noah@leadboat.com> wrote:
> > > On Tue, Jan 09, 2024 at 03:59:19PM -0500, Peter Geoghegan wrote:
> > > > Did the affected system that you investigated happen to have an
> > > > atypically high number of databases? The system 15.4 system that I saw
> > > > the problem on had almost 3,000 databases.
> > >
> > > No, single-digit database count here.
> >
> > My suspicion was that this factor might increase the propensity of
> > calls to GetOldestNonRemovableTransactionId (used to establish
> > VACUUM's OldestXmin) to not agree with the GlobalVis* based state used
> > by pruneheap.c, in the way that we need to worry about here (i.e.
> > inconsistencies that lead to VACUUM getting stuck inside
> > lazy_scan_prune's loop).
>
> Another question about your database/system: does VACUUM get stuck
> while scanning a page some time after it has already completed a round
> of index vacuuming?
I don't know. That particular system experienced the infinite loop only once.
> That's what I see here -- I don't think that pruning leaves behind
> even a single live heap tuple, despite scanning thousands of pages
> before reaching the page that it gets stuck on. Could be another red
> herring. But it doesn't seem impossible that some of the nbtree calls
> to procarray.c routines performed by code added by my commit
> 9dd963ae25, "Recycle nbtree pages deleted during same VACUUM", are
> somehow related. That is, that code could be part of the chain of
> events that cause the problem (whether or not the code itself is
> technically at fault).
>
> I'm referring to calls such as the
> "GetOldestNonRemovableTransactionId(NULL)" and
> "GlobalVisCheckRemovableFullXid()" calls that take place inside
> _bt_pendingfsm_finalize(). It's not like we do stuff like that in very
> many other places.
I see what you mean about the rarity and potential importance of
"GetOldestNonRemovableTransactionId(NULL)". There's just one other caller,
vac_update_datfrozenxid(), which calls it for an unrelated cause.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-10 19:57 Peter Geoghegan <pg@bowt.ie>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-01-10 19:57 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Wed, Jan 10, 2024 at 2:38 PM Noah Misch <noah@leadboat.com> wrote:
> I don't know. That particular system experienced the infinite loop only once.
While I certainly can't recreate the problem on demand, it has been
seen on this same application far more than once.
> > I'm referring to calls such as the
> > "GetOldestNonRemovableTransactionId(NULL)" and
> > "GlobalVisCheckRemovableFullXid()" calls that take place inside
> > _bt_pendingfsm_finalize(). It's not like we do stuff like that in very
> > many other places.
>
> I see what you mean about the rarity and potential importance of
> "GetOldestNonRemovableTransactionId(NULL)". There's just one other caller,
> vac_update_datfrozenxid(), which calls it for an unrelated cause.
I just noticed another detail that adds significant weight to this
theory: it looks like the problem is hit on the first tuple located on
the first heap page that VACUUM scans *after* it completes its first
round of index vacuuming (I'm inferring this from vacrel state,
particular its lpdead_items instrumentation counter). The dead_items
array is as large as possible here (just under 1 GiB), and
lpdead_items is 178956692 (which uses up all of our dead_items space).
VACUUM scans tens of gigabytes of heap pages before it begins this
initial round of index vacuuming (according to vacrel->scanned_pages).
What are the chances that all of this is just a coincidence? Low, I'd say.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-01-10 21:15 Noah Misch <noah@leadboat.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 0 replies; 43+ messages in thread
From: Noah Misch @ 2024-01-10 21:15 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Andres Freund <andres@anarazel.de>
On Wed, Jan 10, 2024 at 02:57:34PM -0500, Peter Geoghegan wrote:
> On Wed, Jan 10, 2024 at 2:38 PM Noah Misch <noah@leadboat.com> wrote:
> > > I'm referring to calls such as the
> > > "GetOldestNonRemovableTransactionId(NULL)" and
> > > "GlobalVisCheckRemovableFullXid()" calls that take place inside
> > > _bt_pendingfsm_finalize(). It's not like we do stuff like that in very
> > > many other places.
> >
> > I see what you mean about the rarity and potential importance of
> > "GetOldestNonRemovableTransactionId(NULL)". There's just one other caller,
> > vac_update_datfrozenxid(), which calls it for an unrelated cause.
>
> I just noticed another detail that adds significant weight to this
> theory: it looks like the problem is hit on the first tuple located on
> the first heap page that VACUUM scans *after* it completes its first
> round of index vacuuming (I'm inferring this from vacrel state,
> particular its lpdead_items instrumentation counter). The dead_items
> array is as large as possible here (just under 1 GiB), and
> lpdead_items is 178956692 (which uses up all of our dead_items space).
> VACUUM scans tens of gigabytes of heap pages before it begins this
> initial round of index vacuuming (according to vacrel->scanned_pages).
>
> What are the chances that all of this is just a coincidence? Low, I'd say.
Agreed. I bet you've made an important finding, there.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-05 20:20 Robert Haas <robertmhaas@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
2 siblings, 1 reply; 43+ messages in thread
From: Robert Haas @ 2024-04-05 20:20 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
On Fri, Oct 29, 2021 at 9:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I can propose the debugging patch to reproduce the issue that replaces
> the hang with the assert and modifies a pair of crash-causing test
> scripts to simplify the reproducing. (Sorry, I have no time now to prune
> down the scripts further as I have to leave for a week.)
Just FYI, I tried to reproduce this today on v16, using this formula,
with some hacking around to try to get it working on my MacBook, and I
couldn't get it to crash.
I doubt my failure to reproduce is because anything has been fixed.
It's probably due to some kind of dumbitude on my part. The patch
doesn't apply to the head of v16, unsurprisingly, and a bunch of the
Linux commands don't work here, so it's all kind of a muddle for me
trying to get the same setup in place. I should probably be better at
reproducing things like this than I am.
I also tried to reproduce with a simpler setup where I just ran a
normal pgbench in one terminal and a pgbench running only "vacuum
pgbench_accounts;" in another. What I see is that such a setup never
does a "goto retry;". I tried putting a loop around this code:
res = HeapTupleSatisfiesVacuum(&tuple, vacrel->cutoffs.OldestXmin,
buf);
...to do the same thing 1000 times, and I do see that if I do that,
the return value of HeapTupleSatisfiesVacuum() sometimes changes part
way through the thousand iterations. But so far I haven't seen a
single instance of it changing to HEAPTUPLE_DEAD. It seems to be all
stuff like HEAP_TUPLE_LIVE -> HEAPTUPLE_DELETE_IN_PROGRESS or
HEAPTUPLE_DELETE_IN_PROGRESS -> HEAPTUPLE_RECENTLY_DEAD. There are a
few other combinations that I see appearing too, but the point is that
the new state in this test never seems to be HEAPTUPLE_DEAD, and
therefore there's no retry and no ability to reproduce the bug.
Any ideas?
P.S. See also discussion on the "relfrozenxid may disagree with row
XIDs after 1ccc1e05ae" thread.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-07 10:00 Alexander Lakhin <exclusion@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Alexander Lakhin @ 2024-04-07 10:00 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi Robert,
05.04.2024 23:20, Robert Haas wrote:
> On Fri, Oct 29, 2021 at 9:30 AM Alexander Lakhin <exclusion@gmail.com> wrote:
>> I can propose the debugging patch to reproduce the issue that replaces
>> the hang with the assert and modifies a pair of crash-causing test
>> scripts to simplify the reproducing. (Sorry, I have no time now to prune
>> down the scripts further as I have to leave for a week.)
> Just FYI, I tried to reproduce this today on v16, using this formula,
> with some hacking around to try to get it working on my MacBook, and I
> couldn't get it to crash.
I've refreshed the script and simplified it a bit not to use Linux
specifics. This works for me (on REL_14_0, with the patch applied,
CPPFLAGS="-O0" ./configure --enable-debug --enable-cassert ...):
echo "
autovacuum=off
fsync=off
" >> "$PGDATA/postgresql.conf"
pg_ctl -w -l server.log start
export PGDATABASE=regression
createdb regression
echo "
vacuum (verbose, skip_locked, index_cleanup off) pg_catalog.pg_class;
select pg_sleep(random()/50);
" >/tmp/17257/pseudo-autovacuum.sql
export PGDATABASE=regression
createdb regression
pgbench -n -f /tmp/17257/inherit.sql -C -T 1200 >pgbench-1.log 2>&1 &
pgbench -n -f /tmp/17257/vacuum.sql -C -T 1200 >pgbench-2.log 2>&1 &
pgbench -n -f /tmp/17257/pseudo-autovacuum.sql -C -c 10 -T 1200 >pgbench-3.log 2>&1 &
wait
grep -E "(TRAP|terminated)" server.log
(Please use the attached inherit.sql, vacuum.sql (excerpts from
src/test/sql/{inherit,vacuum}.sql).)
With PGDATA placed on tmpfs, this script failed for me after 1m31s,
2m35s, 4m12s:
TRAP: FailedAssertion("numretries < 100", File: "vacuumlazy.c", Line: 1726, PID: 951498)
Another possible outcome:
TRAP: FailedAssertion("relid == targetRelId", File: "relcache.c", Line: 1062, PID: 1257766)
And also:
2024-04-07 05:03:21.656 UTC [2905313] LOG: server process (PID 2984687) was terminated by signal 6: Aborted
2024-04-07 05:03:21.656 UTC [2905313] DETAIL: Failed process was running: create table matest0 (id serial primary key,
name text);
With the stack trace:
...
#4 0x00007fc30b4007f3 in __GI_abort () at ./stdlib/abort.c:79
#5 0x0000559f50220719 in index_delete_sort_cmp (deltid1=0x559f523a9f40, deltid2=0x7ffd2f9623f8) at heapam.c:7582
#6 0x0000559f50220847 in index_delete_sort (delstate=0x7ffd2f9636f0) at heapam.c:7623
...
(as in [1])
But on dad1539ae I got no failures for 3 runs (the same is on
REL_16_STABLE with a slightly modified lazy_scan_prune patch).
[1] https://www.postgresql.org/message-id/17255-14c0ac58d0f9b583%40postgresql.org
Best regards,
Alexander
Attachments:
inherit.sql (application/sql, 17.4K) download
vacuum.sql (application/sql, 5.5K) download
assert-in-lazy_scan_prune-loop.patch (text/x-patch, 581B) download
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index d2c735eeda..79cec947f4 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1717,10 +1717,13 @@ lazy_scan_prune(LVRelState *vacrel,
int nfrozen;
OffsetNumber deadoffsets[MaxHeapTuplesPerPage];
xl_heap_freeze_tuple frozen[MaxHeapTuplesPerPage];
+int numretries = 0;
maxoff = PageGetMaxOffsetNumber(page);
retry:
+numretries++;
+Assert(numretries < 100);
/* Initialize (or reset) page-level counters */
tuples_deleted = 0;
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-09 18:59 Robert Haas <robertmhaas@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Robert Haas @ 2024-04-09 18:59 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Sun, Apr 7, 2024 at 6:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I've refreshed the script and simplified it a bit not to use Linux
> specifics.
Thanks. I was able to reproduce this today on macOS 13.6.2 with
REL_14_0. It didn't seem like it reproduced as quickly for me as it
did for you, but I got it to fail an assertion eventually. Then my
debugging efforts were frustrated by
http://postgr.es/m/CA+TgmobW9bEuvSrQR1D1K6_8=DmY2tzkuepAjCWF=j4B1w0rWw@mail.gmail.com
- gah
> But on dad1539ae I got no failures for 3 runs (the same is on
> REL_16_STABLE with a slightly modified lazy_scan_prune patch).
I'm having trouble understanding what this means exactly -- are you
talking about REL_16_STABLE, or about dad1539ae, or both, or what? At
any rate, it's really important here that we understand whether we
still have a bug here, and if so, in which releases and with what test
case. I wasn't aware of dad1539ae but that certainly seems like it
might've made a big difference, if not fixing the problem entirely
then at least making it a lot less likely. And I think it's possible
that some of the related freezing+pruning commits on master might have
removed the problem altogether, but that needs to be tested.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-09 20:00 Alexander Lakhin <exclusion@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Alexander Lakhin @ 2024-04-09 20:00 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
09.04.2024 21:59, Robert Haas wrote:
>
>> But on dad1539ae I got no failures for 3 runs (the same is on
>> REL_16_STABLE with a slightly modified lazy_scan_prune patch).
> I'm having trouble understanding what this means exactly -- are you
> talking about REL_16_STABLE, or about dad1539ae, or both, or what? At
> any rate, it's really important here that we understand whether we
> still have a bug here, and if so, in which releases and with what test
> case. I wasn't aware of dad1539ae but that certainly seems like it
> might've made a big difference, if not fixing the problem entirely
> then at least making it a lot less likely. And I think it's possible
> that some of the related freezing+pruning commits on master might have
> removed the problem altogether, but that needs to be tested.
I was talking about both — dad1539ae eliminated the bug (judging from the
repro) on REL_14_STABLE, and I suppose that 18b87b201 did the same for
REL_15_STABLE/REL_16_STABLE...
(On 18b87b201~1 I've got (twice):
TRAP: FailedAssertion("HeapTupleHeaderIsHeapOnly(htup)", File: "pruneheap.c", Line: 964, PID: 1854895)
I can double-check exactly 18b87b201 with the repro, if it makes sense.)
Best regards,
Alexander
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-15 16:17 Robert Haas <robertmhaas@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 0 replies; 43+ messages in thread
From: Robert Haas @ 2024-04-15 16:17 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Apr 9, 2024 at 4:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> I was talking about both — dad1539ae eliminated the bug (judging from the
> repro) on REL_14_STABLE, and I suppose that 18b87b201 did the same for
> REL_15_STABLE/REL_16_STABLE...
> (On 18b87b201~1 I've got (twice):
> TRAP: FailedAssertion("HeapTupleHeaderIsHeapOnly(htup)", File: "pruneheap.c", Line: 964, PID: 1854895)
I'm inclined to believe that there's a residual bug here, or at least
that there was one before 6dbb490261a6170a3fc3e326c6983ad63e795047 and
related commits. But if we can't reproduce the issue, I don't know how
much we can realistically do here. This logic is so complicated and so
tricky that I don't trust myself to say "yes, this is definitely
wrong, and after change XYZ it's definitely right." The bug is as
likely to be in my analysis as it is to be in the code itself.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-15 17:39 Andres Freund <andres@anarazel.de>
parent: Noah Misch <noah@leadboat.com>
2 siblings, 1 reply; 43+ messages in thread
From: Andres Freund @ 2024-04-15 17:39 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Melanie Plageman <melanieplageman@gmail.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; Matthias van de Meent <boekewurm+postgres@gmail.com>; robertmhaas@gmail.com, Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi,
I've tried a couple times to catch up with this thread. But always kinda felt
I must be missing something. It might be that this is one part of the
confusion:
On 2024-01-06 12:24:13 -0800, Noah Misch wrote:
> Fair enough. While I agree there's a decent chance back-patching would be
> okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
> Matthias theorized. Something like: we update relfrozenxid based on
> OldestXmin, even though GlobalVisState caused us to retain a tuple older than
> OldestXmin. Then relfrozenxid disagrees with table contents.
Looking at the state as of 1ccc1e05ae, I don't see how - in lazy_scan_prune(),
if heap_page_prune() spuriously didn't prune a tuple, because the horizon went
backwards, we'd encounter the tuple in the loop below and call
heap_prepare_freeze_tuple(), which would error out with one of
/*
* Process xmin, while keeping track of whether it's already frozen, or
* will become frozen iff our freeze plan is executed by caller (could be
* neither).
*/
xid = HeapTupleHeaderGetXmin(tuple);
if (!TransactionIdIsNormal(xid))
xmin_already_frozen = true;
else
{
if (TransactionIdPrecedes(xid, cutoffs->relfrozenxid))
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("found xmin %u from before relfrozenxid %u",
xid, cutoffs->relfrozenxid)));
or
if (TransactionIdPrecedes(update_xact, cutoffs->relfrozenxid))
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("multixact %u contains update XID %u from before relfrozenxid %u",
multi, update_xact,
cutoffs->relfrozenxid)));
or
/* Raw xmax is normal XID */
if (TransactionIdPrecedes(xid, cutoffs->relfrozenxid))
ereport(ERROR,
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg_internal("found xmax %u from before relfrozenxid %u",
xid, cutoffs->relfrozenxid)));
I'm not saying that spuriously erroring out would be ok. But I guess I just
don't understand the data corruption theory in this subthread, because we'd
error out if we encountered a tuple that should have been frozen but wasn't?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-26 20:46 Melanie Plageman <melanieplageman@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 43+ messages in thread
From: Melanie Plageman @ 2024-04-26 20:46 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Peter Geoghegan <pg@bowt.ie>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Mon, Apr 15, 2024 at 1:39 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> I've tried a couple times to catch up with this thread. But always kinda felt
> I must be missing something. It might be that this is one part of the
> confusion:
>
> On 2024-01-06 12:24:13 -0800, Noah Misch wrote:
> > Fair enough. While I agree there's a decent chance back-patching would be
> > okay, I think there's also a decent chance that 1ccc1e05ae creates the problem
> > Matthias theorized. Something like: we update relfrozenxid based on
> > OldestXmin, even though GlobalVisState caused us to retain a tuple older than
> > OldestXmin. Then relfrozenxid disagrees with table contents.
>
> Looking at the state as of 1ccc1e05ae, I don't see how - in lazy_scan_prune(),
> if heap_page_prune() spuriously didn't prune a tuple, because the horizon went
> backwards, we'd encounter the tuple in the loop below and call
> heap_prepare_freeze_tuple(), which would error out with one of
>
> /*
> * Process xmin, while keeping track of whether it's already frozen, or
> * will become frozen iff our freeze plan is executed by caller (could be
> * neither).
> */
> xid = HeapTupleHeaderGetXmin(tuple);
> if (!TransactionIdIsNormal(xid))
> xmin_already_frozen = true;
> else
> {
> if (TransactionIdPrecedes(xid, cutoffs->relfrozenxid))
> ereport(ERROR,
> (errcode(ERRCODE_DATA_CORRUPTED),
> errmsg_internal("found xmin %u from before relfrozenxid %u",
> xid, cutoffs->relfrozenxid)));
>
> or
> if (TransactionIdPrecedes(update_xact, cutoffs->relfrozenxid))
> ereport(ERROR,
> (errcode(ERRCODE_DATA_CORRUPTED),
> errmsg_internal("multixact %u contains update XID %u from before relfrozenxid %u",
> multi, update_xact,
> cutoffs->relfrozenxid)));
> or
> /* Raw xmax is normal XID */
> if (TransactionIdPrecedes(xid, cutoffs->relfrozenxid))
> ereport(ERROR,
> (errcode(ERRCODE_DATA_CORRUPTED),
> errmsg_internal("found xmax %u from before relfrozenxid %u",
> xid, cutoffs->relfrozenxid)));
>
>
> I'm not saying that spuriously erroring out would be ok. But I guess I just
> don't understand the data corruption theory in this subthread, because we'd
> error out if we encountered a tuple that should have been frozen but wasn't?
I have a more basic question. How could GlobalVisState->maybe_needed
going backwards cause a problem with relfrozenxid? Yes, if
maybe_needed goes backwards, we may not remove a tuple whose xmin/xmax
are older than VacuumCutoffs->OldestXmin. But, if that tuple's
xmin/xmax are older than OldestXmin, then wouldn't we freeze it? If we
freeze it, there isn't an issue. And if the tuple's xids are not newer
than OldestXmin, then how could we end up advancing relfrozenxid to a
value greater than the tuple's xids?
- Melanie
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-26 21:28 Peter Geoghegan <pg@bowt.ie>
parent: Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-04-26 21:28 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Apr 26, 2024 at 4:46 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> I have a more basic question. How could GlobalVisState->maybe_needed
> going backwards cause a problem with relfrozenxid? Yes, if
> maybe_needed goes backwards, we may not remove a tuple whose xmin/xmax
> are older than VacuumCutoffs->OldestXmin. But, if that tuple's
> xmin/xmax are older than OldestXmin, then wouldn't we freeze it?
You can't freeze every XID older than OldestXmin.
heap_prepare_freeze_tuple() isn't prepared for HEAPTUPLE_DEAD tuples,
and expects that those will be taken care of by the time it is called.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-26 21:56 Melanie Plageman <melanieplageman@gmail.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Melanie Plageman @ 2024-04-26 21:56 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Apr 26, 2024 at 5:28 PM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Fri, Apr 26, 2024 at 4:46 PM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> > I have a more basic question. How could GlobalVisState->maybe_needed
> > going backwards cause a problem with relfrozenxid? Yes, if
> > maybe_needed goes backwards, we may not remove a tuple whose xmin/xmax
> > are older than VacuumCutoffs->OldestXmin. But, if that tuple's
> > xmin/xmax are older than OldestXmin, then wouldn't we freeze it?
>
> You can't freeze every XID older than OldestXmin.
> heap_prepare_freeze_tuple() isn't prepared for HEAPTUPLE_DEAD tuples,
> and expects that those will be taken care of by the time it is called.
But, the tuple isn't HEAPTUPLE_DEAD -- it's HEAPTUPLE_RECENTLY_DEAD.
It will always be HEAPTUPLE_RECENTLY_DEAD in 17 and in <= 16, if
HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, we wouldn't call
heap_prepare_freeze_tuple() because of the retry loop.
- Melanie
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-26 22:20 Peter Geoghegan <pg@bowt.ie>
parent: Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-04-26 22:20 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Apr 26, 2024 at 5:56 PM Melanie Plageman
<melanieplageman@gmail.com> wrote:
>
> On Fri, Apr 26, 2024 at 5:28 PM Peter Geoghegan <pg@bowt.ie> wrote:
> >
> > On Fri, Apr 26, 2024 at 4:46 PM Melanie Plageman
> > <melanieplageman@gmail.com> wrote:
> > > I have a more basic question. How could GlobalVisState->maybe_needed
> > > going backwards cause a problem with relfrozenxid? Yes, if
> > > maybe_needed goes backwards, we may not remove a tuple whose xmin/xmax
> > > are older than VacuumCutoffs->OldestXmin. But, if that tuple's
> > > xmin/xmax are older than OldestXmin, then wouldn't we freeze it?
> >
> > You can't freeze every XID older than OldestXmin.
> > heap_prepare_freeze_tuple() isn't prepared for HEAPTUPLE_DEAD tuples,
> > and expects that those will be taken care of by the time it is called.
>
> But, the tuple isn't HEAPTUPLE_DEAD -- it's HEAPTUPLE_RECENTLY_DEAD.
Why? What tuple is this?
> It will always be HEAPTUPLE_RECENTLY_DEAD in 17 and in <= 16, if
> HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, we wouldn't call
> heap_prepare_freeze_tuple() because of the retry loop.
The retry loop exists precisely because heap_prepare_freeze_tuple()
isn't prepared to deal with HEAPTUPLE_DEAD tuples. So I agree that
that won't be allowed to happen on versions that have the retry loop
(14 - 16).
As Andres pointed out, even if we were to call
heap_prepare_freeze_tuple() with a HEAPTUPLE_DEAD tuple, we'd get a
"can't happen" error (though it's hard to see this because it doesn't
actually rely on the hint bits set in the tuple).
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-04-27 14:37 Melanie Plageman <melanieplageman@gmail.com>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Melanie Plageman @ 2024-04-27 14:37 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Apr 26, 2024 at 6:21 PM Peter Geoghegan <pg@bowt.ie> wrote:
>
> On Fri, Apr 26, 2024 at 5:56 PM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> >
> > On Fri, Apr 26, 2024 at 5:28 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > >
> > > On Fri, Apr 26, 2024 at 4:46 PM Melanie Plageman
> > > <melanieplageman@gmail.com> wrote:
> > > > I have a more basic question. How could GlobalVisState->maybe_needed
> > > > going backwards cause a problem with relfrozenxid? Yes, if
> > > > maybe_needed goes backwards, we may not remove a tuple whose xmin/xmax
> > > > are older than VacuumCutoffs->OldestXmin. But, if that tuple's
> > > > xmin/xmax are older than OldestXmin, then wouldn't we freeze it?
> > >
> > > You can't freeze every XID older than OldestXmin.
> > > heap_prepare_freeze_tuple() isn't prepared for HEAPTUPLE_DEAD tuples,
> > > and expects that those will be taken care of by the time it is called.
> >
> > But, the tuple isn't HEAPTUPLE_DEAD -- it's HEAPTUPLE_RECENTLY_DEAD.
>
> Why? What tuple is this?
In 17, we don't ever get a new HTSV_Result, so if the tuple is not
removed, it would be because HeapTupleSatisfiesVacuumHorizon()
returned HEAPTUPLE_RECENTLY_DEAD and, if GlobalVisTestIsRemovableXid()
was called, dead_after did not precede GlobalVisState->maybe_needed.
This tuple, during this vacuum of the relation, would never be
determined to be HEAPTUPLE_DEAD or it would have been removed.
> > It will always be HEAPTUPLE_RECENTLY_DEAD in 17 and in <= 16, if
> > HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, we wouldn't call
> > heap_prepare_freeze_tuple() because of the retry loop.
>
> The retry loop exists precisely because heap_prepare_freeze_tuple()
> isn't prepared to deal with HEAPTUPLE_DEAD tuples. So I agree that
> that won't be allowed to happen on versions that have the retry loop
> (14 - 16).
So, it can't happen in back branches. Let's just address 17. Help me
understand how this can happen in 17.
We are talking about a tuple with xmin/xmax older than
VacuumCutoffs->OldestXmin in heap_prepare_freeze_tuple(). So,
freeze_xmin is true:
freeze_xmin = TransactionIdPrecedes(xid, cutoffs->OldestXmin);
Then, in heap_tuple_should_freeze(), we may ratchet back
NoFreezePageRelfrozenXid:
if (TransactionIdPrecedes(xid, *NoFreezePageRelfrozenXid))
*NoFreezePageRelfrozenXid = xid;
NoFreezePageRelfrozenXid was initialized with
VacuumCutoffs->OldestXmin and our tuple xmin/xmax is older than
VacuumCutoffs->OldestXmin.
We make a freeze plan for the tuple and move on. Assuming
HeapPageFreeze->freeze_required is never set to true and we don't end
up meeting the other criteria for opportunistic freezing, we may
decide not to freeze any tuples on the page (including this tuple). In
this case, we set relfrozenxid to NoFreezePageRelfrozenXid. This
should not be a value newer than our tuple's xids.
> As Andres pointed out, even if we were to call
> heap_prepare_freeze_tuple() with a HEAPTUPLE_DEAD tuple, we'd get a
> "can't happen" error (though it's hard to see this because it doesn't
> actually rely on the hint bits set in the tuple).
I just don't see how in 17 we could end up calling
heap_prepare_freeze_tuple() on a HEAPTUPLE_DEAD tuple. If
heap_prune_satisfies_vacuum() returned HEAPTUPLE_DEAD, we wouldn't
heap_prepare_freeze_tuple() the tuple.
I assume you are talking about a HEAPTUPLE_RECENTLY_DEAD tuple that
*should* be HEAPTUPLE_DEAD. But it doesn't really matter if it should
be HEAPTUPLE_DEAD and we fail to remove it, as long as we don't leave
it unfrozen and advance relfrozenxid past its xids. And I don't see
how we would do that.
- Melanie
- Melanie
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-05-02 16:52 Peter Geoghegan <pg@bowt.ie>
parent: Melanie Plageman <melanieplageman@gmail.com>
0 siblings, 1 reply; 43+ messages in thread
From: Peter Geoghegan @ 2024-05-02 16:52 UTC (permalink / raw)
To: Melanie Plageman <melanieplageman@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Sat, Apr 27, 2024 at 10:38 AM Melanie Plageman
<melanieplageman@gmail.com> wrote:
> In 17, we don't ever get a new HTSV_Result, so if the tuple is not
> removed, it would be because HeapTupleSatisfiesVacuumHorizon()
> returned HEAPTUPLE_RECENTLY_DEAD and, if GlobalVisTestIsRemovableXid()
> was called, dead_after did not precede GlobalVisState->maybe_needed.
> This tuple, during this vacuum of the relation, would never be
> determined to be HEAPTUPLE_DEAD or it would have been removed.
That makes sense.
> > > It will always be HEAPTUPLE_RECENTLY_DEAD in 17 and in <= 16, if
> > > HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, we wouldn't call
> > > heap_prepare_freeze_tuple() because of the retry loop.
> >
> > The retry loop exists precisely because heap_prepare_freeze_tuple()
> > isn't prepared to deal with HEAPTUPLE_DEAD tuples. So I agree that
> > that won't be allowed to happen on versions that have the retry loop
> > (14 - 16).
>
> So, it can't happen in back branches. Let's just address 17. Help me
> understand how this can happen in 17.
Just to be clear, I never said that it was possible in 17. If I
somehow implied it, then I didn't mean to.
> > As Andres pointed out, even if we were to call
> > heap_prepare_freeze_tuple() with a HEAPTUPLE_DEAD tuple, we'd get a
> > "can't happen" error (though it's hard to see this because it doesn't
> > actually rely on the hint bits set in the tuple).
>
> I just don't see how in 17 we could end up calling
> heap_prepare_freeze_tuple() on a HEAPTUPLE_DEAD tuple. If
> heap_prune_satisfies_vacuum() returned HEAPTUPLE_DEAD, we wouldn't
> heap_prepare_freeze_tuple() the tuple.
That part seems relatively straightforward.
> I assume you are talking about a HEAPTUPLE_RECENTLY_DEAD tuple that
> *should* be HEAPTUPLE_DEAD. But it doesn't really matter if it should
> be HEAPTUPLE_DEAD and we fail to remove it, as long as we don't leave
> it unfrozen and advance relfrozenxid past its xids.
I'm not sure that I agree with this part. You're describing a scheme
under which aggressive VACUUM isn't strictly guaranteed to respect
FreezeLimit. And so (for example) a VACUUM FREEZE wouldn't advance
relfrozenxid right up to OldestXmin/FreezeLimit. Right?
It certainly seems possible that, all things considered, you're right
-- perhaps it literally doesn't matter at all on 17, at least in
practice. I cannot think of any problem that's worse than an assertion
failure within heap_vacuum_rel, just before we go to advance the rel's
pg_class.relfrozenxid (and even that seems extremely unlikely). Nobody
particularly intended for things to work this way (the FreezeLimit
invariant isn't supposed to be violated), but it'd hardly be the first
time that something like this happened. Far from it.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-05-02 18:01 Alena Rybakina <lena.ribackina@yandex.ru>
parent: Peter Geoghegan <pg@bowt.ie>
0 siblings, 1 reply; 43+ messages in thread
From: Alena Rybakina @ 2024-05-02 18:01 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; Melanie Plageman <melanieplageman@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 02.05.2024 19:52, Peter Geoghegan wrote:
> On Sat, Apr 27, 2024 at 10:38 AM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
>> In 17, we don't ever get a new HTSV_Result, so if the tuple is not
>> removed, it would be because HeapTupleSatisfiesVacuumHorizon()
>> returned HEAPTUPLE_RECENTLY_DEAD and, if GlobalVisTestIsRemovableXid()
>> was called, dead_after did not precede GlobalVisState->maybe_needed.
>> This tuple, during this vacuum of the relation, would never be
>> determined to be HEAPTUPLE_DEAD or it would have been removed.
> That makes sense.
>
>>>> It will always be HEAPTUPLE_RECENTLY_DEAD in 17 and in <= 16, if
>>>> HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, we wouldn't call
>>>> heap_prepare_freeze_tuple() because of the retry loop.
>>> The retry loop exists precisely because heap_prepare_freeze_tuple()
>>> isn't prepared to deal with HEAPTUPLE_DEAD tuples. So I agree that
>>> that won't be allowed to happen on versions that have the retry loop
>>> (14 - 16).
>> So, it can't happen in back branches. Let's just address 17. Help me
>> understand how this can happen in 17.
> Just to be clear, I never said that it was possible in 17. If I
> somehow implied it, then I didn't mean to.
>
Hi! I also investigated this issue and reproduced it using this test
added to the isolated tests, where I added 2 tuples, deleted them and
ran vacuum and printed the tuple_deleted and dead_tuples statistics (I
attached test c to this email as a patch). Within400iterationsormore,
Igotthe results:
n_dead_tup|n_live_tup|n_tup_del
----------------+------------+------------- 0| 0| 0 (1 row)
After 400 or more running cycles, I felt the differences, as shown earlier:
n_dead_tup|n_live_tup|n_tup_del
----------+----------+---------
- 0| 0| 0
+ 2| 0| 0
(1 row)
I debugged and found that the test produces results with 0 dead tuples
if GlobalVisTempRels.maybe_needed is less than the x_max of the tuple.
In the code, this condition works in heap_prune_satisfies_vacuum:
else if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after))
{
res = HEAPTUPLE_DEAD;
}
But when GlobalVisTempRels.maybe_needed is equal to the x_max xid of the
tuple, vacuum does not touch this tuple, because the
heap_prune_satisfies_vacuum function returns the status of the
RECENTLY_DEAD tuple.
Unfortunately, I have not found any explanation why
GlobalVisTempRels.maybe_needed does not change after 400 iterations or
more. I'm still studying it. Perhaps this information will help you.
I reproduced the problem on REL_16_STABLE.
--
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
Attachments:
0001-vacuum_test.patch (text/x-patch, 5.6K) download
diff --git a/src/test/isolation/expected/vacuum_test.out b/src/test/isolation/expected/vacuum_test.out
new file mode 100644
index 00000000000..38ef1e68b5d
--- /dev/null
+++ b/src/test/isolation/expected/vacuum_test.out
@@ -0,0 +1,23 @@
+unused step name: s1_activity_print
+unused step name: s1_delete_half_table
+unused step name: s1_set_agressive_vacuum
+unused step name: s1_update
+unused step name: s1_vacuum
+unused step name: s1_vacuum_full
+Parsed test spec with 1 sessions
+
+starting permutation: s1_insert s1_delete_full_table s1_vacuum_parallel s1_print_vacuum_stats_tables
+step s1_insert: INSERT INTO vestat(x) SELECT id FROM generate_series(1,2) As id;
+step s1_delete_full_table: DELETE FROM vestat;
+step s1_vacuum_parallel: VACUUM (PARALLEL 2, INDEX_CLEANUP ON) vestat;
+step s1_print_vacuum_stats_tables:
+ SELECT n_dead_tup, n_live_tup, n_tup_del
+ FROM pg_stat_all_tables vt, pg_class c
+ WHERE vt.relname = 'vestat' AND
+ vt.relid = c.oid;
+
+n_dead_tup|n_live_tup|n_tup_del
+----------+----------+---------
+ 0| 0| 0
+(1 row)
+
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 0342eb39e40..edb751c2ff4 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -1,116 +1 @@
-test: read-only-anomaly
-test: read-only-anomaly-2
-test: read-only-anomaly-3
-test: read-write-unique
-test: read-write-unique-2
-test: read-write-unique-3
-test: read-write-unique-4
-test: simple-write-skew
-test: receipt-report
-test: temporal-range-integrity
-test: project-manager
-test: classroom-scheduling
-test: total-cash
-test: referential-integrity
-test: ri-trigger
-test: partial-index
-test: two-ids
-test: multiple-row-versions
-test: index-only-scan
-test: predicate-lock-hot-tuple
-test: update-conflict-out
-test: deadlock-simple
-test: deadlock-hard
-test: deadlock-soft
-test: deadlock-soft-2
-test: deadlock-parallel
-test: detach-partition-concurrently-1
-test: detach-partition-concurrently-2
-test: detach-partition-concurrently-3
-test: detach-partition-concurrently-4
-test: fk-contention
-test: fk-deadlock
-test: fk-deadlock2
-test: fk-partitioned-1
-test: fk-partitioned-2
-test: fk-snapshot
-test: subxid-overflow
-test: eval-plan-qual
-test: eval-plan-qual-trigger
-test: lock-update-delete
-test: lock-update-traversal
-test: inherit-temp
-test: temp-schema-cleanup
-test: insert-conflict-do-nothing
-test: insert-conflict-do-nothing-2
-test: insert-conflict-do-update
-test: insert-conflict-do-update-2
-test: insert-conflict-do-update-3
-test: insert-conflict-specconflict
-test: merge-insert-update
-test: merge-delete
-test: merge-update
-test: merge-match-recheck
-test: merge-join
-test: delete-abort-savept
-test: delete-abort-savept-2
-test: aborted-keyrevoke
-test: multixact-no-deadlock
-test: multixact-no-forget
-test: lock-committed-update
-test: lock-committed-keyupdate
-test: update-locked-tuple
-test: reindex-concurrently
-test: reindex-concurrently-toast
-test: reindex-schema
-test: propagate-lock-delete
-test: tuplelock-conflict
-test: tuplelock-update
-test: tuplelock-upgrade-no-deadlock
-test: tuplelock-partition
-test: freeze-the-dead
-test: nowait
-test: nowait-2
-test: nowait-3
-test: nowait-4
-test: nowait-5
-test: skip-locked
-test: skip-locked-2
-test: skip-locked-3
-test: skip-locked-4
-test: drop-index-concurrently-1
-test: multiple-cic
-test: alter-table-1
-test: alter-table-2
-test: alter-table-3
-test: alter-table-4
-test: create-trigger
-test: sequence-ddl
-test: async-notify
-test: vacuum-no-cleanup-lock
-test: timeouts
-test: vacuum-concurrent-drop
-test: vacuum-conflict
-test: vacuum-skip-locked
-test: stats
-test: horizons
-test: predicate-hash
-test: predicate-gist
-test: predicate-gin
-test: partition-concurrent-attach
-test: partition-drop-index-locking
-test: partition-key-update-1
-test: partition-key-update-2
-test: partition-key-update-3
-test: partition-key-update-4
-test: partition-merge
-test: partition-split
-test: plpgsql-toast
-test: cluster-conflict
-test: cluster-conflict-partition
-test: truncate-conflict
-test: serializable-parallel
-test: serializable-parallel-2
-test: serializable-parallel-3
-test: matview-write-skew
-test: lock-nowait
+test: vacuum_test
\ No newline at end of file
diff --git a/src/test/isolation/specs/vacuum_test.spec b/src/test/isolation/specs/vacuum_test.spec
new file mode 100644
index 00000000000..1fb4d67cc34
--- /dev/null
+++ b/src/test/isolation/specs/vacuum_test.spec
@@ -0,0 +1,34 @@
+setup
+{
+ CREATE TABLE vestat (x int primary key) WITH (autovacuum_enabled = off);
+
+}
+
+teardown
+{
+ DROP TABLE vestat CASCADE;
+}
+
+session s1
+step s1_set_agressive_vacuum { SET vacuum_freeze_min_age = 0; }
+step s1_insert { INSERT INTO vestat(x) SELECT id FROM generate_series(1,2) As id; }
+step s1_update { UPDATE vestat SET x = x+1; }
+step s1_delete_half_table { DELETE FROM vestat WHERE x % 2 = 0; }
+step s1_delete_full_table { DELETE FROM vestat; }
+step s1_vacuum { VACUUM vestat; }
+step s1_vacuum_full { VACUUM FULL vestat; }
+step s1_vacuum_parallel { VACUUM (PARALLEL 2, INDEX_CLEANUP ON) vestat; }
+step s1_activity_print { select application_name, backend_type from pg_stat_activity order by (application_name, backend_type); }
+step s1_print_vacuum_stats_tables
+{
+ SELECT n_dead_tup, n_live_tup, n_tup_del
+ FROM pg_stat_all_tables vt, pg_class c
+ WHERE vt.relname = 'vestat' AND
+ vt.relid = c.oid;
+}
+
+permutation
+ s1_insert
+ s1_delete_full_table
+ s1_vacuum_parallel
+ s1_print_vacuum_stats_tables
\ No newline at end of file
--
2.34.1
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-05-02 18:36 Alena Rybakina <lena.ribackina@yandex.ru>
parent: Alena Rybakina <lena.ribackina@yandex.ru>
0 siblings, 1 reply; 43+ messages in thread
From: Alena Rybakina @ 2024-05-02 18:36 UTC (permalink / raw)
To: Peter Geoghegan <pg@bowt.ie>; Melanie Plageman <melanieplageman@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Matthias van de Meent <boekewurm+postgres@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 02.05.2024 21:01, Alena Rybakina wrote:
> On 02.05.2024 19:52, Peter Geoghegan wrote:
>> On Sat, Apr 27, 2024 at 10:38 AM Melanie Plageman
>> <melanieplageman@gmail.com> wrote:
>>> In 17, we don't ever get a new HTSV_Result, so if the tuple is not
>>> removed, it would be because HeapTupleSatisfiesVacuumHorizon()
>>> returned HEAPTUPLE_RECENTLY_DEAD and, if GlobalVisTestIsRemovableXid()
>>> was called, dead_after did not precede GlobalVisState->maybe_needed.
>>> This tuple, during this vacuum of the relation, would never be
>>> determined to be HEAPTUPLE_DEAD or it would have been removed.
>> That makes sense.
>>
>>>>> It will always be HEAPTUPLE_RECENTLY_DEAD in 17 and in <= 16, if
>>>>> HeapTupleSatisfiesVacuum() returns HEAPTUPLE_DEAD, we wouldn't call
>>>>> heap_prepare_freeze_tuple() because of the retry loop.
>>>> The retry loop exists precisely because heap_prepare_freeze_tuple()
>>>> isn't prepared to deal with HEAPTUPLE_DEAD tuples. So I agree that
>>>> that won't be allowed to happen on versions that have the retry loop
>>>> (14 - 16).
>>> So, it can't happen in back branches. Let's just address 17. Help me
>>> understand how this can happen in 17.
>> Just to be clear, I never said that it was possible in 17. If I
>> somehow implied it, then I didn't mean to.
>>
> Hi! I also investigated this issue and reproduced it using this test
> added to the isolated tests, where I added 2 tuples, deleted them and
> ran vacuum and printed the tuple_deleted and dead_tuples statistics (I
> attached test c to this email as a patch). Within400iterationsormore,
> Igotthe results:
>
> n_dead_tup|n_live_tup|n_tup_del
> ----------------+------------+------------- 0| 0| 0 (1 row)
>
> After 400 or more running cycles, I felt the differences, as shown
> earlier:
>
> n_dead_tup|n_live_tup|n_tup_del
> ----------+----------+---------
> - 0| 0| 0
> + 2| 0| 0
> (1 row)
>
>
> I debugged and found that the test produces results with 0 dead tuples
> if GlobalVisTempRels.maybe_needed is less than the x_max of the tuple.
> In the code, this condition works in heap_prune_satisfies_vacuum:
>
> else if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after))
> {
> res = HEAPTUPLE_DEAD;
> }
>
> But when GlobalVisTempRels.maybe_needed is equal to the x_max xid of
> the tuple, vacuum does not touch this tuple, because the
> heap_prune_satisfies_vacuum function returns the status of the
> RECENTLY_DEAD tuple.
>
> Unfortunately, I have not found any explanation why
> GlobalVisTempRels.maybe_needed does not change after 400 iterations or
> more. I'm still studying it. Perhaps this information will help you.
>
> I reproduced the problem on REL_16_STABLE.
>
I reproduced this test in the master branch as well, but used a more
complex test for it: I added 700 tuples to the table, deleted half of
the table, and then started vacuum. I expected to get only 350 live
tuples and 0 dead and deleted tuples, but after 800 iterations I got 350
dead tuples and 350 live tuples: n_dead_tup|n_live_tup|n_tup_del
---------------+-------------+-------------
- 0| 350| 0
+ 350| 350| 0
(1 row)
I have added other steps in the test, but so far I have not seen any
falls there or have not reached them.
Just in case, I ran the test with this bash command:
for i in `seq 2000`;do echo "ITER $i"; make -s installcheck -C
src/test/isolation/ || break;done
--
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
Attachments:
vacuum_test.spec (text/x-rpm-spec, 1.3K) download
setup
{
CREATE TABLE vestat (x int primary key) WITH (autovacuum_enabled = off);
}
teardown
{
DROP TABLE vestat CASCADE;
}
session s1
step s1_set_agressive_vacuum { SET vacuum_freeze_min_age = 0; }
step s1_insert { INSERT INTO vestat(x) SELECT id FROM generate_series(1,700) As id; }
step s1_update { UPDATE vestat SET x = x+1; }
step s1_delete_half_table { DELETE FROM vestat WHERE x % 2 = 0; }
step s1_delete_full_table { DELETE FROM vestat; }
step s1_vacuum { VACUUM vestat; }
step s1_vacuum_full { VACUUM FULL vestat; }
step s1_vacuum_parallel { VACUUM (PARALLEL 2, INDEX_CLEANUP ON) vestat; }
step s1_activity_print { select application_name, backend_type from pg_stat_activity order by (application_name, backend_type); }
step s1_print_vacuum_stats_tables
{
SELECT n_dead_tup, n_live_tup, n_tup_del
FROM pg_stat_all_tables vt, pg_class c
WHERE vt.relname = 'vestat' AND
vt.relid = c.oid;
}
permutation
s1_insert
s1_delete_half_table
s1_vacuum
s1_print_vacuum_stats_tables
s1_delete_full_table
s1_vacuum_parallel
s1_print_vacuum_stats_tables
s1_insert
s1_update
s1_vacuum
s1_print_vacuum_stats_tables
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-05-03 09:35 Matthias van de Meent <boekewurm+postgres@gmail.com>
parent: Alena Rybakina <lena.ribackina@yandex.ru>
0 siblings, 1 reply; 43+ messages in thread
From: Matthias van de Meent @ 2024-05-03 09:35 UTC (permalink / raw)
To: Alena Rybakina <lena.ribackina@yandex.ru>; +Cc: Peter Geoghegan <pg@bowt.ie>; Melanie Plageman <melanieplageman@gmail.com>; Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, 2 May 2024 at 20:36, Alena Rybakina <lena.ribackina@yandex.ru> wrote:
>
> Hi! I also investigated this issue and reproduced it using this test added to the isolated tests, where I added 2 tuples, deleted them and ran vacuum and printed the tuple_deleted and dead_tuples statistics (I attached test c to this email as a patch). Within 400 iterations or more, I got the results:
>
> n_dead_tup|n_live_tup|n_tup_del ----------------+------------+------------- 0| 0| 0 (1 row)
>
> After 400 or more running cycles, I felt the differences, as shown earlier:
>
> n_dead_tup|n_live_tup|n_tup_del
> ----------+----------+---------
> - 0| 0| 0
> + 2| 0| 0
> (1 row)
>
>
> I debugged and found that the test produces results with 0 dead tuples if GlobalVisTempRels.maybe_needed is less than the x_max of the tuple. In the code, this condition works in heap_prune_satisfies_vacuum:
>
> else if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after))
> {
> res = HEAPTUPLE_DEAD;
> }
>
> But when GlobalVisTempRels.maybe_needed is equal to the x_max xid of the tuple, vacuum does not touch this tuple, because the heap_prune_satisfies_vacuum function returns the status of the RECENTLY_DEAD tuple.
>
> Unfortunately, I have not found any explanation why GlobalVisTempRels.maybe_needed does not change after 400 iterations or more. I'm still studying it. Perhaps this information will help you.
You should probably check GlobalVisTestShouldUpdate to see why it
wouldn't get updated. IIRC, GobalVis* are only updated every so often
based on a local copy of a shared variable, to reduce contention on
the shared state in the proc arrays. It is quite likely that your
backend was waiting for new other changes, when the current
GlobalVisTempRels state was already "good enough" for the current
visibility snapshot.
> I reproduced the problem on REL_16_STABLE.
>
> I reproduced this test in the master branch as well, but used a more complex test for it: I added 700 tuples to the table, deleted half of the table, and then started vacuum. I expected to get only 350 live tuples and 0 dead and deleted tuples, but after 800 iterations I got 350 dead tuples and 350 live tuples: n_dead_tup|n_live_tup|n_tup_del
> ---------------+-------------
>
> +-------------
> - 0| 350| 0
> + 350| 350| 0
> (1 row)
I think this output is expected. Concurrent tasks (such as autovacuum
and autoanalyze) can have their own visibility snapshots and
transactions, which will block VACUUM from removing rows that may be
visible to those snapshots. Did you verify there was no other
(background) process active in the PostgreSQL test node during your
tests?
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: BUG #17257: (auto)vacuum hangs within lazy_scan_prune()
@ 2024-05-04 11:53 Alena Rybakina <lena.ribackina@yandex.ru>
parent: Matthias van de Meent <boekewurm+postgres@gmail.com>
0 siblings, 0 replies; 43+ messages in thread
From: Alena Rybakina @ 2024-05-04 11:53 UTC (permalink / raw)
To: Matthias van de Meent <boekewurm+postgres@gmail.com>; +Cc: Peter Geoghegan <pg@bowt.ie>; Melanie Plageman <melanieplageman@gmail.com>; Andres Freund <andres@anarazel.de>; Noah Misch <noah@leadboat.com>; Robert Haas <robertmhaas@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi!
On 03.05.2024 12:35, Matthias van de Meent wrote:
> On Thu, 2 May 2024 at 20:36, Alena Rybakina<lena.ribackina@yandex.ru> wrote:
>> Hi! I also investigated this issue and reproduced it using this test added to the isolated tests, where I added 2 tuples, deleted them and ran vacuum and printed the tuple_deleted and dead_tuples statistics (I attached test c to this email as a patch). Within 400 iterations or more, I got the results:
>>
>> n_dead_tup|n_live_tup|n_tup_del ----------------+------------+------------- 0| 0| 0 (1 row)
>>
>> After 400 or more running cycles, I felt the differences, as shown earlier:
>>
>> n_dead_tup|n_live_tup|n_tup_del
>> ----------+----------+---------
>> - 0| 0| 0
>> + 2| 0| 0
>> (1 row)
>>
>>
>> I debugged and found that the test produces results with 0 dead tuples if GlobalVisTempRels.maybe_needed is less than the x_max of the tuple. In the code, this condition works in heap_prune_satisfies_vacuum:
>>
>> else if (GlobalVisTestIsRemovableXid(prstate->vistest, dead_after))
>> {
>> res = HEAPTUPLE_DEAD;
>> }
>>
>> But when GlobalVisTempRels.maybe_needed is equal to the x_max xid of the tuple, vacuum does not touch this tuple, because the heap_prune_satisfies_vacuum function returns the status of the RECENTLY_DEAD tuple.
>>
>> Unfortunately, I have not found any explanation why GlobalVisTempRels.maybe_needed does not change after 400 iterations or more. I'm still studying it. Perhaps this information will help you.
> You should probably check GlobalVisTestShouldUpdate to see why it
> wouldn't get updated. IIRC, GobalVis* are only updated every so often
> based on a local copy of a shared variable, to reduce contention on
> the shared state in the proc arrays. It is quite likely that your
> backend was waiting for new other changes, when the current
> GlobalVisTempRels state was already "good enough" for the current
> visibility snapshot.
Thank you for your advice, I am checking this now. It is a new area for
me and it needs from me some more investigation.
>> I reproduced the problem on REL_16_STABLE.
>>
>> I reproduced this test in the master branch as well, but used a more complex test for it: I added 700 tuples to the table, deleted half of the table, and then started vacuum. I expected to get only 350 live tuples and 0 dead and deleted tuples, but after 800 iterations I got 350 dead tuples and 350 live tuples: n_dead_tup|n_live_tup|n_tup_del
>> ---------------+-------------
>>
>> +-------------
>> - 0| 350| 0
>> + 350| 350| 0
>> (1 row)
> I think this output is expected. Concurrent tasks (such as autovacuum
> and autoanalyze) can have their own visibility snapshots and
> transactions, which will block VACUUM from removing rows that may be
> visible to those snapshots. Did you verify there was no other
> (background) process active in the PostgreSQL test node during your
> tests?
>
I see backgrounds processes like autovacuum launcher, client backend,
logical replication launcher, checkpointer, walwriter, andbackground
writer. I checked background backends throw pg_stat_activity view and I
didn't find a special backend, which impacted on difference in the test.
Maybe I missed something?
Besides, I had a hypothesis that some backends might be so many or we
have some special backend, which interferes with doing of updating the
snapshot, so because of this, some transaction couldn't finish on time
and we got finally RECENTLY_DEAD tuple, but I didn't find any evidence.
To prove it I added the query with output all backends before vacuum and
collected its results, when the test was passed and only when the test
failed I differed last diff with others to find something new, but it
was usual. Even when the test shows the expected results (0 dead_tuples
and 0 deleted tuples) I saw the same backends, maybe some of them were
more, but I didn't find any unusual.
Maybe I should pay attention to something special?
--
Regards,
Alena Rybakina
Postgres Professional:http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 43+ messages in thread
end of thread, other threads:[~2024-05-04 11:53 UTC | newest]
Thread overview: 43+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 13:00 BUG #17257: (auto)vacuum hangs within lazy_scan_prune() PG Bug reporting form <noreply@postgresql.org>
2021-10-29 13:30 ` Alexander Lakhin <exclusion@gmail.com>
2021-10-29 18:17 ` Peter Geoghegan <pg@bowt.ie>
2021-11-01 15:15 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2021-11-03 15:45 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2021-11-03 16:21 ` Peter Geoghegan <pg@bowt.ie>
2021-11-05 11:43 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2021-11-05 21:25 ` Peter Geoghegan <pg@bowt.ie>
2021-11-06 01:20 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2021-11-06 01:59 ` Andres Freund <andres@anarazel.de>
2021-11-10 20:28 ` Peter Geoghegan <pg@bowt.ie>
2023-12-25 02:44 ` Noah Misch <noah@leadboat.com>
2023-12-31 23:53 ` Peter Geoghegan <pg@bowt.ie>
2024-01-06 20:24 ` Noah Misch <noah@leadboat.com>
2024-01-06 21:30 ` Peter Geoghegan <pg@bowt.ie>
2024-01-06 21:41 ` Peter Geoghegan <pg@bowt.ie>
2024-01-06 22:44 ` Noah Misch <noah@leadboat.com>
2024-01-08 17:02 ` Peter Geoghegan <pg@bowt.ie>
2024-01-08 18:21 ` Noah Misch <noah@leadboat.com>
2024-01-08 18:36 ` Peter Geoghegan <pg@bowt.ie>
2024-01-09 20:59 ` Peter Geoghegan <pg@bowt.ie>
2024-01-09 21:44 ` Noah Misch <noah@leadboat.com>
2024-01-09 23:16 ` Peter Geoghegan <pg@bowt.ie>
2024-01-10 19:06 ` Peter Geoghegan <pg@bowt.ie>
2024-01-10 19:38 ` Noah Misch <noah@leadboat.com>
2024-01-10 19:57 ` Peter Geoghegan <pg@bowt.ie>
2024-01-10 21:15 ` Noah Misch <noah@leadboat.com>
2024-04-15 17:39 ` Andres Freund <andres@anarazel.de>
2024-04-26 20:46 ` Melanie Plageman <melanieplageman@gmail.com>
2024-04-26 21:28 ` Peter Geoghegan <pg@bowt.ie>
2024-04-26 21:56 ` Melanie Plageman <melanieplageman@gmail.com>
2024-04-26 22:20 ` Peter Geoghegan <pg@bowt.ie>
2024-04-27 14:37 ` Melanie Plageman <melanieplageman@gmail.com>
2024-05-02 16:52 ` Peter Geoghegan <pg@bowt.ie>
2024-05-02 18:01 ` Alena Rybakina <lena.ribackina@yandex.ru>
2024-05-02 18:36 ` Alena Rybakina <lena.ribackina@yandex.ru>
2024-05-03 09:35 ` Matthias van de Meent <boekewurm+postgres@gmail.com>
2024-05-04 11:53 ` Alena Rybakina <lena.ribackina@yandex.ru>
2024-04-05 20:20 ` Robert Haas <robertmhaas@gmail.com>
2024-04-07 10:00 ` Alexander Lakhin <exclusion@gmail.com>
2024-04-09 18:59 ` Robert Haas <robertmhaas@gmail.com>
2024-04-09 20:00 ` Alexander Lakhin <exclusion@gmail.com>
2024-04-15 16:17 ` Robert Haas <robertmhaas@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox