public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Karina Litskevich <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: pg_stat_statements: Avoid holding excessive lock
Date: Thu, 7 Nov 2024 17:16:51 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com>
References: <CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com>
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
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: pg_stat_statements: Avoid holding excessive lock
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox