Thanks for this patch!
> I have looked at what can be done, and finished with a rather simple
> approach, as we have only one code path when a new entry is inserted,
> and one code path when an entry is removed when the refcount of an
> entry reaches 0 (includes resets, drops for kind matches, etc.). The
> counters are stored in a uint64 atomic, one for each stats kind in
> PgStat_ShmemControl, set only if the option is enabled.
The refcount reaches 0 when all backends release their references to the
stat, so if something like pg_stat_statements relies on a count for
deallocation purposes (to stay within the max), and that value only
decrements when all references are released, then it could end up
constantly triggering deallocation attempts. So, I am wondering if we
actually need another value that tracks "live" entries, or those that
have not yet been marked for drop. This means the live entries count
is decremented as soon as the entry is set to ->dropped.
> Hence, a stats kind may want to protect
> the entry deallocations with an extra lock, but that's not required
> either depending on how aggressive removals should happen.
+1.
As a side note, maybe I am missing something here:
In [0], should this not say ".... the entry should not be freed" instead of
".... the entry should not be dropped". The refcount deals with the freeing
of the entry after all refs are released.
[0]
https://github.com/postgres/postgres/blob/f2bae51dfd5b2edc460c86071c577a45a1acbfd7/src/include/utils/pgstat_internal.h#L98-L99
--
Sami
