MIME-Version: 1.0
References: <CAA5RZ0tvxZTZ7Xwm7FjWxAY_OphCq7tmpCaLgas0qrh+J86pRg@mail.gmail.com>
 <aYDb7g5hXnaP+5BU@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0tgr5iFuBDofX9n4SNbo-SFjzNFqTGHt7yQFAmfWBf-Rw@mail.gmail.com>
 <CAA5RZ0sqJzpBZnOY3rOy5x7+WL_UDnoMkn2S_ZoXPPOWZkXSbA@mail.gmail.com>
 <aYXL6XmVABORjGF5@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0tK=-Dukr4ofmKpPSK4-LwqrGXmzkqGoMuJKPXnGW3=AA@mail.gmail.com>
 <aZLVDJLTiFlxsWZi@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0tPsU_bCATn-Wtf8hMaKmrCwfxtLcY9Pp3NQPpLrH2G_Q@mail.gmail.com>
 <aZQTCJJm61J/X4nC@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAA5RZ0vDh+vbE5SY-+azQBxEhXrywaXGrK_Qn8DKEBwNsrDH_Q@mail.gmail.com> <aZVQ3nT9B6ti8whJ@ip-10-97-1-34.eu-west-3.compute.internal>
In-Reply-To: <aZVQ3nT9B6ti8whJ@ip-10-97-1-34.eu-west-3.compute.internal>
From: Jakub Wartak <jakub.wartak@enterprisedb.com>
Date: Wed, 18 Feb 2026 12:37:01 +0100
Message-ID: <CAKZiRmzoyTxi9hq7bMC6W+JMiZVVhuFhwPbOzgNJV5r+zgMQXw@mail.gmail.com>
Subject: Re: Flush some statistics within running transactions
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>, Michael Paquier <michael@paquier.xyz>, 
	Fujii Masao <masao.fujii@gmail.com>, pgsql-hackers@lists.postgresql.org, 
	Zsolt Parragi <zsolt.parragi@percona.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAKZiRmzoyTxi9hq7bMC6W%2BJMiZVVhuFhwPbOzgNJV5r%2BzgMQXw%40mail.gmail.com>
Precedence: bulk

On Wed, Feb 18, 2026 at 6:41=E2=80=AFAM Bertrand Drouvot
<bertranddrouvot.pg@gmail.com> 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-ena=
ble
  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.