Re: Flush some statistics within running transactions

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Sami Imseih <[email protected]>
To: Bertrand Drouvot <[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: Fri, 13 Feb 2026 20:23:01 -0600
Message-ID: <CAA5RZ0tK=-Dukr4ofmKpPSK4-LwqrGXmzkqGoMuJKPXnGW3=AA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAHGQGwHttst8tv_WWYNoGGfL1UAq4kiy6dpFXoxEkJwHMS9FtQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAA5RZ0tGhu9jXGgFjzx7yK4cseE+t40Q4qZ+KhY66B2MZ=pBrQ@mail.gmail.com>
	<CAA5RZ0tvxZTZ7Xwm7FjWxAY_OphCq7tmpCaLgas0qrh+J86pRg@mail.gmail.com>
	<[email protected]>
	<CAA5RZ0tgr5iFuBDofX9n4SNbo-SFjzNFqTGHt7yQFAmfWBf-Rw@mail.gmail.com>
	<CAA5RZ0sqJzpBZnOY3rOy5x7+WL_UDnoMkn2S_ZoXPPOWZkXSbA@mail.gmail.com>
	<[email protected]>

> PFA attached v6, addressing the reviews comments.

Thanks for the patches!

v6 is getting closer IMO. Here are some comments I have.

v6-0001 looks solid, but some minor comments:
1/

+    pgstat_flush_backend(nowait, PGSTAT_BACKEND_FLUSH_WAL, true);

let's also use explicit (void) cast here

2/

+ * Timeout handler for flushing non-transactional stats.

I also noticed in v6-0005, we refer to "anytime" stats as
"non-transactional". It's better to just refer to them as "anytime"
anywhere instead of "non-transactional:.


v6-0002:

+ if (nowait && !LWLockConditionalAcquire(&stats_shmem->lock, LW_EXCLUSIVE))
+ return true; /* failed to flush */
+
+ LWLockAcquire(&stats_shmem->lock, LW_EXCLUSIVE);

Here the LWLock acquisition will deadlock if we are nowait, successfully
acquire the lock conditionally, and then we try to acquire it again. The
logic here should not try to acquire the lock twice.

+    -- Force anytime flush (inside transaction!)
+    select pg_stat_force_anytime_flush();

Not sure why we need  pg_stat_force_anytime_flush.
A pg_sleep is sufficient, like below. right?

```
select test_custom_stats_fixed_anytime_update() from generate_series(1, 2);
select pg_sleep(1.5);
select 'anytime:'||numcalls from test_custom_stats_fixed_report();
```

v6-0003:

1/
Suggested doc changes:

        <para>
-        Sets the interval at which statistics that can be updated while a
-        transaction is still running are made visible. These include,
for example,
-        WAL activity and I/O operations.
+        Sets the interval at which certain statistics, which can be
updated while a
+        transaction is in progress, are made visible. These include
WAL activity
+        and I/O operations.
         Such statistics are refreshed at the specified interval and
can be observed
         during active transactions in monitoring views such as
-        <link linkend="monitoring-pg-stat-io-view"><structname>pg_stat_io</structname></link>
+        <link linkend="monitoring-pg-stat-wal-view"><structname>pg_stat_wal</structname></link>
         and
-        <link linkend="monitoring-pg-stat-wal-view"><structname>pg_stat_wal</structname></link>.
-        Other statistics are only made visible at transaction end and are not
-        affected by this setting.
+        <link linkend="monitoring-pg-stat-io-view"><structname>pg_stat_io</structname></link>.
         If the value is specified without a unit, milliseconds are assumed.
         The default is 10 seconds (<literal>10s</literal>), which is generally
         the smallest practical value for long-running transactions.

-        Other statistics are only made visible at transaction end and are not
-        affected by this setting.

I removed this, because it's mentioned in the notes section later on.

2/
I don't see we have tests for other timeout based GUCs, but it would nice
to ensure that this woks correctly. Maybe as a custom_stats test where we
SET stats_flush_interval inside the transaction and make sure the stats flush
only after the new timeout has passed. Maybe?

v6-0004:

1/

NIT:

+$node_primary->append_conf('postgresql.conf', "stats_flush_interval= '1s'");

+$node_publisher->append_conf('postgresql.conf', "stats_flush_interval= '1s'");

missing space before the equal sign.

v6-0005:

1/

        /* Partial flush only happens in anytime mode for FLUSH_ANYTIME stats */
        is_partial_flush = (anytime_only && kind_info->flush_mode ==
FLUSH_ANYTIME);

Will this be tue at all time? Let's imagine a Kind that flushes all the fields
ANYTIME, would we not want to delete the pending entry?

+               /* if successfull non-partial flush, remove entry */
+               if (did_flush && !is_partial_flush)
                        pgstat_delete_pending_entry(entry_ref);


2/ indentation:

     Some statistics are updated while a transaction is in progress
(for example,
     <structfield>blks_read</structfield>, <structfield>blks_hit</structfield>,
     <structfield>tup_returned</structfield> and
<structfield>tup_fetched</structfield>).
-     Statistics that either do not depend on transactions or require
transactional
-     consistency are updated only when the transaction ends.
Statistics that require
-     transactional consistency include <structfield>xact_commit</structfield>,
-     <structfield>xact_rollback</structfield>,
<structfield>tup_inserted</structfield>,
-     <structfield>tup_updated</structfield> and
<structfield>tup_deleted</structfield>.
+    Statistics that either do not depend on transactions or require
transactional
+    consistency are updated only when the transaction ends.
Statistics that require
+    transactional consistency include <structfield>xact_commit</structfield>,
+    <structfield>xact_rollback</structfield>,
<structfield>tup_inserted</structfield>,
+    <structfield>tup_updated</structfield> and
<structfield>tup_deleted</structfield>.
    </para>
   </note>

--
Sami Imseih
Amazon Web Services (AWS)

view thread (27+ messages)

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]
  Subject: Re: Flush some statistics within running transactions
  In-Reply-To: <CAA5RZ0tK=-Dukr4ofmKpPSK4-LwqrGXmzkqGoMuJKPXnGW3=AA@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