public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jakub Wartak <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: Sami Imseih <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: [email protected]
Cc: Zsolt Parragi <[email protected]>
Subject: Re: Flush some statistics within running transactions
Date: Wed, 18 Feb 2026 12:37:01 +0100
Message-ID: <CAKZiRmzoyTxi9hq7bMC6W+JMiZVVhuFhwPbOzgNJV5r+zgMQXw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAA5RZ0tvxZTZ7Xwm7FjWxAY_OphCq7tmpCaLgas0qrh+J86pRg@mail.gmail.com>
<[email protected]>
<CAA5RZ0tgr5iFuBDofX9n4SNbo-SFjzNFqTGHt7yQFAmfWBf-Rw@mail.gmail.com>
<CAA5RZ0sqJzpBZnOY3rOy5x7+WL_UDnoMkn2S_ZoXPPOWZkXSbA@mail.gmail.com>
<[email protected]>
<CAA5RZ0tK=-Dukr4ofmKpPSK4-LwqrGXmzkqGoMuJKPXnGW3=AA@mail.gmail.com>
<[email protected]>
<CAA5RZ0tPsU_bCATn-Wtf8hMaKmrCwfxtLcY9Pp3NQPpLrH2G_Q@mail.gmail.com>
<aZQTCJJm61J/[email protected]>
<CAA5RZ0vDh+vbE5SY-+azQBxEhXrywaXGrK_Qn8DKEBwNsrDH_Q@mail.gmail.com>
<[email protected]>
On Wed, Feb 18, 2026 at 6:41 AM Bertrand Drouvot
<[email protected]> wrote:
>
> Hi,
>
> On Tue, Feb 17, 2026 at 01:18:35PM -0600, Sami Imseih wrote:
> > >
> > > > I do not have any further comments on this patchset.
> > >
> > > Thanks for the review!
> >
> > I flipped this CF entry to Ready-for-committer
>
> Thanks!
>
> PFA a mandatory rebase (nothing that needs review) due to a92b809f9da1.
Hi Bertrand!
Thanks for working on this. I've took a quick look on this patchset:
v8-0005: you start using pgstat_schedule_anytime_update() from really
hot macros like pgstat_count_buffer_hit() / pgstat_count_buffer_read()
or pgstat_count_heap_getnext(), e.g.:
#define pgstat_count_buffer_hit(rel)
do {
if (pgstat_should_count_relation(rel))
+ {
(rel)->pgstat_info->counts.blocks_hit++;
+ pgstat_schedule_anytime_update();
+ }
} while (0)
where #define pgstat_schedule_anytime_update() is
do {
if (IsUnderPostmaster &&
!get_timeout_active(ANYTIME_STATS_UPDATE_TIMEOUT))
enable_timeout_after(ANYTIME_STATS_UPDATE_TIMEOUT,
pgstat_flush_interval);
} while (0)
however that function (get_timeout_active()) is not static inlined so I'm
wondering wouldn't there some major performance impact? Those buffer
macros seem to be pretty heavy hitters, e.g. quite often even per single
buffer in PinBufferForBlock():
pgstat_count_buffer_read(rel);
if (*foundPtr)
pgstat_count_buffer_hit(rel);
so it seems to be:
- often unnecessary double work (and probably as this is a function call to
get_timeout_active it won't be optimized by compiler?)
- but the main question is: why do we need that often to recheck and re-enable
timers so often from such hot places?
v8-0001: this patch modifies ProcessInterrupts() which checks for
AnytimeStatsUpdateTimeoutPending and it may happen that it takes LWLocks
(pgstat_report_anytime_stat()->pgstat_flush_pending_entries()->e.g.
pgstat_*_flush_cb()-> pgstat_lock_entry() -> LWLock)
(It's more a question than finding): isn't it too risky to take that
LWLock from potentially random spots as the CHECK_FOR_INTERRUPTS() is
literally everywhere (~318 places). Wouldn't it be safer to flush
from a couple of desired places?
-J.
view thread (22+ 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], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Flush some statistics within running transactions
In-Reply-To: <CAKZiRmzoyTxi9hq7bMC6W+JMiZVVhuFhwPbOzgNJV5r+zgMQXw@mail.gmail.com>
* 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