Author: Noah Misch Commit: Noah Misch diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index d00300c..787a995 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -49,6 +49,7 @@ #include "storage/predicate.h" #include "storage/procarray.h" #include "utils/datum.h" +#include "utils/injection_point.h" #include "utils/inval.h" #include "utils/spccache.h" @@ -3233,6 +3234,7 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup, interesting_attrs = bms_add_members(interesting_attrs, id_attrs); block = ItemPointerGetBlockNumber(otid); + INJECTION_POINT("heap_update-before-pin"); buffer = ReadBuffer(relation, block); page = BufferGetPage(buffer); @@ -6372,6 +6374,9 @@ heap_inplace_update_and_unlock(Relation relation, */ PreInplace_Inval(); + INJECTION_POINT_LOAD("SIInsertDataEntries2"); + INJECTION_POINT_LOAD("SIInsertDataEntries3"); + /*---------- * NO EREPORT(ERROR) from here till changes are complete * diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index ff81744..d837f41 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -25,6 +25,7 @@ #include "storage/shmem.h" #include "storage/sinvaladt.h" #include "storage/spin.h" +#include "utils/injection_point.h" /* * Conceptually, the shared cache invalidation messages are stored in an @@ -386,6 +387,17 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) int max; int i; + /* + * CacheInvalidateSmgr skips the injection point, since this + * facilitates a test that needs this only for delaying pg_class + * syscache invals and relcache invals. + */ + if (n != 1 || data->sm.id != SHAREDINVALSMGR_ID) + { + INJECTION_POINT("SIInsertDataEntries2"); + INJECTION_POINT("SIInsertDataEntries3"); + } + n -= nthistime; LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index 0753a9d..013c68e 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -13,7 +13,7 @@ PGFILEDESC = "injection_points - facility for injection points" REGRESS = injection_points reindex_conc REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress -ISOLATION = basic inplace +ISOLATION = basic syscache-update-pruned inplace TAP_TESTS = 1 diff --git a/src/test/modules/injection_points/expected/syscache-update-pruned.out b/src/test/modules/injection_points/expected/syscache-update-pruned.out new file mode 100644 index 0000000..9caba9a --- /dev/null +++ b/src/test/modules/injection_points/expected/syscache-update-pruned.out @@ -0,0 +1,133 @@ +Parsed test spec with 4 sessions + +starting permutation: cachefill1 at2 vac3 grant1 mkrels4 wakegrant4 wakeinval4 +mkrels +------ + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step cachefill1: SELECT pg_relation_filenode('vactest.orig50') > 0; +?column? +-------- +t +(1 row) + +step at2: ALTER TABLE vactest.orig50 FORCE ROW LEVEL SECURITY; +step vac3: VACUUM pg_class; +step grant1: GRANT SELECT ON vactest.orig50 TO PUBLIC; +step mkrels4: + SELECT vactest.mkrels('intruder', 1, 100); -- repopulate LP_UNUSED + +mkrels +------ + +(1 row) + +step wakegrant4: + SELECT injection_points_detach('heap_update-before-pin'); + SELECT injection_points_wakeup('heap_update-before-pin'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step grant1: <... completed> +ERROR: duplicate key value violates unique constraint "pg_class_oid_index" +step wakeinval4: + SELECT injection_points_detach('SIInsertDataEntries2'); + SELECT injection_points_detach('SIInsertDataEntries3'); + SELECT injection_points_wakeup('SIInsertDataEntries2'); + SELECT injection_points_wakeup('SIInsertDataEntries3'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +step at2: <... completed> +step vac3: <... completed> + +starting permutation: cachefill1 at2 vac3 grant1 wakegrant4 wakeinval4 +mkrels +------ + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +injection_points_attach +----------------------- + +(1 row) + +step cachefill1: SELECT pg_relation_filenode('vactest.orig50') > 0; +?column? +-------- +t +(1 row) + +step at2: ALTER TABLE vactest.orig50 FORCE ROW LEVEL SECURITY; +step vac3: VACUUM pg_class; +step grant1: GRANT SELECT ON vactest.orig50 TO PUBLIC; +step wakegrant4: + SELECT injection_points_detach('heap_update-before-pin'); + SELECT injection_points_wakeup('heap_update-before-pin'); + +injection_points_detach +----------------------- + +(1 row) + +injection_points_wakeup +----------------------- + +(1 row) + +PQconsumeInput failed: server closed the connection unexpectedly + This probably means the server terminated abnormally + before or while processing the request. + diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 58f1900..626039e 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -43,6 +43,7 @@ tests += { 'isolation': { 'specs': [ 'basic', + 'syscache-update-pruned', 'inplace', ], }, diff --git a/src/test/modules/injection_points/specs/syscache-update-pruned.spec b/src/test/modules/injection_points/specs/syscache-update-pruned.spec new file mode 100644 index 0000000..50dde9a --- /dev/null +++ b/src/test/modules/injection_points/specs/syscache-update-pruned.spec @@ -0,0 +1,107 @@ +# Test race conditions involving: +# - s1: heap_update($FROM_SYSCACHE), without a snapshot or pin +# - s2: ALTER TABLE making $FROM_SYSCACHE a dead tuple +# - s3: "VACUUM pg_class" making $FROM_SYSCACHE become LP_UNUSED + +# This is a derivative work of inplace.spec. When introduced, that spec +# reached basically the same race condition, for inplace updates. Commit +# a07e03f fixed the inplace update case. + +# Need s2 to make a non-HOT update. Otherwise, "VACUUM pg_class" would leave +# an LP_REDIRECT that persists. To get non-HOT, make rels so the pg_class row +# for vactest.orig50 is on a filled page (assuming BLCKSZ=8192). Just to save +# on filesystem syscalls, use relkind=c for every other rel. +setup +{ + CREATE EXTENSION injection_points; + CREATE SCHEMA vactest; + CREATE FUNCTION vactest.mkrels(text, int, int) RETURNS void + LANGUAGE plpgsql SET search_path = vactest AS $$ + DECLARE + tname text; + BEGIN + FOR i in $2 .. $3 LOOP + tname := $1 || i; + EXECUTE FORMAT('CREATE TYPE ' || tname || ' AS ()'); + RAISE DEBUG '% at %', tname, ctid + FROM pg_class WHERE oid = tname::regclass; + END LOOP; + END + $$; +} +setup { VACUUM FULL pg_class; -- reduce free space } +setup +{ + SELECT vactest.mkrels('orig', 1, 49); + CREATE TABLE vactest.orig50 (); + SELECT vactest.mkrels('orig', 51, 100); +} +teardown +{ + DROP SCHEMA vactest CASCADE; + DROP EXTENSION injection_points; +} + +# Wait during GRANT. +session s1 +setup { + SELECT injection_points_set_local(); + SELECT injection_points_attach('heap_update-before-pin', 'wait'); +} +step cachefill1 { SELECT pg_relation_filenode('vactest.orig50') > 0; } +step grant1 { GRANT SELECT ON vactest.orig50 TO PUBLIC; } + +# Update of the tuple grant1 will update. It has to be an update that uses a +# relation lock instead of LockTuple(), else GRANT would block it. Wait +# before sending invals, so s1 will not get a cache miss. +session s2 +setup { + SELECT injection_points_set_local(); + SELECT injection_points_attach('SIInsertDataEntries2', 'wait'); +} +step at2 { ALTER TABLE vactest.orig50 FORCE ROW LEVEL SECURITY; } + +# VACUUM to achieve LP_UNUSED. Again delay any s1 cache miss. +session s3 +setup { + SELECT injection_points_set_local(); + SELECT injection_points_attach('SIInsertDataEntries3', 'wait'); +} +step vac3 { VACUUM pg_class; } + +# Non-blocking actions. +session s4 +# Reuse the lp that s1 is waiting to change. I've observed reuse at the 1st +# or 18th CREATE, so create excess. +step mkrels4 { + SELECT vactest.mkrels('intruder', 1, 100); -- repopulate LP_UNUSED +} +step wakegrant4 { + SELECT injection_points_detach('heap_update-before-pin'); + SELECT injection_points_wakeup('heap_update-before-pin'); +} +step wakeinval4 { + SELECT injection_points_detach('SIInsertDataEntries2'); + SELECT injection_points_detach('SIInsertDataEntries3'); + SELECT injection_points_wakeup('SIInsertDataEntries2'); + SELECT injection_points_wakeup('SIInsertDataEntries3'); +} + +# tuple $FROM_SYSCACHE changes from LP_UNUSED to a successor +permutation + cachefill1 # reads pg_class tuple T0 for vactest.orig50, xmax invalid + at2(wakeinval4) # T0 becomes eligible for pruning, T1 is successor + vac3(at2) # T0 becomes LP_UNUSED + grant1(wakegrant4) # pauses at heap_update(T0) + mkrels4 # T0 becomes a new rel + wakegrant4 # s1 wakes, "duplicate key value violates unique" + wakeinval4 + +# subtract mkrels4: tuple $FROM_SYSCACHE becomes LP_UNUSED +permutation + cachefill1 # reads pg_class tuple T0 for vactest.orig50, xmax invalid + at2(wakeinval4) # T0 becomes eligible for pruning, T1 is successor + vac3(at2) # T0 becomes LP_UNUSED + grant1(wakegrant4) # pauses at heap_update(T0) + wakegrant4 # s1 wakes, assertion failure + wakeinval4