Nice finding.
At Tue, 13 Sep 2022 00:39:45 -0500, Jaime Casanova
<jcasa...@systemguards.com.ec> wrote in
> and the problem seems to be that after zero'ing the stats that includes
> the name of the replication slot, this simple patch fixes it... not sure
> if it's the right fix though...
That doesn't work. since what that function clears is not the name in
the slot struct but that in stats entry.
The cause is what pg_stat_reset_replslot wants to do does not match
what pgstat feature thinks as reset.
Unfortunately, we don't have a function to leave a portion alone after
a reset. However, fetching the stats entry in pgstat_reset_replslot is
ugly..
I'm not sure this is less uglier but it works if
pgstat_report_replslot sets the name if it is found to be wiped
out... But that hafly nullify the protction by the assertion just
after.
--- a/src/backend/utils/activity/pgstat_replslot.c
+++ b/src/backend/utils/activity/pgstat_replslot.c
@@ -83,9 +83,11 @@ pgstat_report_replslot(ReplicationSlot *slot, const
PgStat_StatReplSlotEntry *re
statent = &shstatent->stats;
/*
- * Any mismatch should have been fixed in pgstat_create_replslot() or
- * pgstat_acquire_replslot().
+ * pgstat_create_replslot() and pgstat_acquire_replslot() enters the
name,
+ * but pgstat_reset_replslot() may clear it.
*/
+ if (statent->slotname.data[0] == 0)
+ namestrcpy(&shstatent->stats.slotname,
NameStr(slot->data.name));
Assert(namestrcmp(&statent->slotname, NameStr(slot->data.name)) == 0);
Another measure would be to add the region to wipe-out on reset to
PgStat_KindInfo, but it seems too much.. (attached)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/activity/pgstat.c
b/src/backend/utils/activity/pgstat.c
index 6224c498c2..ab88ebbc64 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -263,6 +263,8 @@ static const PgStat_KindInfo
pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
.shared_size = sizeof(PgStatShared_Database),
.shared_data_off = offsetof(PgStatShared_Database, stats),
.shared_data_len = sizeof(((PgStatShared_Database *) 0)->stats),
+ .reset_off = offsetof(PgStatShared_Database, stats),
+ .reset_len = sizeof(((PgStatShared_Database *) 0)->stats),
.pending_size = sizeof(PgStat_StatDBEntry),
.flush_pending_cb = pgstat_database_flush_cb,
@@ -277,6 +279,8 @@ static const PgStat_KindInfo
pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
.shared_size = sizeof(PgStatShared_Relation),
.shared_data_off = offsetof(PgStatShared_Relation, stats),
.shared_data_len = sizeof(((PgStatShared_Relation *) 0)->stats),
+ .reset_off = offsetof(PgStatShared_Relation, stats),
+ .reset_len = sizeof(((PgStatShared_Relation *) 0)->stats),
.pending_size = sizeof(PgStat_TableStatus),
.flush_pending_cb = pgstat_relation_flush_cb,
@@ -291,6 +295,8 @@ static const PgStat_KindInfo
pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
.shared_size = sizeof(PgStatShared_Function),
.shared_data_off = offsetof(PgStatShared_Function, stats),
.shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats),
+ .reset_off = offsetof(PgStatShared_Function, stats),
+ .reset_len = sizeof(((PgStatShared_Function *) 0)->stats),
.pending_size = sizeof(PgStat_BackendFunctionEntry),
.flush_pending_cb = pgstat_function_flush_cb,
@@ -307,6 +313,9 @@ static const PgStat_KindInfo
pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
.shared_size = sizeof(PgStatShared_ReplSlot),
.shared_data_off = offsetof(PgStatShared_ReplSlot, stats),
.shared_data_len = sizeof(((PgStatShared_ReplSlot *) 0)->stats),
+ .reset_off = offsetof(PgStatShared_ReplSlot, stats.spill_txns),
+ .reset_len = sizeof(PgStatShared_ReplSlot) -
+ offsetof(PgStatShared_ReplSlot, stats.spill_txns),
.reset_timestamp_cb = pgstat_replslot_reset_timestamp_cb,
.to_serialized_name = pgstat_replslot_to_serialized_name_cb,
@@ -323,6 +332,8 @@ static const PgStat_KindInfo
pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
.shared_size = sizeof(PgStatShared_Subscription),
.shared_data_off = offsetof(PgStatShared_Subscription, stats),
.shared_data_len = sizeof(((PgStatShared_Subscription *)
0)->stats),
+ .reset_off = offsetof(PgStatShared_Subscription, stats),
+ .reset_len = sizeof(((PgStatShared_Subscription *) 0)->stats),
.pending_size = sizeof(PgStat_BackendSubEntry),
.flush_pending_cb = pgstat_subscription_flush_cb,
diff --git a/src/backend/utils/activity/pgstat_shmem.c
b/src/backend/utils/activity/pgstat_shmem.c
index ac98918688..09a8c3873c 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -915,8 +915,9 @@ shared_stat_reset_contents(PgStat_Kind kind,
PgStatShared_Common *header,
{
const PgStat_KindInfo *kind_info = pgstat_get_kind_info(kind);
- memset(pgstat_get_entry_data(kind, header), 0,
- pgstat_get_entry_len(kind));
+ memset((char *)pgstat_get_entry_data(kind, header) +
+ kind_info->reset_off, 0,
+ kind_info->reset_len);
if (kind_info->reset_timestamp_cb)
kind_info->reset_timestamp_cb(header, ts);
diff --git a/src/include/utils/pgstat_internal.h
b/src/include/utils/pgstat_internal.h
index 901d2041d6..3715a48776 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -215,6 +215,12 @@ typedef struct PgStat_KindInfo
uint32 shared_data_off;
uint32 shared_data_len;
+ /*
+ * The offset/size of the region to wipe out when the entry is reset.
+ */
+ uint32 reset_off;
+ uint32 reset_len;
+
/*
* The size of the pending data for this kind. E.g. how large
* PgStat_EntryRef->pending is. Used for allocations.
