Re: DSA_ALLOC_NO_OOM vs dsm_create ERROR leaving a half-initialized pgstats hash entry

On Thu, Sep 17, 2026 at 11:48:26AM +0900, Michael Paquier wrote:
> + /*
> + * Allocate the stats body before inserting a hash entry. Creating a
> + * new DSA segment can raise ERROR (e.g. ENOSPC on posix shm); doing
> + * that after the insert would leave a live hash entry with an
> + * invalid body.
> + */
> + chunk = pgstat_alloc_entry_body(kind);
>
> Hmm. This still leaves a local entry_ref if pgstat_alloc_entry_body()
> itself fails. Compared to the case of a corrupted shmem area. I think
> that I can live with that. And if I'm reading that right, the backend
> reference that may still be around self-heals on re-entry if a backend
> tries to insert again the same entry?

Ah. 4069df21beb8 points exactly at that case. Perhaps we should
extend pgstat_gc_entry_refs() so as it is able to handle gracefully a
partial reference then? I would imagine something like that, that
forces a release of the local entry if we don't have a shared_entry,
as of:
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -839,6 +839,15 @@ pgstat_gc_entry_refs(void)
Assert(!entry_ref->shared_stats ||
entry_ref->shared_stats->magic == 0xdeadbeef);

+ /* NULL shared_entry marks a partial reference */
+ if (entry_ref->shared_entry == NULL)
+ {
+ Assert(entry_ref->shared_stats == NULL);
+ Assert(entry_ref->pending == NULL);
+ pgstat_release_entry_ref(ent->key, entry_ref, false);
+ continue;
+ }

What do you think about the attached? That would be an independent
safety measure.
--
Michael