public inbox for [email protected]  
help / color / mirror / Atom feed
Re: Flush some statistics within running transactions
2+ messages / 2 participants
[nested] [flat]

* Re: Flush some statistics within running transactions
@ 2026-01-16 11:37 Bertrand Drouvot <[email protected]>
  2026-01-16 16:44 ` Re: Flush some statistics within running transactions Sami Imseih <[email protected]>
  0 siblings, 1 reply; 2+ messages in thread

From: Bertrand Drouvot @ 2026-01-16 11:37 UTC (permalink / raw)
  To: Sami Imseih <[email protected]>; +Cc: [email protected]

Hi,

On Thu, Jan 15, 2026 at 11:25:18AM -0600, Sami Imseih wrote:
> > > > The 1 second flush interval is currently hardcoded but we could imagine increase
> > > > it or make it configurable.
> > >
> > > Someone may want to turn this off as well. I think a GUC will be needed.
> >
> > I gave this more thoughts and I wonder if this should be configurable at all.
> > I mean, we don't do it for PGSTAT_MIN_INTERVAL, PGSTAT_MAX_INTERVAL and
> > PGSTAT_IDLE_INTERVAL. We could imagine make it configurable if it produces
> > noticeable performance impact but that's not what I observed.
> 
> Is there a reason we need a new constant (PGSTAT_ANYTIME_FLUSH_INTERVAL)
> for anytime flushes and can't rely on the existing PGSTAT_MIN_INTERVAL?

It currently gives flexibility for testing. If we agree that 1s is the right value
to set and that it should not be configurable then yeah we could replace it with
PGSTAT_MIN_INTERVAL then.

> Also, How did you benchmark? I am less concerned about long running
> transactions,
> background processes and more about short/high concurrency transactions seeing
> additional overhead due to additional flushing. Is that latter a concern?

I ran 3 kinds of tests:

1/
pgbench -c 32 -j 4 -T 60 -f short.sql -n -r $DB

with short.sql:

\set t1 random(1, 100)
\set t2 random(1, 100)
\set t3 random(1, 100)
\set t4 random(1, 100)
\set t5 random(1, 100)
\set t6 random(1, 100)
\set t7 random(1, 100)
\set t8 random(1, 100)
\set t9 random(1, 100)
\set t10 random(1, 100)
\set row random(1, 1000)

BEGIN;
UPDATE t:t1 SET val = val + 1 WHERE id = :row;
UPDATE t:t2 SET val = val + 1 WHERE id = :row;
UPDATE t:t3 SET val = val + 1 WHERE id = :row;
UPDATE t:t4 SET val = val + 1 WHERE id = :row;
UPDATE t:t5 SET val = val + 1 WHERE id = :row;
UPDATE t:t6 SET val = val + 1 WHERE id = :row;
UPDATE t:t7 SET val = val + 1 WHERE id = :row;
UPDATE t:t8 SET val = val + 1 WHERE id = :row;
UPDATE t:t9 SET val = val + 1 WHERE id = :row;
UPDATE t:t10 SET val = val + 1 WHERE id = :row;
COMMIT;

2/
psql $DB -f long.sql

with long.sql:

DO $$
BEGIN
  FOR i IN 1..100 LOOP
    EXECUTE format('TRUNCATE TABLE t%s', i);
    EXECUTE format('INSERT INTO t%s SELECT generate_series(1, 1000000)', i);
    EXECUTE format('UPDATE t%s SET val = val + 1', i);
    EXECUTE format('SELECT COUNT(1) FROM t%s', i);
  END LOOP;
END $$;

3/
pgbench -i -s 50 $DB
pgbench -c 32 -j 4 -T 60 -N -n -r $DB

I don't think this feature could add a noticeable performance impact, so the tests
have been that simple. Do you think we should worry more?

> > > I’m concerned that fields being temporarily out of sync might impact monitoring
> > > calculations, if the formula is dealing with fields that have
> > > different flush strategies.
> >
> > That's a good point. Maybe we should document the fields flush strategy?
> 
> Yeah, we will need to document this.

Will do in the next version.

> I checked by running seq scans in a long running transaction,
> and I observed both for these values being updated at the same time. I think
> this is OK.
> 

I do think the same.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com






^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: Flush some statistics within running transactions
  2026-01-16 11:37 Re: Flush some statistics within running transactions Bertrand Drouvot <[email protected]>
@ 2026-01-16 16:44 ` Sami Imseih <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Sami Imseih @ 2026-01-16 16:44 UTC (permalink / raw)
  To: Bertrand Drouvot <[email protected]>; +Cc: [email protected]

I took a look at 0001 in depth.

> I don't think this feature could add a noticeable performance impact, so the tests
> have been that simple. Do you think we should worry more?

One observation is there's no coordination between ANYTIME and
TXN_BOUNDARY flushes. While PGSTAT_MIN_INTERVAL
prevents a backend from flushing more than once per second, a backend can
still perform both an ANYTIME flush and a TXN_BOUNDARY flush within
the same 1-second window. Not saying this will be a real problem in
the real-world,
but we definitely took measures in the current implementation to avoid
this scenario.

A few other comments on 0001

+               /* Skip if completely idle */
+               if (!DoingCommandRead || IsTransactionOrTransactionBlock())
+                       pgstat_report_anytime_stat(false);

Does this need to be conditional? worst case, we return right away with an empty
list. Best case, is we are consistently flushing.

+       /*
+        * When in anytime_only mode, the list may not be empty because
+        * FLUSH_AT_TXN_BOUNDARY entries were skipped.
+        */
+       Assert(!anytime_only || dlist_is_empty(&pgStatPending) ==
!have_pending);

Checking for !anytime_only is unnecessary here.
"list_is_empty(&pgStatPending) == !have_pending"
should be true regardless of ANYTIME or TXN_BOUNDARY, right?

Below are a couple of edits for comments I felt would improve
readability of the code.

1/
 /*
- * Flush non-transactional stats
- *
- * This is safe to call even inside a transaction. It only flushes stats
- * kinds marked as FLUSH_ANYTIME.
- *
- * This allows long running transactions to report activity without waiting
- * for transaction to finish.
+ * Flushes only FLUSH_ANYTIME stats using non-blocking locks. Transactional
+ * stats (FLUSH_AT_TXN_BOUNDARY) remain pending until transaction boundary.
+ * Safe to call inside transactions.
  */

2/
 typedef enum PgStat_FlushBehavior
 {
-       FLUSH_ANYTIME,                          /* All fields can
flush anytime */
-       FLUSH_AT_TXN_BOUNDARY,          /* All fields need transaction
boundary */
+       FLUSH_ANYTIME,                          /* All fields can be
flushed anytime,
+                                                                *
including within transactions */
+       FLUSH_AT_TXN_BOUNDARY,          /* All fields can only be flushed at
+                                                                *
transaction boundary */
 } PgStat_FlushBehavior;

I will start looking at the remaining patches next.

--
Sami Imseih
Amazon Web Services (AWS)






^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2026-01-16 16:44 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-16 11:37 Re: Flush some statistics within running transactions Bertrand Drouvot <[email protected]>
2026-01-16 16:44 ` Sami Imseih <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox