Fix cleanup of reused pgstats entries in pgstat_release_entry_ref()

When a stats entry is reinitialized and dropped again while a backend
still holds a reference from an older generation, the backend releasing
the last reference found a generation mismatch and left behind an entry
marked as dropped with a refcount of 0.  Nothing would free it, and the
checkpointer would trigger an assertion at shutdown when flushing the
stats.

Looking more at the issue, using some hardcoded sleeps before the
dshash_find() that takes an exclusive lock when releasing an entry with
some concurrent activity, there was more going on:
- A missing entry could be reported as an error at backend exit, error
that should not be hit even if the entry was reinitialized and dropped
concurrently.
- The generation check was not correct for the case of 0 due to the
first issue.  A generation of 0 refers to an entry that has been freshly
created.

This commit reworks the entry release to be safer concurrent-wise when
an entry targetted for release is found as already existing:
- If marked as dropped without refount at 0, free it.
- In other cases, ignore it.  The entry has been reinitialized or
created again with the same key, and is still in use.

Sami has posted a basic test case (equivalent added in this commit) that
gives a way to reproduce the assertion failure on backend exit, which is
a consequence of the incorrect handling of entry releases.

Reported-by: Sami Imseih <[email protected]>
Reviewed-by: Sami Imseih <[email protected]>
Discussion: 
https://postgr.es/m/CAN12+YJXE+rEOVo74uM5Ofbj_t_m0DHvd_Mo4ppeHV=advv...@mail.gmail.com
Backpatch-through: 15

Branch
------
REL_15_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/7a405e5d7846999368dea8179085e6a96adf53a5

Modified Files
--------------
contrib/test_decoding/t/001_repl_stats.pl | 38 +++++++++++++++++++++++++
src/backend/utils/activity/pgstat_shmem.c | 46 +++++++++++--------------------
2 files changed, 54 insertions(+), 30 deletions(-)

Reply via email to