Re: pgstat: don't assert on dropped entries when writing stats file

On Mon, Oct 19, 2026 at 02:52:41PM +0900, Michael Paquier wrote:
> On top of the generation check, how about doing something in
> pgstat_release_entry_ref() if we have a dropped entry with a refcount
> of 0 and the generation does not match? Hence, I'd suggest that we
> should free the entry more aggressively, perhaps.. I have not tested
> this idea, but just getting rid of this assertion does not feel like
> the right move to me as it means that we leave in the shared hashtable
> entries that should be gone, and that these entries will never be gone
> until the same key is reused. For the replication slots or an
> index-based logic, the turnover should be quick, but that may not be
> the case for all stats kinds.

I have done some testing by reusing your test case with pgbench and a
combination of things, like:
- slot peeks and sleeps to maintain references
- Concurrent activity creates and drops the slots
- Hardcoded sleep in pgstat_release_entry_ref() before the
dshash_find() to let concurrent activity mess more easily with a
backend doing a release.

And this has uncovered two more things, pointing at real issues that
your assertion was a consequence of:
- elog(ERROR) could be hit during backend exit, due to a concurrent
reinit and drop. We need to make the code slightly sloppier, that
should not fail.
- The generation match check should be a secondary check, after
checking that the entry has not been marked for drop. That could
trigger the assertion.

On top of the concurrent pgbenches, I have added some periodic
restarts in the middle of the runs, and the assertion failures can be
hit pretty quickly on HEAD. None of them show up with the patch
attached. There is also a TAP test able to trigger your assertion on
HEAD, based on what you have sent.

Thoughts?
--
Michael