Date: Tue, 04 Jul 2023 11:29:24 +0900 (JST)
Message-Id: <20230704.112924.1693682640969530760.horikyota.ntt@gmail.com>
To: tomas.vondra@enterprisedb.com
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Is a pg_stat_force_next_flush() call sufficient for regression
 tests?
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <022124fb-70c4-e59f-029e-b752853f7c84@enterprisedb.com>
References: <022124fb-70c4-e59f-029e-b752853f7c84@enterprisedb.com>
User-Agent: Mew version 6.8 on Emacs 27.2
Mime-Version: 1.0
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Tue_Jul__4_11_29_24_2023_799)--"
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230704.112924.1693682640969530760.horikyota.ntt%40gmail.com>
Precedence: bulk

----Next_Part(Tue_Jul__4_11_29_24_2023_799)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

At Mon, 3 Jul 2023 15:45:52 +0200, Tomas Vondra <tomas.vondra@enterprisedb.com> wrote in 
> So I'm wondering if pg_stat_force_next_flush() is enough - AFAICS this
> only sets some flag for the *next* pgstat_report_stat() call, but how do
> we know that happens before the query execution?
> 
> Shouldn't there be something like pg_stat_flush() that actually does the
> flushing, instead of just setting the flag?

The reason for the function is that pg_stat_flush() is supposed not to
be called within a transaction.  AFAICS pg_stat_force_next_flush()
takes effect after a successfull transaction end and before the next
command execution.

To verify this, I put in an assertion to check that the flag gets
consumed before reading of pg_stat_io (a.diff), then ran pgbench with
the attached custom script. As expected, it didn't fire at all during
several trials. When I wrapped all lines in t.sql within a
begin-commit block, the assertion fired off immediately as a matter of
course.

Is there any chance concurrent backends or some other things can
actually hinder the backend from reusing buffers?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

----Next_Part(Tue_Jul__4_11_29_24_2023_799)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="a.diff"

diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index d743fc0b28..6529e737ea 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -225,7 +225,7 @@ static dlist_head pgStatPending = DLIST_STATIC_INIT(pgStatPending);
  * Force the next stats flush to happen regardless of
  * PGSTAT_MIN_INTERVAL. Useful in test scripts.
  */
-static bool pgStatForceNextFlush = false;
+bool pgStatForceNextFlush = false;
 
 /*
  * Force-clear existing snapshot before next use when stats_fetch_consistency
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 2a4c8ef87f..7f60b2834a 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1352,7 +1352,9 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 	ReturnSetInfo *rsinfo;
 	PgStat_IO  *backends_io_stats;
 	Datum		reset_time;
+	extern bool pgStatForceNextFlush;
 
+	Assert(!pgStatForceNextFlush);
 	InitMaterializedSRF(fcinfo, 0);
 	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 

----Next_Part(Tue_Jul__4_11_29_24_2023_799)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="t.sql"

--begin;
select pg_stat_force_next_flush();
select sum(reuses) from pg_stat_io;
--commit;

----Next_Part(Tue_Jul__4_11_29_24_2023_799)----