Re: Spurious pgstat_drop_replslot() call

On Fri, Mar 08, 2024 at 10:19:11AM +0000, Bertrand Drouvot wrote:
> Indeed, it does not seem appropriate to remove stats during slot invalidation as
> one could still be interested to look at them.

Yeah, my take is that this can still be interesting to know, at least
for debugging. This would limit the stats to be dropped when the slot
is dropped, and that looks like a sound idea.

> This spurious call has been introduced in be87200efd. I think that initially we
> designed to drop slots when a recovery conflict occurred during logical decoding
> on a standby. But we changed our mind to invalidate such a slot instead.
>
> The spurious call is probably due to the initial design but has not been removed.

This is not a subject that has really been touched on the original
thread mentioned on the commit, so it is a bit hard to be sure. The
only comment is that a previous version of the patch did the stats
drop in the slot invalidation path at an incorrect location.

> I don't think it's worth to add a test but can do if one feel the need.

Well, that would not be complicated while being cheap, no? We have a
few paths in the 035 test where we know that a slot has been
invalidated so it is just a matter of querying once
pg_stat_replication_slot and see if some data is still there.
--
Michael