Re: DSA_ALLOC_NO_OOM vs dsm_create ERROR leaving a half-initialized pgstats hash entry - Mailing list pgsql-hackers

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