pgsql: pgstat: Prevent stats reset from corrupting slotname by removing - Mailing list pgsql-committers

From Andres Freund
Subject pgsql: pgstat: Prevent stats reset from corrupting slotname by removing
Date
Msg-id E1ohDkW-001UId-8o@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
pgstat: Prevent stats reset from corrupting slotname by removing slotname

Previously PgStat_StatReplSlotEntry contained the slotname, which was mainly
used when writing out the stats during shutdown, to identify the slot in the
serialized data (at runtime the index in ReplicationSlotCtl->replication_slots
is used, but that can change during a restart). Unfortunately the slotname was
overwritten when the slot's stats were reset.

That turned out to only cause "real" problems if the slot was active during
the reset, triggering an assertion failure at the next
pgstat_report_replslot(). In other paths the stats were re-initialized during
pgstat_acquire_replslot().

Fix this by removing slotname from PgStat_StatReplSlotEntry. Instead we can
get the slot's name from the slot itself. Besides fixing a bug, this also is
architecturally cleaner (a name is not really statistics). This is safe
because stats, for a slot removed while shut down, will not be restored at
startup.

In 15 the slotname is not removed, but renamed, to avoid changing the stats
format. In master, bump PGSTAT_FILE_FORMAT_ID.

This commit does not contain a test for the fix. I think this can only be
tested by a tap test starting pg_recvlogical in the background and checking
pg_recvlogical's output. That type of test is notoriously hard to be reliable,
so committing it shortly before the release is wrapped seems like a bad idea.

Reported-by: Jaime Casanova <jcasanov@systemguards.com.ec>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Discussion: https://postgr.es/m/YxfagaTXUNa9ggLb@ahch-to
Backpatch: 15-, where the bug was introduced in 5891c7a8ed8f

Branch
------
REL_15_STABLE

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

Modified Files
--------------
src/backend/replication/slot.c               | 28 ++++++++++++++
src/backend/utils/activity/pgstat.c          |  2 +-
src/backend/utils/activity/pgstat_replslot.c | 56 +++++++++++++---------------
src/include/pgstat.h                         |  6 ++-
src/include/replication/slot.h               |  1 +
src/include/utils/pgstat_internal.h          |  5 ++-
6 files changed, 64 insertions(+), 34 deletions(-)


pgsql-committers by date:

Previous
From: Peter Eisentraut
Date:
Subject: pgsql: Use fabsf() instead of Abs() or fabs() where appropriate
Next
From: Peter Eisentraut
Date:
Subject: pgsql: Update list of acknowledgments in release notes