pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Fix invalidation of local pgstats references for entry reinitial
Date: Mon, 09 Dec 2024 01:46:38 +0000
Message-ID: <E1tKSr7-001nUM-W4@gemulon.postgresql.org> (raw)

Fix invalidation of local pgstats references for entry reinitialization

818119afccd3 has introduced the "generation" concept in pgstats entries,
incremented a counter when a pgstats entry is reinitialized, but it did
not count on the fact that backends still holding local references to
such entries need to be refreshed if the cache age is outdated.  The
previous logic only updated local references when an entry was dropped,
but it needs also to consider entries that are reinitialized.

This matters for replication slot stats (as well as custom pgstats kinds
in 18~), where concurrent drops and creates of a slot could cause
incorrect stats to be locally referenced.  This would lead to an
assertion failure at shutdown when writing out the stats file, as the
backend holding an outdated local reference would not be able to drop
during its shutdown sequence the stats entry that should be dropped, as
the last process holding a reference to the stats entry.  The
checkpointer was then complaining about such an entry late in the
shutdown sequence, after the shutdown checkpoint is finished with the
control file updated, causing the stats file to not be generated.  In
non-assert builds, the entry would just be skipped with the stats file
written.

Note that only logical replication slots use statistics.

A test case based on TAP is added to test_decoding, where a persistent
connection peeking at a slot's data is kept with concurrent drops and
creates of the same slot.  This is based on the isolation test case that
Anton has sent.  As it requires a node shutdown with a check to make
sure that the stats file is written with this specific sequence of
events, TAP is used instead.

Reported-by: Anton A. Melnikov
Reviewed-by: Bertrand Drouvot
Discussion: https://postgr.es/m/56bf8ff9-dd8c-47b2-872a-748ede82af99@postgrespro.ru
Backpatch-through: 15

Branch
------
REL_16_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/ae77bcc3aa1b869568c6b01726e4f1276f7381bb

Modified Files
--------------
contrib/test_decoding/t/001_repl_stats.pl | 60 +++++++++++++++++++++++++++++++
src/backend/utils/activity/pgstat_shmem.c | 11 ++++--
2 files changed, 69 insertions(+), 2 deletions(-)



view thread (4+ messages)

Message-ID: <E1tKSr7-001nUM-W4@gemulon.postgresql.org>
Permalink:  ../E1tKSr7-001nUM-W4@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1tKSr7-001nUM-W4@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: michael@paquier.xyz, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Fix invalidation of local pgstats references for entry reinitial
  In-Reply-To: <E1tKSr7-001nUM-W4@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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