On Mon, Sep 07, 2026 at 02:52:59PM +0000, Grigorev Jurij wrote: > However, the stats-file restore path in pgstat_read_statsfile() releases > the dshash lock and raises ERROR without deleting the entry: > > header = pgstat_init_entry(key.kind, p); > dshash_release_lock(pgStatLocal.shared_hash, p); > if (header == NULL) > elog(ERROR, ...); > > At that point, the entry has already been inserted and initialized with > dropped=false, refcount=1 and generation=0, but its body is still > InvalidDsaPointer. After the lock is released, that half-initialized > entry remains in the shared hash rather than being removed on the > allocation failure.
Well, this addition is not really necessary. When we read the stats from disk, we are in the startup process and an ERROR would upgrade to a FATAL, and shared memory gets reinitialized. So it is not possible to observe a half-baked entry as far as I know for this path, at least as coded on HEAD. > I cannot prove that pgstat_read_statsfile() created those particular > entries. On a clean startup, an ERROR while restoring the stats file > normally prevents the cluster from continuing. The same hash-entry > state could also be left if a process were terminated inside > pgstat_init_entry(), after the entry had been marked live but before its > body was assigned. The normal creation path does remove the entry when > pgstat_init_entry() returns NULL. That points to a different issue to me. If you see something going on, please feel free, but I have not heard of other similar reports yet. > The patch is intentionally limited to making the restore caller match > the existing creation path. It does not change the ERROR behavior or > add defensive NULL checks to readers. Saying that, there is I guess an argument of consistency with the entry creation path. If someone has the idea to change the ERROR to a WARNING, then the post-read cleanup doing all the discard would see a half-baked entry, due to pgstat_drop_entry_internal() in pgstat_reset_after_failure -> pgstat_drop_all_entries() if I follow that right. That would be surprising, still that's not something worth more than a HEAD-only change. -- Michael
signature.asc
Description: PGP signature
