Re: pg_stat_statements: Avoid holding excessive lock

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Karina Litskevich <litskevichkarina@gmail.com>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-11-07T08:16:51Z
Lists: pgsql-hackers
On Tue, Nov 05, 2024 at 08:37:08PM +0300, Karina Litskevich wrote:
> I suggest eliminating holding the excessive lock. See the attached patch.
> This would also restore the consistency between the code and the comments
> about entry's mutex spinlock usage.

You are right.  minmax_stats_since and stats_since are only set when
an entry is allocated or reset, so this is not going to matter.

> +		/*
> +		 * There is no need to hold entry->mutex when reading stats_since and
> +		 * minmax_stats_since for (unlike counters) they are always written
> +		 * while holding pgss->lock exclusively. We are holding pgss->lock
> +		 * shared so there should be no race here.
> +		 */
>  		stats_since = entry->stats_since;
>  		minmax_stats_since = entry->minmax_stats_since;
> -		SpinLockRelease(&entry->mutex);

The comment could be simpler, say a "The spinlock is not required when
reading these two as they are always updated when holding pgss->lock
exclusively.".  Or something like that.
--
Michael

Commits

  1. pg_stat_statements: Avoid some locking during PGSS entry scans