Subject: Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)

From: Justin Pryzby <[email-masked]>
Date: Wed, 11 Jan 2023 15:58:51 -0600

> Subject: [PATCH v45 4/5] Add system view tracking IO ops per backend type

The patch can/will fail with:

CREATE TABLESPACE test_io_shared_stats_tblspc LOCATION '';
+WARNING:  tablespaces created by regression test cases should have names starting with "regress_"

CREATE TABLESPACE test_stats LOCATION '';
+WARNING:  tablespaces created by regression test cases should have names starting with "regress_"

(I already sent patches to address the omission in cirrus.yml)

1760             :                  errhint("Target must be \"archiver\", \"io\", \"bgwriter\", \"recovery_prefetch\", or \"wal\".")));
=> Do you want to put these in order?

pgstat_get_io_op_name() isn't currently being hit by tests; actually,
it's completely unused.

FlushRelationBuffers() isn't being hit for local buffers.

> +      <entry><structname>pg_stat_io</structname><indexterm><primary>pg_stat_io</primary></indexterm></entry>
> +      <entry>
> +       One row per backend type, context, target object combination showing
> +       cluster-wide I/O statistics.

I suggest: "One row for each combination of of .."

> +   The <structname>pg_stat_io</structname> and
> +   <structname>pg_statio_</structname> set of views are especially useful for
> +   determining the effectiveness of the buffer cache.  When the number of actual
> +   disk reads is much smaller than the number of buffer hits, then the cache is
> +   satisfying most read requests without invoking a kernel call.

I would change this say "Postgres' own buffer cache is satisfying ..."

> However, these
> +   statistics do not give the entire story: due to the way in which
> +   <productname>PostgreSQL</productname> handles disk I/O, data that is not in
> +   the <productname>PostgreSQL</productname> buffer cache might still reside in
> +   the kernel's I/O cache, and might therefore still be fetched without

I suggest to refer to "the kernel's page cache"

> +   The <structname>pg_stat_io</structname> view will contain one row for each
> +   backend type, I/O context, and target I/O object combination showing
> +   cluster-wide I/O statistics. Combinations which do not make sense are
> +   omitted.

"..for each combination of .."

> +          <varname>io_context</varname> for a type of I/O operation. For

"for I/O operations"

> +          <literal>vacuum</literal>: I/O operations done outside of shared
> +          buffers incurred while vacuuming and analyzing permanent relations.

s/incurred/performed/

> +          <literal>bulkread</literal>: Qualifying large read I/O operations
> +          done outside of shared buffers, for example, a sequential scan of a
> +          large table.

I don't think it's correct to say that it's "outside of" shared-buffers.
s/Qualifying/Certain/

> +          <literal>bulkwrite</literal>: Qualifying large write I/O operations
> +          done outside of shared buffers, such as <command>COPY</command>.

Same

> +        Target object of an I/O operation. Possible values are:
> +       <itemizedlist>
> +        <listitem>
> +         <para>
> +          <literal>relation</literal>: This includes permanent relations.

It says "includes permanent" but what seems to mean is that it
"exclusive of temporary relations".

> +     <row>
> +      <entry role="catalog_table_entry">
> +       <para role="column_definition">
> +        <structfield>read</structfield> <type>bigint</type>
> +       </para>
> +       <para>
> +        Number of read operations in units of <varname>op_bytes</varname>.

This looks too much like it means "bytes".
Should say: "in number of blocks of size >op_bytes<"

But wait - is it the number of read operations "in units of op_bytes"
(which would means this already multiplied by op_bytes, and is in units
of bytes).

Or the "number of read operations" *of* op_bytes chunks ?  Which would
mean this is a "pure" number, and could be multipled by op_bytes to
obtain a size in bytes.

> +        Number of write operations in units of <varname>op_bytes</varname>.

> +        Number of relation extend operations in units of
> +        <varname>op_bytes</varname>.

same

> +        In <varname>io_context</varname> <literal>normal</literal>, this counts
> +        the number of times a block was evicted from a buffer and replaced with
> +        another block. In <varname>io_context</varname>s
> +        <literal>bulkwrite</literal>, <literal>bulkread</literal>, and
> +        <literal>vacuum</literal>, this counts the number of times a block was
> +        evicted from shared buffers in order to add the shared buffer to a
> +        separate size-limited ring buffer.

This never defines what "evicted" means.  Does it mea that a dirty
buffer was written out ?

> +        The number of times an existing buffer in a size-limited ring buffer
> +        outside of shared buffers was reused as part of an I/O operation in the
> +        <literal>bulkread</literal>, <literal>bulkwrite</literal>, or
> +        <literal>vacuum</literal> <varname>io_context</varname>s.

Maybe say "as part of a bulk I/O operation (bulkread, bulkwrite, or
vacuum)."

> +  <para>
> +   <structname>pg_stat_io</structname> can be used to inform database tuning.

> +   For example:
> +   <itemizedlist>
> +    <listitem>
> +     <para>
> +      A high <varname>evicted</varname> count can indicate that shared buffers
> +      should be increased.
> +     </para>
> +    </listitem>
> +    <listitem>
> +     <para>
> +      Client backends rely on the checkpointer to ensure data is persisted to
> +      permanent storage. Large numbers of <varname>files_synced</varname> by
> +      <literal>client backend</literal>s could indicate a misconfiguration of
> +      shared buffers or of checkpointer. More information on checkpointer

of *the* checkpointer

> +      Normally, client backends should be able to rely on auxiliary processes
> +      like the checkpointer and background writer to write out dirty data as

*the* bg writer

> +      much as possible. Large numbers of writes by client backends could
> +      indicate a misconfiguration of shared buffers or of checkpointer. More

*the* ckpointer

Should this link to various docs for checkpointer/bgwriter ?

Maybe the docs for ALTER/COPY/VACUUM/CREATE/etc should be updated to
refer to some central description of ring buffers.  Maybe something
should be included to the appendix.

-- 
Justin

---

From: Melanie Plageman <[email-masked]>
Date: Thu, 12 Jan 2023 21:19:36 -0500

Attached is v46.

On Wed, Dec 28, 2022 at 6:56 PM Andres Freund <[email-masked]> wrote:
> On 2022-10-06 13:42:09 -0400, Melanie Plageman wrote:
> > > Additionally, some minor notes:
> > >
> > > - Since the stats are counting blocks, it would make sense to prefix the view columns with "blks_", and word them in the past tense (to match current style), i.e. "blks_written", "blks_read", "blks_extended", "blks_fsynced" (realistically one would combine this new view with other data e.g. from pg_stat_database or pg_stat_statements, which all use the "blks_" prefix, and stop using pg_stat_bgwriter for this which does not use such a prefix)
> >
> > I have changed the column names to be in the past tense.
>
> For a while I was convinced by the consistency argument (after Melanie
> pointing it out to me). But the more I look, the less convinced I am. The
> existing IO related stats in pg_stat_database, pg_stat_bgwriter aren't past
> tense, just the ones in pg_stat_statements. pg_stat_database uses past tense
> for tup_*, but not xact_*, deadlocks, checksum_failures etc.
>
> And even pg_stat_statements isn't consistent about it - otherwise it'd be
> 'planned' instead of 'plans', 'called' instead of 'calls' etc.
>
> I started to look at the naming "tense" issue again, after I got "confused"
> about "extended", because that somehow makes me think about more detailed
> stats or such, rather than files getting extended.
>
> ISTM that 'evictions', 'extends', 'fsyncs', 'reads', 'reuses', 'writes' are
> clearer than the past tense versions, and about as consistent with existing
> columns.

I have updated the column names to the above recommendation.

On Wed, Jan 11, 2023 at 11:32 AM vignesh C <[email-masked]> wrote:
>
> For some reason cfbot is not able to apply this patch as in [1],
> please have a look and post an updated patch if required:
> === Applying patches on top of PostgreSQL commit ID
> 3c6fc58209f24b959ee18f5d19ef96403d08f15c ===
> === applying patch
> ./v45-0001-pgindent-and-some-manual-cleanup-in-pgstat-relat.patch
> patching file src/backend/storage/buffer/bufmgr.c
> patching file src/backend/storage/buffer/localbuf.c
> patching file src/backend/utils/activity/pgstat.c
> patching file src/backend/utils/activity/pgstat_relation.c
> patching file src/backend/utils/adt/pgstatfuncs.c
> patching file src/include/pgstat.h
> patching file src/include/utils/pgstat_internal.h
> === applying patch ./v45-0002-pgstat-Infrastructure-to-track-IO-operations.patch
> gpatch: **** Only garbage was found in the patch input.
>
> [1] - http://cfbot.cputube.org/patch_41_3272.log
>

This was an issue with cfbot that Thomas has now fixed as he describes
in [1].

> (I already sent patches to address the omission in cirrus.yml)

Thanks. I've fixed this
I make a tablespace in amcheck -- are there recommendations for naming
tablespaces in contrib also?

> => Do you want to put these in order?

Thanks. Fixed.

> pgstat_get_io_op_name() isn't currently being hit by tests; actually,

Deleted it.

> FlushRelationBuffers() isn't being hit for local buffers.

I added a test.

> I suggest: "One row for each combination of of .."

I have made this change.

> I would change this say "Postgres' own buffer cache is satisfying ..."

So, this is existing copy to which I added the pg_stat_io view name and
re-flowed the indentation.
However, I think your suggestions are a good idea, so I've taken them
and just rewritten this paragraph altogether.

> I suggest to refer to "the kernel's page cache"

same applies here.

> "..for each combination of .."

I have changed this.

> > +          <varname>io_context</varname> for a type of I/O operation. For

So I actually mean for a type of I/O operation -- that is, relation data
is normally written to a shared buffer but sometimes we bypass shared
buffers and just call write and sometimes we use a buffer access
strategy and write it to a special ring buffer (made up of buffers
stolen from shared buffers, but still). So I don't want to say "for I/O
operations" because I think that would imply that writes of relation
data will always be in the same IO Context.

> > +          buffers incurred while vacuuming and analyzing permanent relations.

I changed this.

> I don't think it's correct to say that it's "outside of" shared-buffers.

I suppose "outside of" gives the wrong idea. But I need to make clear
that this I/O is to and from buffers which are not a part of shared
buffers right now -- they may still be accessible from the same data
structures which access shared buffers but they are currently being used
in a different way.

> s/Qualifying/Certain/

I feel like qualifying is more specific than certain, but I would be open
to changing it if there was a specific reason you don't like it.

> "exclusive of temporary relations".

I've changed this.

> mean this is a "pure" number, and could be multipled by op_bytes to

It is the number of read operations of op_bytes size -- thanks so much
for pointing this out. The wording was really unclear.
The idea is that you can do something like:
SELECT pg_size_pretty(reads * op_bytes) FROM pg_stat_io;
and get it in bytes.

The view will contain other types of IO that are not in BLCKSZ chunks,
which is where this column will be handy.

> This never defines what "evicted" means.  Does it mea that a dirty

Thanks. I've updated this.

> Maybe say "as part of a bulk I/O operation (bulkread, bulkwrite, or

I've changed this.

> > +      indicate a misconfiguration of shared buffers or of checkpointer. More

I've made most of these changes.

> Should this link to various docs for checkpointer/bgwriter ?

I couldn't find docs related to tuning checkpointer outside of the WAL
configuration docs. There is the docs page for the CHECKPOINT command --
but I don't think that is very relevant here.

> should be included to the appendix.

I agree it would be nice to explain Buffer Access Strategies in the docs.

- Melanie

[1] https://www.postgresql.org/message-id/CA%2BhUKGLiY1e%2B1%3DpB7hXJOyGj1dJOfgde%2BHmiSnv3gDKayUFJMA%40mail.gmail.com
From 8960d4b3902374a999b3c7e572995d70b2cb0557 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Fri, 9 Dec 2022 18:23:19 -0800
Subject: [PATCH v46 1/5] pgindent and some manual cleanup in pgstat related
 code

---
 src/backend/storage/buffer/bufmgr.c          | 22 ++++++++++----------
 src/backend/storage/buffer/localbuf.c        |  4 ++--
 src/backend/utils/activity/pgstat.c          |  3 ++-
 src/backend/utils/activity/pgstat_relation.c |  1 +
 src/backend/utils/adt/pgstatfuncs.c          |  2 +-
 src/include/pgstat.h                         |  1 +
 src/include/utils/pgstat_internal.h          |  1 +
 7 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 3fb38a25cf..8075828e8a 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -516,7 +516,7 @@ PrefetchSharedBuffer(SMgrRelation smgr_reln,
 
 	/* create a tag so we can lookup the buffer */
 	InitBufferTag(&newTag, &smgr_reln->smgr_rlocator.locator,
-				   forkNum, blockNum);
+				  forkNum, blockNum);
 
 	/* determine its hash code and partition lock ID */
 	newHash = BufTableHashCode(&newTag);
@@ -3297,8 +3297,8 @@ DropRelationsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
 		uint32		buf_state;
 
 		/*
-		 * As in DropRelationBuffers, an unlocked precheck should be
-		 * safe and saves some cycles.
+		 * As in DropRelationBuffers, an unlocked precheck should be safe and
+		 * saves some cycles.
 		 */
 
 		if (!use_bsearch)
@@ -3425,8 +3425,8 @@ DropDatabaseBuffers(Oid dbid)
 		uint32		buf_state;
 
 		/*
-		 * As in DropRelationBuffers, an unlocked precheck should be
-		 * safe and saves some cycles.
+		 * As in DropRelationBuffers, an unlocked precheck should be safe and
+		 * saves some cycles.
 		 */
 		if (bufHdr->tag.dbOid != dbid)
 			continue;
@@ -3572,8 +3572,8 @@ FlushRelationBuffers(Relation rel)
 		bufHdr = GetBufferDescriptor(i);
 
 		/*
-		 * As in DropRelationBuffers, an unlocked precheck should be
-		 * safe and saves some cycles.
+		 * As in DropRelationBuffers, an unlocked precheck should be safe and
+		 * saves some cycles.
 		 */
 		if (!BufTagMatchesRelFileLocator(&bufHdr->tag, &rel->rd_locator))
 			continue;
@@ -3645,8 +3645,8 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 		uint32		buf_state;
 
 		/*
-		 * As in DropRelationBuffers, an unlocked precheck should be
-		 * safe and saves some cycles.
+		 * As in DropRelationBuffers, an unlocked precheck should be safe and
+		 * saves some cycles.
 		 */
 
 		if (!use_bsearch)
@@ -3880,8 +3880,8 @@ FlushDatabaseBuffers(Oid dbid)
 		bufHdr = GetBufferDescriptor(i);
 
 		/*
-		 * As in DropRelationBuffers, an unlocked precheck should be
-		 * safe and saves some cycles.
+		 * As in DropRelationBuffers, an unlocked precheck should be safe and
+		 * saves some cycles.
 		 */
 		if (bufHdr->tag.dbOid != dbid)
 			continue;
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index b2720df6ea..8372acc383 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -610,8 +610,8 @@ AtProcExit_LocalBuffers(void)
 {
 	/*
 	 * We shouldn't be holding any remaining pins; if we are, and assertions
-	 * aren't enabled, we'll fail later in DropRelationBuffers while
-	 * trying to drop the temp rels.
+	 * aren't enabled, we'll fail later in DropRelationBuffers while trying to
+	 * drop the temp rels.
 	 */
 	CheckForLocalBufferLeaks();
 }
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 7e9dc17e68..0fa5370bcd 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -426,7 +426,7 @@ pgstat_discard_stats(void)
 		ereport(DEBUG2,
 				(errcode_for_file_access(),
 				 errmsg_internal("unlinked permanent statistics file \"%s\"",
-						PGSTAT_STAT_PERMANENT_FILENAME)));
+								 PGSTAT_STAT_PERMANENT_FILENAME)));
 	}
 
 	/*
@@ -986,6 +986,7 @@ pgstat_build_snapshot(void)
 
 		entry->data = MemoryContextAlloc(pgStatLocal.snapshot.context,
 										 kind_info->shared_size);
+
 		/*
 		 * Acquire the LWLock directly instead of using
 		 * pg_stat_lock_entry_shared() which requires a reference.
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index 1730425de1..2e20b93c20 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -783,6 +783,7 @@ pgstat_relation_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	if (lstats->t_counts.t_numscans)
 	{
 		TimestampTz t = GetCurrentTransactionStopTimestamp();
+
 		if (t > tabentry->lastscan)
 			tabentry->lastscan = t;
 	}
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 6cddd74aa7..58bd1360b9 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -906,7 +906,7 @@ pg_stat_get_backend_client_addr(PG_FUNCTION_ARGS)
 	clean_ipv6_addr(beentry->st_clientaddr.addr.ss_family, remote_host);
 
 	PG_RETURN_DATUM(DirectFunctionCall1(inet_in,
-										 CStringGetDatum(remote_host)));
+										CStringGetDatum(remote_host)));
 }
 
 Datum
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index d3e965d744..5e3326a3b9 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -476,6 +476,7 @@ extern void pgstat_report_connect(Oid dboid);
 
 extern PgStat_StatDBEntry *pgstat_fetch_stat_dbentry(Oid dboid);
 
+
 /*
  * Functions in pgstat_function.c
  */
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 08412d6404..12fd51f1ae 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -626,6 +626,7 @@ extern void pgstat_wal_snapshot_cb(void);
 extern bool pgstat_subscription_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
 extern void pgstat_subscription_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts);
 
+
 /*
  * Functions in pgstat_xact.c
  */
-- 
2.34.1

From 6792d761ebf5e9261dab452130dcc2cd5ac48d0e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 9 Jan 2023 14:42:53 -0500
Subject: [PATCH v46 5/5] pg_stat_io documentation

Author: Melanie Plageman <[email-masked]>
Author: Samay Sharma <[email-masked]>
Reviewed-by: Maciek Sakrejda <[email-masked]>
Reviewed-by: Lukas Fittl <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Reviewed-by: Justin Pryzby <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 doc/src/sgml/monitoring.sgml | 318 +++++++++++++++++++++++++++++++++--
 1 file changed, 304 insertions(+), 14 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 8d51ca3773..12246d46f4 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -469,6 +469,16 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
       </entry>
      </row>
 
+     <row>
+      <entry><structname>pg_stat_io</structname><indexterm><primary>pg_stat_io</primary></indexterm></entry>
+      <entry>
+       One row for each combination of backend type, context, and target object
+       containing cluster-wide I/O statistics.
+       See <link linkend="monitoring-pg-stat-io-view">
+       <structname>pg_stat_io</structname></link> for details.
+     </entry>
+     </row>
+
      <row>
       <entry><structname>pg_stat_replication_slots</structname><indexterm><primary>pg_stat_replication_slots</primary></indexterm></entry>
       <entry>One row per replication slot, showing statistics about the
@@ -665,20 +675,16 @@ postgres   27093  0.0  0.0  30096  2752 ?        Ss   11:34   0:00 postgres: ser
   </para>
 
   <para>
-   The <structname>pg_statio_</structname> views are primarily useful to
-   determine the effectiveness of the buffer cache.  When the number
-   of actual disk reads is much smaller than the number of buffer
-   hits, then the cache is satisfying most read requests without
-   invoking a kernel call. However, these statistics do not give the
-   entire story: due to the way in which <productname>PostgreSQL</productname>
-   handles disk I/O, data that is not in the
-   <productname>PostgreSQL</productname> buffer cache might still reside in the
-   kernel's I/O cache, and might therefore still be fetched without
-   requiring a physical read. Users interested in obtaining more
-   detailed information on <productname>PostgreSQL</productname> I/O behavior are
-   advised to use the <productname>PostgreSQL</productname> statistics views
-   in combination with operating system utilities that allow insight
-   into the kernel's handling of I/O.
+   The <structname>pg_stat_io</structname> and
+   <structname>pg_statio_</structname> set of views are useful for determining
+   the effectiveness of the buffer cache. They can be used to calculate a cache
+   hit ratio. Note that while <productname>PostgreSQL</productname>'s I/O
+   statistics capture most instances in which the kernel was invoked in order
+   to perform I/O, they do not differentiate between data which had to be
+   fetched from disk and that which already resided in the kernel page cache.
+   Users are advised to use the <productname>PostgreSQL</productname>
+   statistics views in combination with operating system utilities for a more
+   complete picture of their database's I/O performance.
   </para>
 
  </sect2>
@@ -3643,6 +3649,290 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
     <structfield>last_archived_wal</structfield> have also been successfully
     archived.
   </para>
+ </sect2>
+
+ <sect2 id="monitoring-pg-stat-io-view">
+  <title><structname>pg_stat_io</structname></title>
+
+  <indexterm>
+   <primary>pg_stat_io</primary>
+  </indexterm>
+
+  <para>
+   The <structname>pg_stat_io</structname> view will contain one row for each
+   combination of backend type, I/O context, and target I/O object showing
+   cluster-wide I/O statistics. Combinations which do not make sense are
+   omitted.
+  </para>
+
+  <para>
+   Currently, I/O on relations (e.g. tables, indexes) is tracked. However,
+   relation I/O which bypasses shared buffers (e.g. when moving a table from one
+   tablespace to another) is currently not tracked.
+  </para>
+
+  <table id="pg-stat-io-view" xreflabel="pg_stat_io">
+   <title><structname>pg_stat_io</structname> View</title>
+   <tgroup cols="1">
+    <thead>
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        Column Type
+       </para>
+       <para>
+        Description
+       </para>
+      </entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>backend_type</structfield> <type>text</type>
+       </para>
+       <para>
+        Type of backend (e.g. background worker, autovacuum worker). See <link
+        linkend="monitoring-pg-stat-activity-view">
+        <structname>pg_stat_activity</structname></link> for more information
+        on <varname>backend_type</varname>s. Some
+        <varname>backend_type</varname>s do not accumulate I/O operation
+        statistics and will not be included in the view.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>io_context</structfield> <type>text</type>
+       </para>
+       <para>
+        The context of an I/O operation. Possible values are:
+       </para>
+       <itemizedlist>
+        <listitem>
+         <para>
+          <literal>normal</literal>: The default or standard
+          <varname>io_context</varname> for a type of I/O operation. For
+          example, by default, relation data is read into and written out from
+          shared buffers. Thus, reads and writes of relation data to and from
+          shared buffers are tracked in <varname>io_context</varname>
+          <literal>normal</literal>.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>vacuum</literal>: I/O operations performed outside of shared
+          buffers while vacuuming and analyzing permanent relations.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>bulkread</literal>: Qualifying large read I/O operations
+          done outside of shared buffers, for example, a sequential scan of a
+          large table.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>bulkwrite</literal>: Qualifying large write I/O operations
+          done outside of shared buffers, such as <command>COPY</command>.
+         </para>
+        </listitem>
+       </itemizedlist>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>io_object</structfield> <type>text</type>
+       </para>
+       <para>
+        Target object of an I/O operation. Possible values are:
+       <itemizedlist>
+        <listitem>
+         <para>
+          <literal>relation</literal>: Permanent relations.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>temp relation</literal>: Temporary relations.
+         </para>
+        </listitem>
+       </itemizedlist>
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>read</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of read operations of <varname>op_bytes</varname> size.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>written</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of write operations of <varname>op_bytes</varname> size.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>extended</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of relation extend operations of <varname>op_bytes</varname>
+        size.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>op_bytes</structfield> <type>bigint</type>
+       </para>
+       <para>
+        The number of bytes per unit of I/O read, written, or extended.
+       </para>
+       <para>
+        Relation data reads, writes, and extends are done in
+        <varname>block_size</varname> units, derived from the build-time
+        parameter <symbol>BLCKSZ</symbol>, which is <literal>8192</literal> by
+        default.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>evicted</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of times a block has been written out from a shared or local
+        buffer in order to make it available for another use.
+       </para>
+       <para>
+        In <varname>io_context</varname> <literal>normal</literal>, this counts
+        the number of times a block was evicted from a buffer and replaced with
+        another block. In <varname>io_context</varname>s
+        <literal>bulkwrite</literal>, <literal>bulkread</literal>, and
+        <literal>vacuum</literal>, this counts the number of times a block was
+        evicted from shared buffers in order to add the shared buffer to a
+        separate size-limited ring buffer for use in a bulk I/O operation.
+        </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>reused</structfield> <type>bigint</type>
+       </para>
+       <para>
+        The number of times an existing buffer in a size-limited ring buffer
+        outside of shared buffers was reused as part of an I/O operation in the
+        <literal>bulkread</literal>, <literal>bulkwrite</literal>, or
+        <literal>vacuum</literal> <varname>io_context</varname>s.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>files_synced</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of <literal>fsync</literal> calls. These are only tracked in
+        <varname>io_context</varname> <literal>normal</literal>.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>stats_reset</structfield> <type>timestamp with time zone</type>
+       </para>
+       <para>
+        Time at which these statistics were last reset.
+       </para>
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </table>
+
+  <para>
+   Some <varname>backend_type</varname>s never perform I/O operations in some
+   <varname>io_context</varname>s and/or on some <varname>io_object</varname>s.
+   These rows are omitted from the view. For example, the checkpointer does not
+   checkpoint temporary tables, so there will be no rows for
+   <varname>backend_type</varname> <literal>checkpointer</literal> and
+   <varname>io_object</varname> <literal>temp relation</literal>.
+  </para>
+
+  <para>
+   In addition, some I/O operations will never be performed either by certain
+   <varname>backend_type</varname>s or in certain
+   <varname>io_context</varname>s or on certain <varname>io_object</varname>s.
+   These cells will be NULL. For example, temporary tables are not
+   <literal>fsync</literal>ed, so <varname>files_synced</varname> will be NULL
+   for <varname>io_object</varname> <literal>temp relation</literal>. Also, the
+   background writer does not perform reads, so <varname>read</varname> will be
+   NULL in rows for <varname>backend_type</varname> <literal>background
+   writer</literal>.
+  </para>
+
+  <para>
+   <structname>pg_stat_io</structname> can be used to inform database tuning.
+   For example:
+   <itemizedlist>
+    <listitem>
+     <para>
+      A high <varname>evicted</varname> count can indicate that shared buffers
+      should be increased.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Client backends rely on the checkpointer to ensure data is persisted to
+      permanent storage. Large numbers of <varname>files_synced</varname> by
+      <literal>client backend</literal>s could indicate a misconfiguration of
+      shared buffers or of the checkpointer. More information on configuring
+      checkpointer can be found in <xref linkend="wal-configuration"/>.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Normally, client backends should be able to rely on auxiliary processes
+      like the checkpointer and the background writer to write out dirty data
+      as much as possible. Large numbers of writes by client backends could
+      indicate a misconfiguration of shared buffers or of the checkpointer.
+      More information on configuring the checkpointer can be found in <xref
+      linkend="wal-configuration"/>.
+     </para>
+    </listitem>
+   </itemizedlist>
+  </para>
+
 
  </sect2>
 
-- 
2.34.1

From 57c9a605e59175ad4ac8857e40ff9cddc7909812 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 9 Jan 2023 14:42:25 -0500
Subject: [PATCH v46 4/5] Add system view tracking IO ops per backend type

Add pg_stat_io, a system view which tracks the number of IOOps
(evictions, reuses, reads, writes, extensions, and fsyncs) done on each
IOObject (relation, temp relation) in each IOContext ("normal" and those
using a BufferAccessStrategy) by each type of backend (e.g. client
backend, checkpointer).

Some BackendTypes do not accumulate IO operations statistics and will
not be included in the view.

Some IOContexts are not used by some BackendTypes and will not be in the
view. For example, checkpointer does not use a BufferAccessStrategy
(currently), so there will be no rows for BufferAccessStrategy
IOContexts for checkpointer.

Some IOObjects are never operated on in some IOContexts or by some
BackendTypes. These rows are omitted from the view. For example,
checkpointer will never operate on IOOBJECT_TEMP_RELATION data, so those
rows are omitted.

Some IOOps are invalid in combination with certain IOContexts and
certain IOObjects. Those cells will be NULL in the view to distinguish
between 0 observed IOOps of that type and an invalid combination. For
example, temporary tables are not fsynced so cells for all BackendTypes
for IOOBJECT_TEMP_RELATION and IOOP_FSYNC will be NULL.

Some BackendTypes never perform certain IOOps. Those cells will also be
NULL in the view. For example, bgwriter should not perform reads.

View stats are populated with statistics incremented when a backend
performs an IO Operation and maintained by the cumulative statistics
subsystem.

Each row of the view shows stats for a particular BackendType, IOObject,
IOContext combination (e.g. a client backend's operations on permanent
relations in shared buffers) and each column in the view is the total
number of IO Operations done (e.g. writes). So a cell in the view would
be, for example, the number of blocks of relation data written from
shared buffers by client backends since the last stats reset.

In anticipation of tracking WAL IO and non-block-oriented IO (such as
temporary file IO), the "op_bytes" column specifies the unit of the
"reads", "writes", and "extends" columns for a given row.

Note that some of the cells in the view are redundant with fields in
pg_stat_bgwriter (e.g. buffers_backend), however these have been kept in
pg_stat_bgwriter for backwards compatibility. Deriving the redundant
pg_stat_bgwriter stats from the IO operations stats structures was also
problematic due to the separate reset targets for 'bgwriter' and 'io'.

Suggested by Andres Freund

Author: Melanie Plageman <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 contrib/amcheck/expected/check_heap.out |  31 +++
 contrib/amcheck/sql/check_heap.sql      |  24 +++
 src/backend/catalog/system_views.sql    |  15 ++
 src/backend/utils/adt/pgstatfuncs.c     | 154 +++++++++++++++
 src/include/catalog/pg_proc.dat         |   9 +
 src/test/regress/expected/rules.out     |  12 ++
 src/test/regress/expected/stats.out     | 246 ++++++++++++++++++++++++
 src/test/regress/sql/stats.sql          | 150 +++++++++++++++
 src/tools/pgindent/typedefs.list        |   1 +
 9 files changed, 642 insertions(+)

diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out
index c010361025..2cf63302d5 100644
--- a/contrib/amcheck/expected/check_heap.out
+++ b/contrib/amcheck/expected/check_heap.out
@@ -66,6 +66,19 @@ SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'ALL-VISIBLE');
 INSERT INTO heaptest (a, b)
 	(SELECT gs, repeat('x', gs)
 		FROM generate_series(1,50) gs);
+-- pg_stat_io test:
+-- verify_heapam always uses a BAS_BULKREAD BufferAccessStrategy. This allows
+-- us to reliably test that pg_stat_io BULKREAD reads are being captured
+-- without relying on the size of shared buffers or on an expensive operation
+-- like CREATE DATABASE.
+--
+-- Create an alternative tablespace and move the heaptest table to it, causing
+-- it to be rewritten.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE test_stats LOCATION '';
+SELECT sum(reads) AS stats_bulkreads_before
+  FROM pg_stat_io WHERE io_context = 'bulkread' \gset
+ALTER TABLE heaptest SET TABLESPACE test_stats;
 -- Check that valid options are not rejected nor corruption reported
 -- for a non-empty table
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'none');
@@ -88,6 +101,23 @@ SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 0, endblock :=
 -------+--------+--------+-----
 (0 rows)
 
+-- verify_heapam should have read in the page written out by
+--   ALTER TABLE ... SET TABLESPACE ...
+-- causing an additional bulkread, which should be reflected in pg_stat_io.
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(reads) AS stats_bulkreads_after
+  FROM pg_stat_io WHERE io_context = 'bulkread' \gset
+SELECT :stats_bulkreads_after > :stats_bulkreads_before;
+ ?column? 
+----------
+ t
+(1 row)
+
 CREATE ROLE regress_heaptest_role;
 -- verify permissions are checked (error due to function not callable)
 SET ROLE regress_heaptest_role;
@@ -195,6 +225,7 @@ ERROR:  cannot check relation "test_foreign_table"
 DETAIL:  This operation is not supported for foreign tables.
 -- cleanup
 DROP TABLE heaptest;
+DROP TABLESPACE test_stats;
 DROP TABLE test_partition;
 DROP TABLE test_partitioned;
 DROP OWNED BY regress_heaptest_role; -- permissions
diff --git a/contrib/amcheck/sql/check_heap.sql b/contrib/amcheck/sql/check_heap.sql
index 298de6886a..a8e9c3c1e6 100644
--- a/contrib/amcheck/sql/check_heap.sql
+++ b/contrib/amcheck/sql/check_heap.sql
@@ -20,11 +20,26 @@ SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'NONE');
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'ALL-FROZEN');
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'ALL-VISIBLE');
 
+
 -- Add some data so subsequent tests are not entirely trivial
 INSERT INTO heaptest (a, b)
 	(SELECT gs, repeat('x', gs)
 		FROM generate_series(1,50) gs);
 
+-- pg_stat_io test:
+-- verify_heapam always uses a BAS_BULKREAD BufferAccessStrategy. This allows
+-- us to reliably test that pg_stat_io BULKREAD reads are being captured
+-- without relying on the size of shared buffers or on an expensive operation
+-- like CREATE DATABASE.
+--
+-- Create an alternative tablespace and move the heaptest table to it, causing
+-- it to be rewritten.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE test_stats LOCATION '';
+SELECT sum(reads) AS stats_bulkreads_before
+  FROM pg_stat_io WHERE io_context = 'bulkread' \gset
+ALTER TABLE heaptest SET TABLESPACE test_stats;
+
 -- Check that valid options are not rejected nor corruption reported
 -- for a non-empty table
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'none');
@@ -32,6 +47,14 @@ SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'all-frozen');
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'all-visible');
 SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 0, endblock := 0);
 
+-- verify_heapam should have read in the page written out by
+--   ALTER TABLE ... SET TABLESPACE ...
+-- causing an additional bulkread, which should be reflected in pg_stat_io.
+SELECT pg_stat_force_next_flush();
+SELECT sum(reads) AS stats_bulkreads_after
+  FROM pg_stat_io WHERE io_context = 'bulkread' \gset
+SELECT :stats_bulkreads_after > :stats_bulkreads_before;
+
 CREATE ROLE regress_heaptest_role;
 
 -- verify permissions are checked (error due to function not callable)
@@ -110,6 +133,7 @@ SELECT * FROM verify_heapam('test_foreign_table',
 
 -- cleanup
 DROP TABLE heaptest;
+DROP TABLESPACE test_stats;
 DROP TABLE test_partition;
 DROP TABLE test_partitioned;
 DROP OWNED BY regress_heaptest_role; -- permissions
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 447c9b970f..494a8791a5 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1117,6 +1117,21 @@ CREATE VIEW pg_stat_bgwriter AS
         pg_stat_get_buf_alloc() AS buffers_alloc,
         pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
 
+CREATE VIEW pg_stat_io AS
+SELECT
+       b.backend_type,
+       b.io_context,
+       b.io_object,
+       b.reads,
+       b.writes,
+       b.extends,
+       b.op_bytes,
+       b.evictions,
+       b.reuses,
+       b.fsyncs,
+       b.stats_reset
+FROM pg_stat_get_io() b;
+
 CREATE VIEW pg_stat_wal AS
     SELECT
         w.wal_records,
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 5c8e8336bf..51b941d4c8 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1234,6 +1234,160 @@ pg_stat_get_buf_alloc(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_bgwriter()->buf_alloc);
 }
 
+/*
+* When adding a new column to the pg_stat_io view, add a new enum value
+* here above IO_NUM_COLUMNS.
+*/
+typedef enum io_stat_col
+{
+	IO_COL_BACKEND_TYPE,
+	IO_COL_IO_CONTEXT,
+	IO_COL_IO_OBJECT,
+	IO_COL_READS,
+	IO_COL_WRITES,
+	IO_COL_EXTENDS,
+	IO_COL_CONVERSION,
+	IO_COL_EVICTIONS,
+	IO_COL_REUSES,
+	IO_COL_FSYNCS,
+	IO_COL_RESET_TIME,
+	IO_NUM_COLUMNS,
+} io_stat_col;
+
+/*
+ * When adding a new IOOp, add a new io_stat_col and add a case to this
+ * function returning the corresponding io_stat_col.
+ */
+static io_stat_col
+pgstat_get_io_op_index(IOOp io_op)
+{
+	switch (io_op)
+	{
+		case IOOP_EVICT:
+			return IO_COL_EVICTIONS;
+		case IOOP_READ:
+			return IO_COL_READS;
+		case IOOP_REUSE:
+			return IO_COL_REUSES;
+		case IOOP_WRITE:
+			return IO_COL_WRITES;
+		case IOOP_EXTEND:
+			return IO_COL_EXTENDS;
+		case IOOP_FSYNC:
+			return IO_COL_FSYNCS;
+	}
+
+	elog(ERROR, "unrecognized IOOp value: %d", io_op);
+	pg_unreachable();
+}
+
+#ifdef USE_ASSERT_CHECKING
+static bool
+pgstat_iszero_io_object(const PgStat_Counter *obj)
+{
+	for (IOOp io_op = IOOP_EVICT; io_op < IOOP_NUM_TYPES; io_op++)
+	{
+		if (obj[io_op] != 0)
+			return false;
+	}
+
+	return true;
+}
+#endif
+
+Datum
+pg_stat_get_io(PG_FUNCTION_ARGS)
+{
+	ReturnSetInfo *rsinfo;
+	PgStat_IO  *backends_io_stats;
+	Datum		reset_time;
+
+	InitMaterializedSRF(fcinfo, 0);
+	rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+	backends_io_stats = pgstat_fetch_stat_io();
+
+	reset_time = TimestampTzGetDatum(backends_io_stats->stat_reset_timestamp);
+
+	for (BackendType bktype = B_INVALID; bktype < BACKEND_NUM_TYPES; bktype++)
+	{
+		bool		bktype_tracked;
+		Datum		bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
+		PgStat_BackendIO *bktype_stats = &backends_io_stats->stats[bktype];
+
+		/*
+		 * For those BackendTypes without IO Operation stats, skip
+		 * representing them in the view altogether. We still loop through
+		 * their counters so that we can assert that all values are zero.
+		 */
+		bktype_tracked = pgstat_tracks_io_bktype(bktype);
+
+		for (IOContext io_context = IOCONTEXT_BULKREAD;
+			 io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		{
+			const char *context_name = pgstat_get_io_context_name(io_context);
+
+			for (IOObject io_obj = IOOBJECT_RELATION;
+				 io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+			{
+				const char *obj_name = pgstat_get_io_object_name(io_obj);
+
+				Datum		values[IO_NUM_COLUMNS] = {0};
+				bool		nulls[IO_NUM_COLUMNS] = {0};
+
+				/*
+				 * Some combinations of IOContext, IOObject, and BackendType
+				 * are not valid for any type of IOOp. In such cases, omit the
+				 * entire row from the view.
+				 */
+				if (!bktype_tracked ||
+					!pgstat_tracks_io_object(bktype, io_context, io_obj))
+				{
+					Assert(pgstat_iszero_io_object(bktype_stats->data[io_context][io_obj]));
+					continue;
+				}
+
+				values[IO_COL_BACKEND_TYPE] = bktype_desc;
+				values[IO_COL_IO_CONTEXT] = CStringGetTextDatum(context_name);
+				values[IO_COL_IO_OBJECT] = CStringGetTextDatum(obj_name);
+				values[IO_COL_RESET_TIME] = TimestampTzGetDatum(reset_time);
+
+				/*
+				 * Hard-code this to the value of BLCKSZ for now. Future
+				 * values could include XLOG_BLCKSZ, once WAL IO is tracked,
+				 * and constant multipliers, once non-block-oriented IO (e.g.
+				 * temporary file IO) is tracked.
+				 */
+				values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
+
+				/*
+				 * Some combinations of BackendType and IOOp, of IOContext and
+				 * IOOp, and of IOObject and IOOp are not tracked. Set these
+				 * cells in the view NULL and assert that these stats are zero
+				 * as expected.
+				 */
+				for (IOOp io_op = IOOP_EVICT; io_op < IOOP_NUM_TYPES; io_op++)
+				{
+					int			col_idx = pgstat_get_io_op_index(io_op);
+
+					nulls[col_idx] = !pgstat_tracks_io_op(bktype, io_context, io_obj, io_op);
+
+					if (!nulls[col_idx])
+						values[col_idx] =
+							Int64GetDatum(bktype_stats->data[io_context][io_obj][io_op]);
+					else
+						Assert(bktype_stats->data[io_context][io_obj][io_op] == 0);
+				}
+
+				tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
+									 values, nulls);
+			}
+		}
+	}
+
+	return (Datum) 0;
+}
+
 /*
  * Returns statistics of WAL activity
  */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 3810de7b22..57a889cf49 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5690,6 +5690,15 @@
   proname => 'pg_stat_get_buf_alloc', provolatile => 's', proparallel => 'r',
   prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_buf_alloc' },
 
+{ oid => '8459', descr => 'statistics: per backend type IO statistics',
+  proname => 'pg_stat_get_io', provolatile => 'v',
+  prorows => '30', proretset => 't',
+  proparallel => 'r', prorettype => 'record', proargtypes => '',
+  proallargtypes => '{text,text,text,int8,int8,int8,int8,int8,int8,int8,timestamptz}',
+  proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{backend_type,io_context,io_object,reads,writes,extends,op_bytes,evictions,reuses,fsyncs,stats_reset}',
+  prosrc => 'pg_stat_get_io' },
+
 { oid => '1136', descr => 'statistics: information about WAL activity',
   proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
   proparallel => 'r', prorettype => 'record', proargtypes => '',
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index fb9f936d43..6ae7882864 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1876,6 +1876,18 @@ pg_stat_gssapi| SELECT s.pid,
     s.gss_enc AS encrypted
    FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id)
   WHERE (s.client_port IS NOT NULL);
+pg_stat_io| SELECT b.backend_type,
+    b.io_context,
+    b.io_object,
+    b.reads,
+    b.writes,
+    b.extends,
+    b.op_bytes,
+    b.evictions,
+    b.reuses,
+    b.fsyncs,
+    b.stats_reset
+   FROM pg_stat_get_io() b(backend_type, io_context, io_object, reads, writes, extends, op_bytes, evictions, reuses, fsyncs, stats_reset);
 pg_stat_progress_analyze| SELECT s.pid,
     s.datid,
     d.datname,
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 1d84407a03..a66fe86b05 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1126,4 +1126,250 @@ SELECT pg_stat_get_subscription_stats(NULL);
  
 (1 row)
 
+-- Test that the following operations are tracked in pg_stat_io:
+-- - reads of target blocks into shared buffers
+-- - writes of shared buffers to permanent storage
+-- - extends of relations using shared buffers
+-- - fsyncs done to ensure the durability of data dirtying shared buffers
+-- There is no test for blocks evicted from shared buffers, because we cannot
+-- be sure of the state of shared buffers at the point the test is run.
+-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
+-- extends.
+SELECT sum(extends) AS io_sum_shared_extends_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+CREATE TABLE test_io_shared(a int);
+INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(extends) AS io_sum_shared_extends_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_extends_after > :io_sum_shared_extends_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
+-- and fsyncs.
+-- The second checkpoint ensures that stats from the first checkpoint have been
+-- reported and protects against any potential races amongst the table
+-- creation, a possible timing-triggered checkpoint, and the explicit
+-- checkpoint in the test.
+SELECT sum(writes) AS io_sum_shared_writes_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+SELECT sum(fsyncs) AS io_sum_shared_fsyncs_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+CHECKPOINT;
+CHECKPOINT;
+SELECT sum(writes) AS io_sum_shared_writes_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT sum(fsyncs) AS io_sum_shared_fsyncs_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_writes_after > :io_sum_shared_writes_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT current_setting('fsync') = 'off' OR :io_sum_shared_fsyncs_after > :io_sum_shared_fsyncs_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Change the tablespace so that the table is rewritten directly, then SELECT
+-- from it to cause it to be read back into shared buffers.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_io_stats_tblspc LOCATION '';
+SELECT sum(reads) AS io_sum_shared_reads_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+ALTER TABLE test_io_shared SET TABLESPACE regress_io_stats_tblspc;
+-- SELECT from the table so that the data is read into shared buffers and
+-- io_context 'normal', io_object 'relation' reads are counted.
+SELECT COUNT(*) FROM test_io_shared;
+ count 
+-------
+   100
+(1 row)
+
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(reads) AS io_sum_shared_reads_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_reads_after > :io_sum_shared_reads_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_shared;
+-- Test that the follow IOCONTEXT_LOCAL IOOps are tracked in pg_stat_io:
+-- - eviction of local buffers in order to reuse them
+-- - reads of temporary table blocks into local buffers
+-- - writes of local buffers to permanent storage
+-- - extends of temporary tables
+-- Set temp_buffers to a low value so that we can trigger writes with fewer
+-- inserted tuples. Do so in a new session in case temporary tables have been
+-- accessed by previous tests in this session.
+\c
+SET temp_buffers TO '1MB';
+CREATE TEMPORARY TABLE test_io_local(a int, b TEXT);
+SELECT sum(extends) AS io_sum_local_extends_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+SELECT sum(evictions) AS io_sum_local_evictions_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+SELECT sum(writes) AS io_sum_local_writes_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+-- Insert tuples into the temporary table, generating extends in the stats.
+-- Insert enough values that we need to reuse and write out dirty local
+-- buffers, generating evictions and writes.
+INSERT INTO test_io_local SELECT generate_series(1, 8000) as id, repeat('a', 100);
+SELECT sum(reads) AS io_sum_local_reads_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+-- Read in evicted buffers, generating reads.
+SELECT COUNT(*) FROM test_io_local;
+ count 
+-------
+  8000
+(1 row)
+
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(evictions) AS io_sum_local_evictions_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT sum(reads) AS io_sum_local_reads_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT sum(writes) AS io_sum_local_writes_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT sum(extends) AS io_sum_local_extends_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT :io_sum_local_evictions_after > :io_sum_local_evictions_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :io_sum_local_reads_after > :io_sum_local_reads_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :io_sum_local_writes_after > :io_sum_local_writes_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :io_sum_local_extends_after > :io_sum_local_extends_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Change the tablespaces so that the temporary table is rewritten to other
+-- local buffers, exercising a different codepath than standard local buffer
+-- writes.
+ALTER TABLE test_io_local SET TABLESPACE regress_io_stats_tblspc;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(writes) AS io_sum_local_writes_new_tblspc
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT :io_sum_local_writes_new_tblspc > :io_sum_local_writes_after;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_local;
+RESET temp_buffers;
+DROP TABLESPACE regress_io_stats_tblspc;
+-- Test that reuse of strategy buffers and reads of blocks into these reused
+-- buffers while VACUUMing are tracked in pg_stat_io.
+-- Set wal_skip_threshold smaller than the expected size of
+-- test_io_vac_strategy so that, even if wal_level is minimal, VACUUM FULL will
+-- fsync the newly rewritten test_io_vac_strategy instead of writing it to WAL.
+-- Writing it to WAL will result in the newly written relation pages being in
+-- shared buffers -- preventing us from testing BAS_VACUUM BufferAccessStrategy
+-- reads.
+SET wal_skip_threshold = '1 kB';
+SELECT sum(reuses) AS io_sum_vac_strategy_reuses_before FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+SELECT sum(reads) AS io_sum_vac_strategy_reads_before FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+CREATE TABLE test_io_vac_strategy(a int, b int) WITH (autovacuum_enabled = 'false');
+INSERT INTO test_io_vac_strategy SELECT i, i from generate_series(1, 8000)i;
+-- Ensure that the next VACUUM will need to perform IO by rewriting the table
+-- first with VACUUM (FULL).
+VACUUM (FULL) test_io_vac_strategy;
+VACUUM (PARALLEL 0) test_io_vac_strategy;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(reuses) AS io_sum_vac_strategy_reuses_after FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+SELECT sum(reads) AS io_sum_vac_strategy_reads_after FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+SELECT :io_sum_vac_strategy_reads_after > :io_sum_vac_strategy_reads_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT :io_sum_vac_strategy_reuses_after > :io_sum_vac_strategy_reuses_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+RESET wal_skip_threshold;
+-- Test that extends done by a CTAS, which uses a BAS_BULKWRITE
+-- BufferAccessStrategy, are tracked in pg_stat_io.
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_before FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+CREATE TABLE test_io_bulkwrite_strategy AS SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_after FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_extends_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Test IO stats reset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_pre_reset FROM pg_stat_io \gset
+SELECT pg_stat_reset_shared('io');
+ pg_stat_reset_shared 
+----------------------
+ 
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_post_reset FROM pg_stat_io \gset
+SELECT :io_stats_post_reset < :io_stats_pre_reset;
+ ?column? 
+----------
+ t
+(1 row)
+
 -- End of Stats Test
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index b4d6753c71..64b3da2765 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -536,4 +536,154 @@ SELECT pg_stat_get_replication_slot(NULL);
 SELECT pg_stat_get_subscription_stats(NULL);

+-- Test that the following operations are tracked in pg_stat_io:
+-- - reads of target blocks into shared buffers
+-- - writes of shared buffers to permanent storage
+-- - extends of relations using shared buffers
+-- - fsyncs done to ensure the durability of data dirtying shared buffers
+
+-- There is no test for blocks evicted from shared buffers, because we cannot
+-- be sure of the state of shared buffers at the point the test is run.
+
+-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
+-- extends.
+SELECT sum(extends) AS io_sum_shared_extends_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+CREATE TABLE test_io_shared(a int);
+INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+SELECT sum(extends) AS io_sum_shared_extends_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_extends_after > :io_sum_shared_extends_before;
+
+-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
+-- and fsyncs.
+-- The second checkpoint ensures that stats from the first checkpoint have been
+-- reported and protects against any potential races amongst the table
+-- creation, a possible timing-triggered checkpoint, and the explicit
+-- checkpoint in the test.
+SELECT sum(writes) AS io_sum_shared_writes_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+SELECT sum(fsyncs) AS io_sum_shared_fsyncs_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+CHECKPOINT;
+CHECKPOINT;
+SELECT sum(writes) AS io_sum_shared_writes_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT sum(fsyncs) AS io_sum_shared_fsyncs_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+
+SELECT :io_sum_shared_writes_after > :io_sum_shared_writes_before;
+SELECT current_setting('fsync') = 'off' OR :io_sum_shared_fsyncs_after > :io_sum_shared_fsyncs_before;
+
+-- Change the tablespace so that the table is rewritten directly, then SELECT
+-- from it to cause it to be read back into shared buffers.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_io_stats_tblspc LOCATION '';
+SELECT sum(reads) AS io_sum_shared_reads_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+ALTER TABLE test_io_shared SET TABLESPACE regress_io_stats_tblspc;
+-- SELECT from the table so that the data is read into shared buffers and
+-- io_context 'normal', io_object 'relation' reads are counted.
+SELECT COUNT(*) FROM test_io_shared;
+SELECT pg_stat_force_next_flush();
+SELECT sum(reads) AS io_sum_shared_reads_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_reads_after > :io_sum_shared_reads_before;
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_shared;
+
+-- Test that the follow IOCONTEXT_LOCAL IOOps are tracked in pg_stat_io:
+-- - eviction of local buffers in order to reuse them
+-- - reads of temporary table blocks into local buffers
+-- - writes of local buffers to permanent storage
+-- - extends of temporary tables
+
+-- Set temp_buffers to a low value so that we can trigger writes with fewer
+-- inserted tuples. Do so in a new session in case temporary tables have been
+-- accessed by previous tests in this session.
+\c
+SET temp_buffers TO '1MB';
+CREATE TEMPORARY TABLE test_io_local(a int, b TEXT);
+SELECT sum(extends) AS io_sum_local_extends_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+SELECT sum(evictions) AS io_sum_local_evictions_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+SELECT sum(writes) AS io_sum_local_writes_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+-- Insert tuples into the temporary table, generating extends in the stats.
+-- Insert enough values that we need to reuse and write out dirty local
+-- buffers, generating evictions and writes.
+INSERT INTO test_io_local SELECT generate_series(1, 8000) as id, repeat('a', 100);
+
+SELECT sum(reads) AS io_sum_local_reads_before
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+-- Read in evicted buffers, generating reads.
+SELECT COUNT(*) FROM test_io_local;
+SELECT pg_stat_force_next_flush();
+SELECT sum(evictions) AS io_sum_local_evictions_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT sum(reads) AS io_sum_local_reads_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT sum(writes) AS io_sum_local_writes_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT sum(extends) AS io_sum_local_extends_after
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT :io_sum_local_evictions_after > :io_sum_local_evictions_before;
+SELECT :io_sum_local_reads_after > :io_sum_local_reads_before;
+SELECT :io_sum_local_writes_after > :io_sum_local_writes_before;
+SELECT :io_sum_local_extends_after > :io_sum_local_extends_before;
+
+-- Change the tablespaces so that the temporary table is rewritten to other
+-- local buffers, exercising a different codepath than standard local buffer
+-- writes.
+ALTER TABLE test_io_local SET TABLESPACE regress_io_stats_tblspc;
+SELECT pg_stat_force_next_flush();
+SELECT sum(writes) AS io_sum_local_writes_new_tblspc
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT :io_sum_local_writes_new_tblspc > :io_sum_local_writes_after;
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_local;
+RESET temp_buffers;
+DROP TABLESPACE regress_io_stats_tblspc;
+
+-- Test that reuse of strategy buffers and reads of blocks into these reused
+-- buffers while VACUUMing are tracked in pg_stat_io.
+
+-- Set wal_skip_threshold smaller than the expected size of
+-- test_io_vac_strategy so that, even if wal_level is minimal, VACUUM FULL will
+-- fsync the newly rewritten test_io_vac_strategy instead of writing it to WAL.
+-- Writing it to WAL will result in the newly written relation pages being in
+-- shared buffers -- preventing us from testing BAS_VACUUM BufferAccessStrategy
+-- reads.
+SET wal_skip_threshold = '1 kB';
+SELECT sum(reuses) AS io_sum_vac_strategy_reuses_before FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+SELECT sum(reads) AS io_sum_vac_strategy_reads_before FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+CREATE TABLE test_io_vac_strategy(a int, b int) WITH (autovacuum_enabled = 'false');
+INSERT INTO test_io_vac_strategy SELECT i, i from generate_series(1, 8000)i;
+-- Ensure that the next VACUUM will need to perform IO by rewriting the table
+-- first with VACUUM (FULL).
+VACUUM (FULL) test_io_vac_strategy;
+VACUUM (PARALLEL 0) test_io_vac_strategy;
+SELECT pg_stat_force_next_flush();
+SELECT sum(reuses) AS io_sum_vac_strategy_reuses_after FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+SELECT sum(reads) AS io_sum_vac_strategy_reads_after FROM pg_stat_io WHERE io_context = 'vacuum' \gset
+SELECT :io_sum_vac_strategy_reads_after > :io_sum_vac_strategy_reads_before;
+SELECT :io_sum_vac_strategy_reuses_after > :io_sum_vac_strategy_reuses_before;
+RESET wal_skip_threshold;
+
+-- Test that extends done by a CTAS, which uses a BAS_BULKWRITE
+-- BufferAccessStrategy, are tracked in pg_stat_io.
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_before FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+CREATE TABLE test_io_bulkwrite_strategy AS SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_after FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_extends_before;
+
+-- Test IO stats reset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_pre_reset FROM pg_stat_io \gset
+SELECT pg_stat_reset_shared('io');
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_post_reset FROM pg_stat_io \gset
+SELECT :io_stats_post_reset < :io_stats_pre_reset;
+
 -- End of Stats Test
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 7b66b1bc89..c4ecef2bf8 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3377,6 +3377,7 @@ intset_internal_node
 intset_leaf_node
 intset_node
 intvKEY
+io_stat_col
 itemIdCompact
 itemIdCompactData
 iterator
-- 
2.34.1

From 89ab692edfcf2af9e81fe6a45c3cad63158c8e20 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 17:20:50 -0500
Subject: [PATCH v46 3/5] pgstat: Count IO for relations

Count IOOps done on IOObjects in IOContexts by various BackendTypes
using the IO stats infrastructure introduced by a previous commit.

The primary concern of these statistics is IO operations on data blocks
during the course of normal database operations. IO operations done by,
for example, the archiver or syslogger are not counted in these
statistics. WAL IO, temporary file IO, and IO done directly though smgr*
functions (such as when building an index) are not yet counted but would
be useful future additions.

Author: Melanie Plageman <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 src/backend/storage/buffer/bufmgr.c   | 109 ++++++++++++++++++++++----
 src/backend/storage/buffer/freelist.c |  58 ++++++++++----
 src/backend/storage/buffer/localbuf.c |  13 ++-
 src/backend/storage/smgr/md.c         |  25 ++++++
 src/include/storage/buf_internals.h   |   8 +-
 src/include/storage/bufmgr.h          |   7 +-
 6 files changed, 184 insertions(+), 36 deletions(-)

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 8075828e8a..47d2b4c522 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -481,8 +481,9 @@ static BufferDesc *BufferAlloc(SMgrRelation smgr,
 							   ForkNumber forkNum,
 							   BlockNumber blockNum,
 							   BufferAccessStrategy strategy,
-							   bool *foundPtr);
-static void FlushBuffer(BufferDesc *buf, SMgrRelation reln);
+							   bool *foundPtr, IOContext *io_context);
+static void FlushBuffer(BufferDesc *buf, SMgrRelation reln,
+						IOContext io_context, IOObject io_object);
 static void FindAndDropRelationBuffers(RelFileLocator rlocator,
 									   ForkNumber forkNum,
 									   BlockNumber nForkBlock,
@@ -823,6 +824,8 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 	BufferDesc *bufHdr;
 	Block		bufBlock;
 	bool		found;
+	IOContext	io_context;
+	IOObject	io_object;
 	bool		isExtend;
 	bool		isLocalBuf = SmgrIsTemp(smgr);
 
@@ -855,7 +858,14 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 
 	if (isLocalBuf)
 	{
-		bufHdr = LocalBufferAlloc(smgr, forkNum, blockNum, &found);
+		/*
+		 * LocalBufferAlloc() will set the io_context to IOCONTEXT_NORMAL. We
+		 * do not use a BufferAccessStrategy for I/O of temporary tables.
+		 * However, in some cases, the "strategy" may not be NULL, so we can't
+		 * rely on IOContextForStrategy() to set the right IOContext for us.
+		 * This may happen in cases like CREATE TEMPORARY TABLE AS...
+		 */
+		bufHdr = LocalBufferAlloc(smgr, forkNum, blockNum, &found, &io_context);
 		if (found)
 			pgBufferUsage.local_blks_hit++;
 		else if (isExtend)
@@ -871,7 +881,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 		 * not currently in memory.
 		 */
 		bufHdr = BufferAlloc(smgr, relpersistence, forkNum, blockNum,
-							 strategy, &found);
+							 strategy, &found, &io_context);
 		if (found)
 			pgBufferUsage.shared_blks_hit++;
 		else if (isExtend)
@@ -986,7 +996,16 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 	 */
 	Assert(!(pg_atomic_read_u32(&bufHdr->state) & BM_VALID));	/* spinlock not needed */
 
-	bufBlock = isLocalBuf ? LocalBufHdrGetBlock(bufHdr) : BufHdrGetBlock(bufHdr);
+	if (isLocalBuf)
+	{
+		bufBlock = LocalBufHdrGetBlock(bufHdr);
+		io_object = IOOBJECT_TEMP_RELATION;
+	}
+	else
+	{
+		bufBlock = BufHdrGetBlock(bufHdr);
+		io_object = IOOBJECT_RELATION;
+	}
 
 	if (isExtend)
 	{
@@ -995,6 +1014,8 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 		/* don't set checksum for all-zero page */
 		smgrextend(smgr, forkNum, blockNum, (char *) bufBlock, false);
 
+		pgstat_count_io_op(IOOP_EXTEND, io_object, io_context);
+
 		/*
 		 * NB: we're *not* doing a ScheduleBufferTagForWriteback here;
 		 * although we're essentially performing a write. At least on linux
@@ -1020,6 +1041,8 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 
 			smgrread(smgr, forkNum, blockNum, (char *) bufBlock);
 
+			pgstat_count_io_op(IOOP_READ, io_object, io_context);
+
 			if (track_io_timing)
 			{
 				INSTR_TIME_SET_CURRENT(io_time);
@@ -1113,14 +1136,19 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
  * *foundPtr is actually redundant with the buffer's BM_VALID flag, but
  * we keep it for simplicity in ReadBuffer.
  *
+ * io_context is passed as an output parameter to avoid calling
+ * IOContextForStrategy() when there is a shared buffers hit and no IO
+ * statistics need be captured.
+ *
  * No locks are held either at entry or exit.
  */
 static BufferDesc *
 BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 			BlockNumber blockNum,
 			BufferAccessStrategy strategy,
-			bool *foundPtr)
+			bool *foundPtr, IOContext *io_context)
 {
+	bool		from_ring;
 	BufferTag	newTag;			/* identity of requested block */
 	uint32		newHash;		/* hash value for newTag */
 	LWLock	   *newPartitionLock;	/* buffer partition lock for it */
@@ -1172,8 +1200,11 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 			{
 				/*
 				 * If we get here, previous attempts to read the buffer must
-				 * have failed ... but we shall bravely try again.
+				 * have failed ... but we shall bravely try again. Set
+				 * io_context since we will in fact need to count an IO
+				 * Operation.
 				 */
+				*io_context = IOContextForStrategy(strategy);
 				*foundPtr = false;
 			}
 		}
@@ -1187,6 +1218,8 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 	 */
 	LWLockRelease(newPartitionLock);
 
+	*io_context = IOContextForStrategy(strategy);
+
 	/* Loop here in case we have to try another victim buffer */
 	for (;;)
 	{
@@ -1200,7 +1233,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 		 * Select a victim buffer.  The buffer is returned with its header
 		 * spinlock still held!
 		 */
-		buf = StrategyGetBuffer(strategy, &buf_state);
+		buf = StrategyGetBuffer(strategy, &buf_state, &from_ring);
 
 		Assert(BUF_STATE_GET_REFCOUNT(buf_state) == 0);
 
@@ -1254,7 +1287,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 					UnlockBufHdr(buf, buf_state);
 
 					if (XLogNeedsFlush(lsn) &&
-						StrategyRejectBuffer(strategy, buf))
+						StrategyRejectBuffer(strategy, buf, from_ring))
 					{
 						/* Drop lock/pin and loop around for another buffer */
 						LWLockRelease(BufferDescriptorGetContentLock(buf));
@@ -1269,7 +1302,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 														  smgr->smgr_rlocator.locator.dbOid,
 														  smgr->smgr_rlocator.locator.relNumber);
 
-				FlushBuffer(buf, NULL);
+				FlushBuffer(buf, NULL, *io_context, IOOBJECT_RELATION);
 				LWLockRelease(BufferDescriptorGetContentLock(buf));
 
 				ScheduleBufferTagForWriteback(&BackendWritebackContext,
@@ -1441,6 +1474,28 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
 
 	UnlockBufHdr(buf, buf_state);
 
+	if (oldFlags & BM_VALID)
+	{
+		/*
+		 * When a BufferAccessStrategy is in use, blocks evicted from shared
+		 * buffers are counted as IOOP_EVICT in the corresponding context
+		 * (e.g. IOCONTEXT_BULKWRITE). Shared buffers are evicted by a
+		 * strategy in two cases: 1) while initially claiming buffers for the
+		 * strategy ring 2) to replace an existing strategy ring buffer
+		 * because it is pinned or in use and cannot be reused.
+		 *
+		 * Blocks evicted from buffers already in the strategy ring are
+		 * counted as IOOP_REUSE in the corresponding strategy context.
+		 *
+		 * At this point, we can accurately count evictions and reuses,
+		 * because we have successfully claimed the valid buffer. Previously,
+		 * we may have been forced to release the buffer due to concurrent
+		 * pinners or erroring out.
+		 */
+		pgstat_count_io_op(from_ring ? IOOP_REUSE : IOOP_EVICT,
+						   IOOBJECT_RELATION, *io_context);
+	}
+
 	if (oldPartitionLock != NULL)
 	{
 		BufTableDelete(&oldTag, oldHash);
@@ -2570,7 +2625,7 @@ SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context)
 	PinBuffer_Locked(bufHdr);
 	LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
 
-	FlushBuffer(bufHdr, NULL);
+	FlushBuffer(bufHdr, NULL, IOCONTEXT_NORMAL, IOOBJECT_RELATION);
 
 	LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 
@@ -2820,7 +2875,7 @@ BufferGetTag(Buffer buffer, RelFileLocator *rlocator, ForkNumber *forknum,
  * as the second parameter.  If not, pass NULL.
  */
 static void
-FlushBuffer(BufferDesc *buf, SMgrRelation reln)
+FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOContext io_context, IOObject io_object)
 {
 	XLogRecPtr	recptr;
 	ErrorContextCallback errcallback;
@@ -2912,6 +2967,26 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln)
 			  bufToWrite,
 			  false);
 
+	/*
+	 * When a strategy is in use, only flushes of dirty buffers already in the
+	 * strategy ring are counted as strategy writes (IOCONTEXT
+	 * [BULKREAD|BULKWRITE|VACUUM] IOOP_WRITE) for the purpose of IO
+	 * statistics tracking.
+	 *
+	 * If a shared buffer initially added to the ring must be flushed before
+	 * being used, this is counted as an IOCONTEXT_NORMAL IOOP_WRITE.
+	 *
+	 * If a shared buffer which was added to the ring later because the
+	 * current strategy buffer is pinned or in use or because all strategy
+	 * buffers were dirty and rejected (for BAS_BULKREAD operations only)
+	 * requires flushing, this is counted as an IOCONTEXT_NORMAL IOOP_WRITE
+	 * (from_ring will be false).
+	 *
+	 * When a strategy is not in use, the write can only be a "regular" write
+	 * of a dirty shared buffer (IOCONTEXT_NORMAL IOOP_WRITE).
+	 */
+	pgstat_count_io_op(IOOP_WRITE, IOOBJECT_RELATION, io_context);
+
 	if (track_io_timing)
 	{
 		INSTR_TIME_SET_CURRENT(io_time);
@@ -3554,6 +3629,8 @@ FlushRelationBuffers(Relation rel)
 				buf_state &= ~(BM_DIRTY | BM_JUST_DIRTIED);
 				pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
 
+				pgstat_count_io_op(IOOP_WRITE, IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL);
+
 				/* Pop the error context stack */
 				error_context_stack = errcallback.previous;
 			}
@@ -3586,7 +3663,7 @@ FlushRelationBuffers(Relation rel)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, RelationGetSmgr(rel));
+			FlushBuffer(bufHdr, RelationGetSmgr(rel), IOCONTEXT_NORMAL, IOOBJECT_RELATION);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3684,7 +3761,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, srelent->srel);
+			FlushBuffer(bufHdr, srelent->srel, IOCONTEXT_NORMAL, IOOBJECT_RELATION);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3894,7 +3971,7 @@ FlushDatabaseBuffers(Oid dbid)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, NULL);
+			FlushBuffer(bufHdr, NULL, IOCONTEXT_NORMAL, IOOBJECT_RELATION);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3921,7 +3998,7 @@ FlushOneBuffer(Buffer buffer)
 
 	Assert(LWLockHeldByMe(BufferDescriptorGetContentLock(bufHdr)));
 
-	FlushBuffer(bufHdr, NULL);
+	FlushBuffer(bufHdr, NULL, IOCONTEXT_NORMAL, IOOBJECT_RELATION);
 }
 
 /*
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 7dec35801c..c690d5f15f 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -15,6 +15,7 @@
  */
 #include "postgres.h"
 
+#include "pgstat.h"
 #include "port/atomics.h"
 #include "storage/buf_internals.h"
 #include "storage/bufmgr.h"
@@ -81,12 +82,6 @@ typedef struct BufferAccessStrategyData
 	 */
 	int			current;
 
-	/*
-	 * True if the buffer just returned by StrategyGetBuffer had been in the
-	 * ring already.
-	 */
-	bool		current_was_in_ring;
-
 	/*
 	 * Array of buffer numbers.  InvalidBuffer (that is, zero) indicates we
 	 * have not yet selected a buffer for this ring slot.  For allocation
@@ -198,13 +193,15 @@ have_free_buffer(void)
  *	return the buffer with the buffer header spinlock still held.
  */
 BufferDesc *
-StrategyGetBuffer(BufferAccessStrategy strategy, uint32 *buf_state)
+StrategyGetBuffer(BufferAccessStrategy strategy, uint32 *buf_state, bool *from_ring)
 {
 	BufferDesc *buf;
 	int			bgwprocno;
 	int			trycounter;
 	uint32		local_buf_state;	/* to avoid repeated (de-)referencing */
 
+	*from_ring = false;
+
 	/*
 	 * If given a strategy object, see whether it can select a buffer. We
 	 * assume strategy objects don't need buffer_strategy_lock.
@@ -213,7 +210,10 @@ StrategyGetBuffer(BufferAccessStrategy strategy, uint32 *buf_state)
 	{
 		buf = GetBufferFromRing(strategy, buf_state);
 		if (buf != NULL)
+		{
+			*from_ring = true;
 			return buf;
+		}
 	}
 
 	/*
@@ -602,7 +602,7 @@ FreeAccessStrategy(BufferAccessStrategy strategy)
 
 /*
  * GetBufferFromRing -- returns a buffer from the ring, or NULL if the
- *		ring is empty.
+ *		ring is empty / not usable.
  *
  * The bufhdr spin lock is held on the returned buffer.
  */
@@ -625,10 +625,7 @@ GetBufferFromRing(BufferAccessStrategy strategy, uint32 *buf_state)
 	 */
 	bufnum = strategy->buffers[strategy->current];
 	if (bufnum == InvalidBuffer)
-	{
-		strategy->current_was_in_ring = false;
 		return NULL;
-	}
 
 	/*
 	 * If the buffer is pinned we cannot use it under any circumstances.
@@ -644,7 +641,6 @@ GetBufferFromRing(BufferAccessStrategy strategy, uint32 *buf_state)
 	if (BUF_STATE_GET_REFCOUNT(local_buf_state) == 0
 		&& BUF_STATE_GET_USAGECOUNT(local_buf_state) <= 1)
 	{
-		strategy->current_was_in_ring = true;
 		*buf_state = local_buf_state;
 		return buf;
 	}
@@ -654,7 +650,6 @@ GetBufferFromRing(BufferAccessStrategy strategy, uint32 *buf_state)
 	 * Tell caller to allocate a new buffer with the normal allocation
 	 * strategy.  He'll then replace this ring element via AddBufferToRing.
 	 */
-	strategy->current_was_in_ring = false;
 	return NULL;
 }
 
@@ -670,6 +665,39 @@ AddBufferToRing(BufferAccessStrategy strategy, BufferDesc *buf)
 	strategy->buffers[strategy->current] = BufferDescriptorGetBuffer(buf);
 }
 
+/*
+ * Utility function returning the IOContext of a given BufferAccessStrategy's
+ * strategy ring.
+ */
+IOContext
+IOContextForStrategy(BufferAccessStrategy strategy)
+{
+	if (!strategy)
+		return IOCONTEXT_NORMAL;
+
+	switch (strategy->btype)
+	{
+		case BAS_NORMAL:
+
+			/*
+			 * Currently, GetAccessStrategy() returns NULL for
+			 * BufferAccessStrategyType BAS_NORMAL, so this case is
+			 * unreachable.
+			 */
+			pg_unreachable();
+			return IOCONTEXT_NORMAL;
+		case BAS_BULKREAD:
+			return IOCONTEXT_BULKREAD;
+		case BAS_BULKWRITE:
+			return IOCONTEXT_BULKWRITE;
+		case BAS_VACUUM:
+			return IOCONTEXT_VACUUM;
+	}
+
+	elog(ERROR, "unrecognized BufferAccessStrategyType: %d", strategy->btype);
+	pg_unreachable();
+}
+
 /*
  * StrategyRejectBuffer -- consider rejecting a dirty buffer
  *
@@ -682,14 +710,14 @@ AddBufferToRing(BufferAccessStrategy strategy, BufferDesc *buf)
  * if this buffer should be written and re-used.
  */
 bool
-StrategyRejectBuffer(BufferAccessStrategy strategy, BufferDesc *buf)
+StrategyRejectBuffer(BufferAccessStrategy strategy, BufferDesc *buf, bool from_ring)
 {
 	/* We only do this in bulkread mode */
 	if (strategy->btype != BAS_BULKREAD)
 		return false;
 
 	/* Don't muck with behavior of normal buffer-replacement strategy */
-	if (!strategy->current_was_in_ring ||
+	if (!from_ring ||
 		strategy->buffers[strategy->current] != BufferDescriptorGetBuffer(buf))
 		return false;
 
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 8372acc383..2108bbe7d8 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -18,6 +18,7 @@
 #include "access/parallel.h"
 #include "catalog/catalog.h"
 #include "executor/instrument.h"
+#include "pgstat.h"
 #include "storage/buf_internals.h"
 #include "storage/bufmgr.h"
 #include "utils/guc_hooks.h"
@@ -107,7 +108,7 @@ PrefetchLocalBuffer(SMgrRelation smgr, ForkNumber forkNum,
  */
 BufferDesc *
 LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
-				 bool *foundPtr)
+				 bool *foundPtr, IOContext *io_context)
 {
 	BufferTag	newTag;			/* identity of requested block */
 	LocalBufferLookupEnt *hresult;
@@ -127,6 +128,14 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
 	hresult = (LocalBufferLookupEnt *)
 		hash_search(LocalBufHash, (void *) &newTag, HASH_FIND, NULL);
 
+	/*
+	 * IO Operations on local buffers are only done in IOCONTEXT_NORMAL. Set
+	 * io_context here (instead of after a buffer hit would have returned) for
+	 * convenience since we don't have to worry about the overhead of calling
+	 * IOContextForStrategy().
+	 */
+	*io_context = IOCONTEXT_NORMAL;
+
 	if (hresult)
 	{
 		b = hresult->id;
@@ -230,6 +239,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
 		buf_state &= ~BM_DIRTY;
 		pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
 
+		pgstat_count_io_op(IOOP_WRITE, IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL);
 		pgBufferUsage.local_blks_written++;
 	}
 
@@ -256,6 +266,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
 		ClearBufferTag(&bufHdr->tag);
 		buf_state &= ~(BM_VALID | BM_TAG_VALID);
 		pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
+		pgstat_count_io_op(IOOP_EVICT, IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL);
 	}
 
 	hresult = (LocalBufferLookupEnt *)
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 60c9905eff..37bae4bf73 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -983,6 +983,15 @@ mdimmedsync(SMgrRelation reln, ForkNumber forknum)
 	{
 		MdfdVec    *v = &reln->md_seg_fds[forknum][segno - 1];
 
+		/*
+		 * fsyncs done through mdimmedsync() should be tracked in a separate
+		 * IOContext than those done through mdsyncfiletag() to differentiate
+		 * between unavoidable client backend fsyncs (e.g. those done during
+		 * index build) and those which ideally would have been done by the
+		 * checkpointer. Since other IO operations bypassing the buffer
+		 * manager could also be tracked in such an IOContext, wait until
+		 * these are also tracked to track immediate fsyncs.
+		 */
 		if (FileSync(v->mdfd_vfd, WAIT_EVENT_DATA_FILE_IMMEDIATE_SYNC) < 0)
 			ereport(data_sync_elevel(ERROR),
 					(errcode_for_file_access(),
@@ -1021,6 +1030,19 @@ register_dirty_segment(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg)
 
 	if (!RegisterSyncRequest(&tag, SYNC_REQUEST, false /* retryOnError */ ))
 	{
+		/*
+		 * We have no way of knowing if the current IOContext is
+		 * IOCONTEXT_NORMAL or IOCONTEXT_[BULKREAD, BULKWRITE, VACUUM] at this
+		 * point, so count the fsync as being in the IOCONTEXT_NORMAL
+		 * IOContext. This is probably okay, because the number of backend
+		 * fsyncs doesn't say anything about the efficacy of the
+		 * BufferAccessStrategy. And counting both fsyncs done in
+		 * IOCONTEXT_NORMAL and IOCONTEXT_[BULKREAD, BULKWRITE, VACUUM] under
+		 * IOCONTEXT_NORMAL is likely clearer when investigating the number of
+		 * backend fsyncs.
+		 */
+		pgstat_count_io_op(IOOP_FSYNC, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+
 		ereport(DEBUG1,
 				(errmsg_internal("could not forward fsync request because request queue is full")));
 
@@ -1410,6 +1432,9 @@ mdsyncfiletag(const FileTag *ftag, char *path)
 	if (need_to_close)
 		FileClose(file);
 
+	if (result >= 0)
+		pgstat_count_io_op(IOOP_FSYNC, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+
 	errno = save_errno;
 	return result;
 }
diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index ed8aa2519c..0b44814740 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -15,6 +15,7 @@
 #ifndef BUFMGR_INTERNALS_H
 #define BUFMGR_INTERNALS_H
 
+#include "pgstat.h"
 #include "port/atomics.h"
 #include "storage/buf.h"
 #include "storage/bufmgr.h"
@@ -391,11 +392,12 @@ extern void IssuePendingWritebacks(WritebackContext *context);
 extern void ScheduleBufferTagForWriteback(WritebackContext *context, BufferTag *tag);
 
 /* freelist.c */
+extern IOContext IOContextForStrategy(BufferAccessStrategy bas);
 extern BufferDesc *StrategyGetBuffer(BufferAccessStrategy strategy,
-									 uint32 *buf_state);
+									 uint32 *buf_state, bool *from_ring);
 extern void StrategyFreeBuffer(BufferDesc *buf);
 extern bool StrategyRejectBuffer(BufferAccessStrategy strategy,
-								 BufferDesc *buf);
+								 BufferDesc *buf, bool from_ring);
 
 extern int	StrategySyncStart(uint32 *complete_passes, uint32 *num_buf_alloc);
 extern void StrategyNotifyBgWriter(int bgwprocno);
@@ -417,7 +419,7 @@ extern PrefetchBufferResult PrefetchLocalBuffer(SMgrRelation smgr,
 												ForkNumber forkNum,
 												BlockNumber blockNum);
 extern BufferDesc *LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum,
-									BlockNumber blockNum, bool *foundPtr);
+									BlockNumber blockNum, bool *foundPtr, IOContext *io_context);
 extern void MarkLocalBufferDirty(Buffer buffer);
 extern void DropRelationLocalBuffers(RelFileLocator rlocator,
 									 ForkNumber forkNum,
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 33eadbc129..b8a18b8081 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -23,7 +23,12 @@
 
 typedef void *Block;
 
-/* Possible arguments for GetAccessStrategy() */
+/*
+ * Possible arguments for GetAccessStrategy().
+ *
+ * If adding a new BufferAccessStrategyType, also add a new IOContext so
+ * IO statistics using this strategy are tracked.
+ */
 typedef enum BufferAccessStrategyType
 {
 	BAS_NORMAL,					/* Normal random access */
-- 
2.34.1

From 67a4cee981382eb63dd19c32458ef23da9eab5b9 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 17:20:41 -0500
Subject: [PATCH v46 2/5] pgstat: Infrastructure to track IO operations

Introduce "IOOp", an IO operation done by a backend, "IOObject", the
target object of the IO, and "IOContext", the context or location of the
IO operations on that object. For example, the checkpointer may write a
shared buffer out. This would be considered an IOOP_WRITE IOOp on an
IOOBJECT_RELATION IOObject in the IOCONTEXT_NORMAL IOContext by
BackendType B_CHECKPOINTER.

Each IOOp (evict, extend, fsync, read, reuse, and write) can be counted
per IOObject (relation, temp relation) per IOContext (normal, bulkread,
bulkwrite, or vacuum) through a call to pgstat_count_io_op().

Note that this commit introduces the infrastructure to count IO
Operation statistics. A subsequent commit will add calls to
pgstat_count_io_op() in the appropriate locations.

IOContext IOCONTEXT_NORMAL concerns operations on local and shared
buffers, while IOCONTEXT_BULKREAD, IOCONTEXT_BULKWRITE, and
IOCONTEXT_VACUUM IOContexts concern IO operations on buffers as part of
a BufferAccessStrategy.

IOObject IOOBJECT_TEMP_RELATION concerns IO Operations on buffers
containing temporary table data, while IOObject IOOBJECT_RELATION
concerns IO Operations on buffers containing permanent relation data.

Stats on IOOps on all IOObjects in all IOContexts for a given backend
are first counted in a backend's local memory and then flushed to shared
memory and accumulated with those from all other backends, exited and
live.

Some BackendTypes will not flush their pending statistics at regular
intervals and explicitly call pgstat_flush_io_ops() during the course of
normal operations to flush their backend-local IO operation statistics
to shared memory in a timely manner.

Because not all BackendType, IOOp, IOObject, IOContext combinations are
valid, the validity of the stats is checked before flushing pending
stats and before reading in the existing stats file to shared memory.

The aggregated stats in shared memory could be extended in the future
with per-backend stats -- useful for per connection IO statistics and
monitoring.

Author: Melanie Plageman <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Reviewed-by: Justin Pryzby <[email-masked]>
Reviewed-by: Kyotaro Horiguchi <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 doc/src/sgml/monitoring.sgml                  |   2 +
 src/backend/utils/activity/Makefile           |   1 +
 src/backend/utils/activity/meson.build        |   1 +
 src/backend/utils/activity/pgstat.c           |  26 ++
 src/backend/utils/activity/pgstat_bgwriter.c  |   7 +-
 .../utils/activity/pgstat_checkpointer.c      |   7 +-
 src/backend/utils/activity/pgstat_io.c        | 377 ++++++++++++++++++
 src/backend/utils/activity/pgstat_relation.c  |  15 +-
 src/backend/utils/activity/pgstat_shmem.c     |   4 +
 src/backend/utils/activity/pgstat_wal.c       |   4 +-
 src/backend/utils/adt/pgstatfuncs.c           |   4 +-
 src/include/miscadmin.h                       |   2 +
 src/include/pgstat.h                          |  66 +++
 src/include/utils/pgstat_internal.h           |  32 ++
 src/tools/pgindent/typedefs.list              |   6 +
 15 files changed, 548 insertions(+), 6 deletions(-)
 create mode 100644 src/backend/utils/activity/pgstat_io.c

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 358d2ff90f..8d51ca3773 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -5418,6 +5418,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
         the <structname>pg_stat_bgwriter</structname>
         view, <literal>archiver</literal> to reset all the counters shown in
         the <structname>pg_stat_archiver</structname> view,
+        <literal>io</literal> to reset all the counters shown in the
+        <structname>pg_stat_io</structname> view,
         <literal>wal</literal> to reset all the counters shown in the
         <structname>pg_stat_wal</structname> view or
         <literal>recovery_prefetch</literal> to reset all the counters shown
diff --git a/src/backend/utils/activity/Makefile b/src/backend/utils/activity/Makefile
index a80eda3cf4..7d7482dde0 100644
--- a/src/backend/utils/activity/Makefile
+++ b/src/backend/utils/activity/Makefile
@@ -22,6 +22,7 @@ OBJS = \
 	pgstat_checkpointer.o \
 	pgstat_database.o \
 	pgstat_function.o \
+	pgstat_io.o \
 	pgstat_relation.o \
 	pgstat_replslot.o \
 	pgstat_shmem.o \
diff --git a/src/backend/utils/activity/meson.build b/src/backend/utils/activity/meson.build
index a2b872c24b..518ee3f798 100644
--- a/src/backend/utils/activity/meson.build
+++ b/src/backend/utils/activity/meson.build
@@ -9,6 +9,7 @@ backend_sources += files(
   'pgstat_checkpointer.c',
   'pgstat_database.c',
   'pgstat_function.c',
+  'pgstat_io.c',
   'pgstat_relation.c',
   'pgstat_replslot.c',
   'pgstat_shmem.c',
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 0fa5370bcd..608c3b59da 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -72,6 +72,7 @@
  * - pgstat_checkpointer.c
  * - pgstat_database.c
  * - pgstat_function.c
+ * - pgstat_io.c
  * - pgstat_relation.c
  * - pgstat_replslot.c
  * - pgstat_slru.c
@@ -359,6 +360,15 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 		.snapshot_cb = pgstat_checkpointer_snapshot_cb,
 	},
 
+	[PGSTAT_KIND_IO] = {
+		.name = "io_ops",
+
+		.fixed_amount = true,
+
+		.reset_all_cb = pgstat_io_reset_all_cb,
+		.snapshot_cb = pgstat_io_snapshot_cb,
+	},
+
 	[PGSTAT_KIND_SLRU] = {
 		.name = "slru",
 
@@ -582,6 +592,7 @@ pgstat_report_stat(bool force)
 
 	/* Don't expend a clock check if nothing to do */
 	if (dlist_is_empty(&pgStatPending) &&
+		!have_iostats &&
 		!have_slrustats &&
 		!pgstat_have_pending_wal())
 	{
@@ -628,6 +639,9 @@ pgstat_report_stat(bool force)
 	/* flush database / relation / function / ... stats */
 	partial_flush |= pgstat_flush_pending_entries(nowait);
 
+	/* flush IO stats */
+	partial_flush |= pgstat_flush_io(nowait);
+
 	/* flush wal stats */
 	partial_flush |= pgstat_flush_wal(nowait);
 
@@ -1322,6 +1336,12 @@ pgstat_write_statsfile(void)
 	pgstat_build_snapshot_fixed(PGSTAT_KIND_CHECKPOINTER);
 	write_chunk_s(fpout, &pgStatLocal.snapshot.checkpointer);
 
+	/*
+	 * Write IO stats struct
+	 */
+	pgstat_build_snapshot_fixed(PGSTAT_KIND_IO);
+	write_chunk_s(fpout, &pgStatLocal.snapshot.io);
+
 	/*
 	 * Write SLRU stats struct
 	 */
@@ -1496,6 +1516,12 @@ pgstat_read_statsfile(void)
 	if (!read_chunk_s(fpin, &shmem->checkpointer.stats))
 		goto error;
 
+	/*
+	 * Read IO stats struct
+	 */
+	if (!read_chunk_s(fpin, &shmem->io.stats))
+		goto error;
+
 	/*
 	 * Read SLRU stats struct
 	 */
diff --git a/src/backend/utils/activity/pgstat_bgwriter.c b/src/backend/utils/activity/pgstat_bgwriter.c
index 9247f2dda2..92be384b0d 100644
--- a/src/backend/utils/activity/pgstat_bgwriter.c
+++ b/src/backend/utils/activity/pgstat_bgwriter.c
@@ -24,7 +24,7 @@ PgStat_BgWriterStats PendingBgWriterStats = {0};

 /*
- * Report bgwriter statistics
+ * Report bgwriter and IO statistics
  */
 void
 pgstat_report_bgwriter(void)
@@ -56,6 +56,11 @@ pgstat_report_bgwriter(void)
 	 * Clear out the statistics buffer, so it can be re-used.
 	 */
 	MemSet(&PendingBgWriterStats, 0, sizeof(PendingBgWriterStats));
+
+	/*
+	 * Report IO statistics
+	 */
+	pgstat_flush_io(false);
 }
 
 /*
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index 3e9ab45103..26dec112f6 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -24,7 +24,7 @@ PgStat_CheckpointerStats PendingCheckpointerStats = {0};

 /*
- * Report checkpointer statistics
+ * Report checkpointer and IO statistics
  */
 void
 pgstat_report_checkpointer(void)
@@ -62,6 +62,11 @@ pgstat_report_checkpointer(void)
 	 * Clear out the statistics buffer, so it can be re-used.
 	 */
 	MemSet(&PendingCheckpointerStats, 0, sizeof(PendingCheckpointerStats));
+
+	/*
+	 * Report IO statistics
+	 */
+	pgstat_flush_io(false);
 }
 
 /*
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
new file mode 100644
index 0000000000..f95ab9c94d
--- /dev/null
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -0,0 +1,377 @@
+/* -------------------------------------------------------------------------
+ *
+ * pgstat_io.c
+ *	  Implementation of IO statistics.
+ *
+ * This file contains the implementation of IO statistics. It is kept separate
+ * from pgstat.c to enforce the line between the statistics access / storage
+ * implementation and the details about individual types of statistics.
+ *
+ * Copyright (c) 2021-2023, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ *	  src/backend/utils/activity/pgstat_io.c
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "utils/pgstat_internal.h"
+
+
+static PgStat_BackendIO PendingIOStats;
+bool		have_iostats = false;
+
+
+void
+pgstat_count_io_op(IOOp io_op, IOObject io_object, IOContext io_context)
+{
+	Assert(io_context < IOCONTEXT_NUM_TYPES);
+	Assert(io_object < IOOBJECT_NUM_TYPES);
+	Assert(io_op < IOOP_NUM_TYPES);
+	Assert(pgstat_tracks_io_op(MyBackendType, io_context, io_object, io_op));
+
+	PendingIOStats.data[io_context][io_object][io_op]++;
+
+	have_iostats = true;
+}
+
+PgStat_IO *
+pgstat_fetch_stat_io(void)
+{
+	pgstat_snapshot_fixed(PGSTAT_KIND_IO);
+
+	return &pgStatLocal.snapshot.io;
+}
+
+/*
+ * Flush out locally pending IO statistics
+ *
+ * If no stats have been recorded, this function returns false.
+ *
+ * If nowait is true, this function returns true if the lock could not be
+ * acquired. Otherwise, return false.
+ */
+bool
+pgstat_flush_io(bool nowait)
+{
+	LWLock	   *bktype_lock;
+	PgStat_BackendIO *bktype_shstats;
+
+	if (!have_iostats)
+		return false;
+
+	bktype_lock = &pgStatLocal.shmem->io.locks[MyBackendType];
+	bktype_shstats =
+		&pgStatLocal.shmem->io.stats.stats[MyBackendType];
+
+	if (!nowait)
+		LWLockAcquire(bktype_lock, LW_EXCLUSIVE);
+	else if (!LWLockConditionalAcquire(bktype_lock, LW_EXCLUSIVE))
+		return true;
+
+	for (IOContext io_context = IOCONTEXT_FIRST;
+		 io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		for (IOObject io_object = IOOBJECT_FIRST;
+			 io_object < IOOBJECT_NUM_TYPES; io_object++)
+			for (IOOp io_op = IOOP_FIRST;
+				 io_op < IOOP_NUM_TYPES; io_op++)
+				bktype_shstats->data[io_context][io_object][io_op] +=
+					PendingIOStats.data[io_context][io_object][io_op];
+
+	Assert(pgstat_bktype_io_stats_valid(bktype_shstats, MyBackendType));
+
+	LWLockRelease(bktype_lock);
+
+	memset(&PendingIOStats, 0, sizeof(PendingIOStats));
+
+	have_iostats = false;
+
+	return false;
+}
+
+const char *
+pgstat_get_io_context_name(IOContext io_context)
+{
+	switch (io_context)
+	{
+		case IOCONTEXT_BULKREAD:
+			return "bulkread";
+		case IOCONTEXT_BULKWRITE:
+			return "bulkwrite";
+		case IOCONTEXT_NORMAL:
+			return "normal";
+		case IOCONTEXT_VACUUM:
+			return "vacuum";
+	}
+
+	elog(ERROR, "unrecognized IOContext value: %d", io_context);
+	pg_unreachable();
+}
+
+const char *
+pgstat_get_io_object_name(IOObject io_object)
+{
+	switch (io_object)
+	{
+		case IOOBJECT_RELATION:
+			return "relation";
+		case IOOBJECT_TEMP_RELATION:
+			return "temp relation";
+	}
+
+	elog(ERROR, "unrecognized IOObject value: %d", io_object);
+	pg_unreachable();
+}
+
+void
+pgstat_io_reset_all_cb(TimestampTz ts)
+{
+	for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+	{
+		LWLock	   *bktype_lock = &pgStatLocal.shmem->io.locks[i];
+		PgStat_BackendIO *bktype_shstats = &pgStatLocal.shmem->io.stats.stats[i];
+
+		LWLockAcquire(bktype_lock, LW_EXCLUSIVE);
+
+		/*
+		 * Use the lock in the first BackendType's PgStat_BackendIO to protect
+		 * the reset timestamp as well.
+		 */
+		if (i == 0)
+			pgStatLocal.shmem->io.stats.stat_reset_timestamp = ts;
+
+		memset(bktype_shstats, 0, sizeof(*bktype_shstats));
+		LWLockRelease(bktype_lock);
+	}
+}
+
+void
+pgstat_io_snapshot_cb(void)
+{
+	for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+	{
+		LWLock	   *bktype_lock = &pgStatLocal.shmem->io.locks[i];
+		PgStat_BackendIO *bktype_shstats = &pgStatLocal.shmem->io.stats.stats[i];
+		PgStat_BackendIO *bktype_snap = &pgStatLocal.snapshot.io.stats[i];
+
+		LWLockAcquire(bktype_lock, LW_SHARED);
+
+		/*
+		 * Use the lock in the first BackendType's PgStat_BackendIO to protect
+		 * the reset timestamp as well.
+		 */
+		if (i == 0)
+			pgStatLocal.snapshot.io.stat_reset_timestamp =
+				pgStatLocal.shmem->io.stats.stat_reset_timestamp;
+
+		/* using struct assignment due to better type safety */
+		*bktype_snap = *bktype_shstats;
+		LWLockRelease(bktype_lock);
+	}
+}
+
+/*
+* IO statistics are not collected for all BackendTypes.
+*
+* The following BackendTypes do not participate in the cumulative stats
+* subsystem or do not perform IO on which we currently track:
+* - Syslogger because it is not connected to shared memory
+* - Archiver because most relevant archiving IO is delegated to a
+*   specialized command or module
+* - WAL Receiver and WAL Writer IO is not tracked in pg_stat_io for now
+*
+* Function returns true if BackendType participates in the cumulative stats
+* subsystem for IO and false if it does not.
+*/
+bool
+pgstat_tracks_io_bktype(BackendType bktype)
+{
+	/*
+	 * List every type so that new backend types trigger a warning about
+	 * needing to adjust this switch.
+	 */
+	switch (bktype)
+	{
+		case B_INVALID:
+		case B_ARCHIVER:
+		case B_LOGGER:
+		case B_WAL_RECEIVER:
+		case B_WAL_WRITER:
+			return false;
+
+		case B_AUTOVAC_LAUNCHER:
+		case B_AUTOVAC_WORKER:
+		case B_BACKEND:
+		case B_BG_WORKER:
+		case B_BG_WRITER:
+		case B_CHECKPOINTER:
+		case B_STANDALONE_BACKEND:
+		case B_STARTUP:
+		case B_WAL_SENDER:
+			return true;
+	}
+
+	return false;
+}
+
+/*
+ * Some BackendTypes do not perform IO in certain IOContexts. Some IOObjects
+ * are never operated on in some IOContexts. Check that the given BackendType
+ * is expected to do IO in the given IOContext and that the given IOObject is
+ * expected to be operated on in the given IOContext.
+ */
+bool
+pgstat_tracks_io_object(BackendType bktype, IOContext io_context,
+						IOObject io_object)
+{
+	bool		no_temp_rel;
+
+	/*
+	 * Some BackendTypes should never track IO statistics.
+	 */
+	if (!pgstat_tracks_io_bktype(bktype))
+		return false;
+
+	/*
+	 * Currently, IO on temporary relations can only occur in the
+	 * IOCONTEXT_NORMAL IOContext.
+	 */
+	if (io_context != IOCONTEXT_NORMAL &&
+		io_object == IOOBJECT_TEMP_RELATION)
+		return false;
+
+	/*
+	 * In core Postgres, only regular backends and WAL Sender processes
+	 * executing queries will use local buffers and operate on temporary
+	 * relations. Parallel workers will not use local buffers (see
+	 * InitLocalBuffers()); however, extensions leveraging background workers
+	 * have no such limitation, so track IO on IOOBJECT_TEMP_RELATION for
+	 * BackendType B_BG_WORKER.
+	 */
+	no_temp_rel = bktype == B_AUTOVAC_LAUNCHER || bktype == B_BG_WRITER ||
+		bktype == B_CHECKPOINTER || bktype == B_AUTOVAC_WORKER ||
+		bktype == B_STANDALONE_BACKEND || bktype == B_STARTUP;
+
+	if (no_temp_rel && io_context == IOCONTEXT_NORMAL &&
+		io_object == IOOBJECT_TEMP_RELATION)
+		return false;
+
+	/*
+	 * Some BackendTypes do not currently perform any IO in certain
+	 * IOContexts, and, while it may not be inherently incorrect for them to
+	 * do so, excluding those rows from the view makes the view easier to use.
+	 */
+	if ((bktype == B_CHECKPOINTER || bktype == B_BG_WRITER) &&
+		(io_context == IOCONTEXT_BULKREAD ||
+		 io_context == IOCONTEXT_BULKWRITE ||
+		 io_context == IOCONTEXT_VACUUM))
+		return false;
+
+	if (bktype == B_AUTOVAC_LAUNCHER && io_context == IOCONTEXT_VACUUM)
+		return false;
+
+	if ((bktype == B_AUTOVAC_WORKER || bktype == B_AUTOVAC_LAUNCHER) &&
+		io_context == IOCONTEXT_BULKWRITE)
+		return false;
+
+	return true;
+}
+
+/*
+ * Some BackendTypes will never do certain IOOps and some IOOps should not
+ * occur in certain IOContexts. Check that the given IOOp is valid for the
+ * given BackendType in the given IOContext. Note that there are currently no
+ * cases of an IOOp being invalid for a particular BackendType only within a
+ * certain IOContext.
+ */
+bool
+pgstat_tracks_io_op(BackendType bktype, IOContext io_context,
+					IOObject io_object, IOOp io_op)
+{
+	bool		strategy_io_context;
+
+	/* if (io_context, io_object) will never collect stats, we're done */
+	if (!pgstat_tracks_io_object(bktype, io_context, io_object))
+		return false;
+
+	/*
+	 * Some BackendTypes will not do certain IOOps.
+	 */
+	if ((bktype == B_BG_WRITER || bktype == B_CHECKPOINTER) &&
+		(io_op == IOOP_READ || io_op == IOOP_EVICT))
+		return false;
+
+	if ((bktype == B_AUTOVAC_LAUNCHER || bktype == B_BG_WRITER ||
+		 bktype == B_CHECKPOINTER) && io_op == IOOP_EXTEND)
+		return false;
+
+	/*
+	 * Some IOOps are not valid in certain IOContexts and some IOOps are only
+	 * valid in certain contexts.
+	 */
+	if (io_context == IOCONTEXT_BULKREAD && io_op == IOOP_EXTEND)
+		return false;
+
+	strategy_io_context = io_context == IOCONTEXT_BULKREAD ||
+		io_context == IOCONTEXT_BULKWRITE || io_context == IOCONTEXT_VACUUM;
+
+	/*
+	 * IOOP_REUSE is only relevant when a BufferAccessStrategy is in use.
+	 */
+	if (!strategy_io_context && io_op == IOOP_REUSE)
+		return false;
+
+	/*
+	 * IOOP_FSYNC IOOps done by a backend using a BufferAccessStrategy are
+	 * counted in the IOCONTEXT_NORMAL IOContext. See comment in
+	 * register_dirty_segment() for more details.
+	 */
+	if (strategy_io_context && io_op == IOOP_FSYNC)
+		return false;
+
+	/*
+	 * Temporary tables are not logged and thus do not require fsync'ing.
+	 */
+	if (io_context == IOCONTEXT_NORMAL &&
+		io_object == IOOBJECT_TEMP_RELATION && io_op == IOOP_FSYNC)
+		return false;
+
+	return true;
+}
+
+/*
+ * Check that stats have not been counted for any combination of IOContext,
+ * IOObject, and IOOp which are not tracked for the passed-in BackendType. The
+ * passed-in PgStat_BackendIO must contain stats from the BackendType specified
+ * by the second parameter. Caller is responsible for locking the passed-in
+ * PgStat_BackendIO, if needed.
+ */
+bool
+pgstat_bktype_io_stats_valid(PgStat_BackendIO *backend_io,
+							 BackendType bktype)
+{
+	bool		bktype_tracked = pgstat_tracks_io_bktype(bktype);
+
+	for (IOContext io_context = IOCONTEXT_FIRST;
+		 io_context < IOCONTEXT_NUM_TYPES; io_context++)
+	{
+		for (IOObject io_object = IOOBJECT_FIRST;
+			 io_object < IOOBJECT_NUM_TYPES; io_object++)
+		{
+			/*
+			 * Don't bother trying to skip to the next loop iteration if
+			 * pgstat_tracks_io_object() would return false here. We still
+			 * need to validate that each counter is zero anyway.
+			 */
+			for (IOOp io_op = IOOP_FIRST; io_op < IOOP_NUM_TYPES; io_op++)
+			{
+				if ((!bktype_tracked || !pgstat_tracks_io_op(bktype, io_context, io_object, io_op)) &&
+					backend_io->data[io_context][io_object][io_op] != 0)
+					return false;
+			}
+		}
+	}
+
+	return true;
+}
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index 2e20b93c20..f793ac1516 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -206,7 +206,7 @@ pgstat_drop_relation(Relation rel)
 }
 
 /*
- * Report that the table was just vacuumed.
+ * Report that the table was just vacuumed and flush IO statistics.
  */
 void
 pgstat_report_vacuum(Oid tableoid, bool shared,
@@ -258,10 +258,18 @@ pgstat_report_vacuum(Oid tableoid, bool shared,
 	}
 
 	pgstat_unlock_entry(entry_ref);
+
+	/*
+	 * Flush IO statistics now. pgstat_report_stat() will flush IO stats,
+	 * however this will not be called until after an entire autovacuum cycle
+	 * is done -- which will likely vacuum many relations -- or until the
+	 * VACUUM command has processed all tables and committed.
+	 */
+	pgstat_flush_io(false);
 }
 
 /*
- * Report that the table was just analyzed.
+ * Report that the table was just analyzed and flush IO statistics.
  *
  * Caller must provide new live- and dead-tuples estimates, as well as a
  * flag indicating whether to reset the mod_since_analyze counter.
@@ -341,6 +349,9 @@ pgstat_report_analyze(Relation rel,
 	}
 
 	pgstat_unlock_entry(entry_ref);
+
+	/* see pgstat_report_vacuum() */
+	pgstat_flush_io(false);
 }
 
 /*
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c
index c1506b53d0..09fffd0e82 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -202,6 +202,10 @@ StatsShmemInit(void)
 		LWLockInitialize(&ctl->checkpointer.lock, LWTRANCHE_PGSTATS_DATA);
 		LWLockInitialize(&ctl->slru.lock, LWTRANCHE_PGSTATS_DATA);
 		LWLockInitialize(&ctl->wal.lock, LWTRANCHE_PGSTATS_DATA);
+
+		for (int i = 0; i < BACKEND_NUM_TYPES; i++)
+			LWLockInitialize(&ctl->io.locks[i],
+							 LWTRANCHE_PGSTATS_DATA);
 	}
 	else
 	{
diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c
index e7a82b5fed..e8598b2f4e 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -34,7 +34,7 @@ static WalUsage prevWalUsage;
 
 /*
  * Calculate how much WAL usage counters have increased and update
- * shared statistics.
+ * shared WAL and IO statistics.
  *
  * Must be called by processes that generate WAL, that do not call
  * pgstat_report_stat(), like walwriter.
@@ -43,6 +43,8 @@ void
 pgstat_report_wal(bool force)
 {
 	pgstat_flush_wal(force);
+
+	pgstat_flush_io(force);
 }
 
 /*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 58bd1360b9..5c8e8336bf 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1593,6 +1593,8 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
 		pgstat_reset_of_kind(PGSTAT_KIND_BGWRITER);
 		pgstat_reset_of_kind(PGSTAT_KIND_CHECKPOINTER);
 	}
+	else if (strcmp(target, "io") == 0)
+		pgstat_reset_of_kind(PGSTAT_KIND_IO);
 	else if (strcmp(target, "recovery_prefetch") == 0)
 		XLogPrefetchResetStats();
 	else if (strcmp(target, "wal") == 0)
@@ -1601,7 +1603,7 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("unrecognized reset target: \"%s\"", target),
-				 errhint("Target must be \"archiver\", \"bgwriter\", \"recovery_prefetch\", or \"wal\".")));
+				 errhint("Target must be \"archiver\", \"bgwriter\", \"io\", \"recovery_prefetch\", or \"wal\".")));
 
 	PG_RETURN_VOID();
 }
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 96b3a1e1a0..c309e0233d 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -332,6 +332,8 @@ typedef enum BackendType
 	B_WAL_WRITER,
 } BackendType;
 
+#define BACKEND_NUM_TYPES (B_WAL_WRITER + 1)
+
 extern PGDLLIMPORT BackendType MyBackendType;
 
 extern const char *GetBackendTypeDesc(BackendType backendType);
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 5e3326a3b9..f2a66ed7fb 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -48,6 +48,7 @@ typedef enum PgStat_Kind
 	PGSTAT_KIND_ARCHIVER,
 	PGSTAT_KIND_BGWRITER,
 	PGSTAT_KIND_CHECKPOINTER,
+	PGSTAT_KIND_IO,
 	PGSTAT_KIND_SLRU,
 	PGSTAT_KIND_WAL,
 } PgStat_Kind;
@@ -276,6 +277,55 @@ typedef struct PgStat_CheckpointerStats
 	PgStat_Counter buf_fsync_backend;
 } PgStat_CheckpointerStats;
 
+
+/*
+ * Types related to counting IO operations
+ */
+typedef enum IOContext
+{
+	IOCONTEXT_BULKREAD,
+	IOCONTEXT_BULKWRITE,
+	IOCONTEXT_NORMAL,
+	IOCONTEXT_VACUUM,
+} IOContext;
+
+#define IOCONTEXT_FIRST IOCONTEXT_BULKREAD
+#define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)
+
+typedef enum IOObject
+{
+	IOOBJECT_RELATION,
+	IOOBJECT_TEMP_RELATION,
+} IOObject;
+
+#define IOOBJECT_FIRST IOOBJECT_RELATION
+#define IOOBJECT_NUM_TYPES (IOOBJECT_TEMP_RELATION + 1)
+
+typedef enum IOOp
+{
+	IOOP_EVICT,
+	IOOP_EXTEND,
+	IOOP_FSYNC,
+	IOOP_READ,
+	IOOP_REUSE,
+	IOOP_WRITE,
+} IOOp;
+
+#define IOOP_FIRST IOOP_EVICT
+#define IOOP_NUM_TYPES (IOOP_WRITE + 1)
+
+typedef struct PgStat_BackendIO
+{
+	PgStat_Counter data[IOCONTEXT_NUM_TYPES][IOOBJECT_NUM_TYPES][IOOP_NUM_TYPES];
+} PgStat_BackendIO;
+
+typedef struct PgStat_IO
+{
+	TimestampTz stat_reset_timestamp;
+	PgStat_BackendIO stats[BACKEND_NUM_TYPES];
+} PgStat_IO;
+
+
 typedef struct PgStat_StatDBEntry
 {
 	PgStat_Counter xact_commit;
@@ -453,6 +503,22 @@ extern void pgstat_report_checkpointer(void);
 extern PgStat_CheckpointerStats *pgstat_fetch_stat_checkpointer(void);

+/*
+ * Functions in pgstat_io.c
+ */
+
+extern void pgstat_count_io_op(IOOp io_op, IOObject io_object, IOContext io_context);
+extern PgStat_IO *pgstat_fetch_stat_io(void);
+extern const char *pgstat_get_io_context_name(IOContext io_context);
+extern const char *pgstat_get_io_object_name(IOObject io_object);
+
+extern bool pgstat_tracks_io_bktype(BackendType bktype);
+extern bool pgstat_tracks_io_object(BackendType bktype,
+									IOContext io_context, IOObject io_object);
+extern bool pgstat_tracks_io_op(BackendType bktype, IOContext io_context,
+								IOObject io_object, IOOp io_op);
+
+
 /*
  * Functions in pgstat_database.c
  */
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 12fd51f1ae..bf8e4c3b8b 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -329,6 +329,17 @@ typedef struct PgStatShared_Checkpointer
 	PgStat_CheckpointerStats reset_offset;
 } PgStatShared_Checkpointer;
 
+/* shared version of PgStat_IO */
+typedef struct PgStatShared_IO
+{
+	/*
+	 * locks[i] protects stats.stats[i]. locks[0] also protects
+	 * stats.stat_reset_timestamp.
+	 */
+	LWLock		locks[BACKEND_NUM_TYPES];
+	PgStat_IO	stats;
+} PgStatShared_IO;
+
 typedef struct PgStatShared_SLRU
 {
 	/* lock protects ->stats */
@@ -419,6 +430,7 @@ typedef struct PgStat_ShmemControl
 	PgStatShared_Archiver archiver;
 	PgStatShared_BgWriter bgwriter;
 	PgStatShared_Checkpointer checkpointer;
+	PgStatShared_IO io;
 	PgStatShared_SLRU slru;
 	PgStatShared_Wal wal;
 } PgStat_ShmemControl;
@@ -442,6 +454,8 @@ typedef struct PgStat_Snapshot
 
 	PgStat_CheckpointerStats checkpointer;
 
+	PgStat_IO	io;
+
 	PgStat_SLRUStats slru[SLRU_NUM_ELEMENTS];
 
 	PgStat_WalStats wal;
@@ -549,6 +563,17 @@ extern void pgstat_database_reset_timestamp_cb(PgStatShared_Common *header, Time
 extern bool pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);

+/*
+ * Functions in pgstat_io.c
+ */
+
+extern void pgstat_io_reset_all_cb(TimestampTz ts);
+extern void pgstat_io_snapshot_cb(void);
+extern bool pgstat_flush_io(bool nowait);
+extern bool pgstat_bktype_io_stats_valid(PgStat_BackendIO *context_ops,
+										 BackendType bktype);
+
+
 /*
  * Functions in pgstat_relation.c
  */
@@ -643,6 +668,13 @@ extern void pgstat_create_transactional(PgStat_Kind kind, Oid dboid, Oid objoid)
 extern PGDLLIMPORT PgStat_LocalState pgStatLocal;

+/*
+ * Variables in pgstat_io.c
+ */
+
+extern PGDLLIMPORT bool have_iostats;
+
+
 /*
  * Variables in pgstat_slru.c
  */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 23bafec5f7..7b66b1bc89 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1106,7 +1106,10 @@ ID
 INFIX
 INT128
 INTERFACE_INFO
+IOContext
 IOFuncSelector
+IOObject
+IOOp
 IPCompareMethod
 ITEM
 IV
@@ -2016,6 +2019,7 @@ PgStatShared_Common
 PgStatShared_Database
 PgStatShared_Function
 PgStatShared_HashEntry
+PgStatShared_IO
 PgStatShared_Relation
 PgStatShared_ReplSlot
 PgStatShared_SLRU
@@ -2033,6 +2037,8 @@ PgStat_FetchConsistency
 PgStat_FunctionCallUsage
 PgStat_FunctionCounts
 PgStat_HashKey
+PgStat_IO
+PgStat_BackendIO
 PgStat_Kind
 PgStat_KindInfo
 PgStat_LocalState
-- 
2.34.1

---

From: Justin Pryzby <[email-masked]>
Date: Thu, 12 Jan 2023 23:23:05 -0600

> tablespaces in contrib also?

That's the test_stats one I mentioned.

Check with -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS

> structures which access shared buffers but they are currently being used

This would be a good place to link to a description of the ringbuffer,
if we had one.

> to changing it if there was a specific reason you don't like it.

I suggested to change it because at first I started to interpret it as
"The act of qualifying large I/O ops .." rather than "Large I/O ops that
qualify..".

This is still a bit too easy to misinterpret as being in units of bytes.
I suggest: Number of read operations (which are each of the size
specified in >op_bytes<).

+ in order to add the shared buffer to a separate size-limited ring buffer

separate comma

+ More information on configuring checkpointer can be found in Section 30.5. 

*the* checkpointer (as in the following paragraph)

I still think it's a bit hard to understand the <varname>s adjacent to
<literal>s.

+ Some backend_types
+ in some io_contexts
+ on some io_objects
+ in certain io_contexts
+ on certain io_objects

Maybe these should not use underscores:  Some backend types never
perform I/O operations in some I/O contexts and/or on some i/o objects.

+ for (BackendType bktype = B_INVALID; bktype < BACKEND_NUM_TYPES; bktype++)
+ for (IOContext io_context = IOCONTEXT_BULKREAD; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ for (IOObject io_obj = IOOBJECT_RELATION; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+ for (IOOp io_op = IOOP_EVICT; io_op < IOOP_NUM_TYPES; io_op++)

These look a bit fragile due to starting at some hardcoded "first"
value.  In other places you use symbols "FIRST" symbols:

+       for (IOContext io_context = IOCONTEXT_FIRST; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+               for (IOObject io_object = IOOBJECT_FIRST; io_object < IOOBJECT_NUM_TYPES; io_object++)
+                       for (IOOp io_op = IOOP_FIRST; io_op < IOOP_NUM_TYPES; io_op++)

I think that's marginally better, but I think having to define both
FIRST and NUM is excessive and doesn't make it less fragile.  Not sure
what anyone else will say, but I'd prefer if it started at "0".

Thanks for working on this - I'm looking forward to updating my rrdtool
script for this soon.  It'll be nice to finally distinguish huge number
of "backend ringbuffer writes during ALTER" from other backend writes.
Currently, that makes it look like something is terribly wrong.

-- 
Justin

---

From: Melanie Plageman <[email-masked]>
Date: Fri, 13 Jan 2023 13:38:15 -0500

Attached is v47.

> Check with -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS

Thanks. I have now changed both tablespace names and checked using that
macro.

> This would be a good place to link to a description of the ringbuffer,

Indeed.

> "The act of qualifying large I/O ops .." rather than "Large I/O ops that

I have changed it to "certain".

I have changed this.

> *the* checkpointer (as in the following paragraph)

above items changed.

> I still think it's a bit hard to understand the <varname>s adjacent to

I agree it isn't great -- is there a different XML tag you suggest
instead of literal?

I've changed this.

Also, taking another look, I forgot to update the docs' column name
tenses in the last version. That is now done.

> what anyone else will say, but I'd prefer if it started at "0".

Thanks for catching the discrepancy in pg_stat_get_io(). I have changed
those instances to use _FIRST.

I think that having the loop start from the first enum value (except
when that value is something special like _INVALID like with
BackendType) is confusing. I agree that having multiple macros to allow
iteration through all enum values introduces some fragility. I'm not
sure about using the number 0 with the enum as the loop variable
data type. Is that a common pattern?

In this version, I have updated the loops in pg_stat_get_io() to use
_FIRST.

> Currently, that makes it look like something is terribly wrong.

Cool! I'm glad to know you will use it.

- Melanie
From ed7b2975732d599d8c809e21c2e33554c77ffc43 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 17:20:50 -0500
Subject: [PATCH v47 3/5] pgstat: Count IO for relations

 
 
 	if (isLocalBuf)
 	{

 	if (isExtend)
 	{

 

 

 

 

 
+	/*

 
 
 /*
 
+#include "pgstat.h"
 
-	/*
 
+	*from_ring = false;
+
 	/*
 
 	/*
 
 /*
 
 	/*

+/*

 
+	/*

 
 
+		/*

 
+	if (result >= 0)
 
+#include "pgstat.h"
 
 /* freelist.c */

 typedef void *Block;

From 28a57a119e2d74e00b329da850c75584178e6967 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 17:20:41 -0500
Subject: [PATCH v47 2/5] pgstat: Infrastructure to track IO operations

 
+	[PGSTAT_KIND_IO] = {
+		.name = "io_ops",
+
+		.fixed_amount = true,
+

 
+	/* flush IO stats */

+	/*
 
+	/*
+	 * Read IO stats struct
+	 */

 /*
 
 /*

 /*
 
 /*
 
 /*

 /*

 /*
 
 /*
 
 /*
 
 	PG_RETURN_VOID();
 }

 
+
+/*

+/*

 
+	PgStat_IO	io;
+
 
 	PgStat_WalStats wal;

+/*

+/*

From b112a4114b9b61b19e6a62ed5fc82a01d8b1d1a2 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 9 Jan 2023 14:42:25 -0500
Subject: [PATCH v47 4/5] Add system view tracking IO ops per backend type

index c010361025..055107f6b5 100644
+CREATE TABLESPACE regress_test_stats_tblspc LOCATION '';
+ALTER TABLE heaptest SET TABLESPACE regress_test_stats_tblspc;
 
+DROP TABLESPACE regress_test_stats_tblspc;
index 298de6886a..1cfd52bd13 100644
 
+
 
+-- pg_stat_io test:
+CREATE TABLESPACE regress_test_stats_tblspc LOCATION '';
+ALTER TABLE heaptest SET TABLESPACE regress_test_stats_tblspc;

 
 -- cleanup
 DROP TABLE heaptest;
+DROP TABLESPACE regress_test_stats_tblspc;
 DROP TABLE test_partition;
 DROP TABLE test_partitioned;
 DROP OWNED BY regress_heaptest_role; -- permissions
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index d2a8c82900..f875742068 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1116,6 +1116,21 @@ CREATE VIEW pg_stat_bgwriter AS
         pg_stat_get_buf_alloc() AS buffers_alloc,
         pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
 
index 5c8e8336bf..6a5a1dc79f 100644
 
+/*
+			for (IOObject io_obj = IOOBJECT_FIRST;
 
index a969ae63eb..dd5ddffc4d 100644
  
 (1 row)

From f8c9077631169a778c893fd16b7a973ad5725f2a Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Fri, 9 Dec 2022 18:23:19 -0800
Subject: [PATCH v47 1/5] pgindent and some manual cleanup in pgstat related
 code

---

 
 
 		/*
 
 		if (!use_bsearch)
 
 		/*
 
 		/*
 
 		/*
 
 		if (!use_bsearch)
 
 		/*
 
 	/*

 
 Datum

+
 /*
 
+
 /*

From 62beac2b02e1b724f96533d81515b2e9b3d1f5d9 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 9 Jan 2023 14:42:53 -0500
Subject: [PATCH v47 5/5] pg_stat_io documentation

index 8d51ca3773..ec8dba00f9 100644
 
+     <row>
 
   <para>
 
  </sect2>
@@ -3643,6 +3649,290 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
     <structfield>last_archived_wal</structfield> have also been successfully
     archived.
   </para>
+ </sect2>
+
+ <sect2 id="monitoring-pg-stat-io-view">
+  <title><structname>pg_stat_io</structname></title>
+
+  <indexterm>
+   <primary>pg_stat_io</primary>
+  </indexterm>
+
+  <para>
+   The <structname>pg_stat_io</structname> view will contain one row for each
+   combination of backend type, I/O context, and target I/O object showing
+   cluster-wide I/O statistics. Combinations which do not make sense are
+   omitted.
+  </para>
+
+  <para>
+   Currently, I/O on relations (e.g. tables, indexes) is tracked. However,
+   relation I/O which bypasses shared buffers (e.g. when moving a table from one
+   tablespace to another) is currently not tracked.
+  </para>
+
+  <table id="pg-stat-io-view" xreflabel="pg_stat_io">
+   <title><structname>pg_stat_io</structname> View</title>
+   <tgroup cols="1">
+    <thead>
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        Column Type
+       </para>
+       <para>
+        Description
+       </para>
+      </entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>backend_type</structfield> <type>text</type>
+       </para>
+       <para>
+        Type of backend (e.g. background worker, autovacuum worker). See <link
+        linkend="monitoring-pg-stat-activity-view">
+        <structname>pg_stat_activity</structname></link> for more information
+        on <varname>backend_type</varname>s. Some
+        <varname>backend_type</varname>s do not accumulate I/O operation
+        statistics and will not be included in the view.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>io_context</structfield> <type>text</type>
+       </para>
+       <para>
+        The context of an I/O operation. Possible values are:
+       </para>
+       <itemizedlist>
+        <listitem>
+         <para>
+          <literal>normal</literal>: The default or standard
+          <varname>io_context</varname> for a type of I/O operation. For
+          example, by default, relation data is read into and written out from
+          shared buffers. Thus, reads and writes of relation data to and from
+          shared buffers are tracked in <varname>io_context</varname>
+          <literal>normal</literal>.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>vacuum</literal>: I/O operations performed outside of shared
+          buffers while vacuuming and analyzing permanent relations.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>bulkread</literal>: Certain large read I/O operations
+          done outside of shared buffers, for example, a sequential scan of a
+          large table.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>bulkwrite</literal>: Certain large write I/O operations
+          done outside of shared buffers, such as <command>COPY</command>.
+         </para>
+        </listitem>
+       </itemizedlist>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>io_object</structfield> <type>text</type>
+       </para>
+       <para>
+        Target object of an I/O operation. Possible values are:
+       <itemizedlist>
+        <listitem>
+         <para>
+          <literal>relation</literal>: Permanent relations.
+         </para>
+        </listitem>
+        <listitem>
+         <para>
+          <literal>temp relation</literal>: Temporary relations.
+         </para>
+        </listitem>
+       </itemizedlist>
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>reads</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of read operations, each of the size specified in
+        <varname>op_bytes</varname>.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>writes</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of write operations, each of the size specified in
+        <varname>op_bytes</varname>.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>extends</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of relation extend operations, each of the size specified in
+        <varname>op_bytes</varname>.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>op_bytes</structfield> <type>bigint</type>
+       </para>
+       <para>
+        The number of bytes per unit of I/O read, written, or extended.
+       </para>
+       <para>
+        Relation data reads, writes, and extends are done in
+        <varname>block_size</varname> units, derived from the build-time
+        parameter <symbol>BLCKSZ</symbol>, which is <literal>8192</literal> by
+        default.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>evictions</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of times a block has been written out from a shared or local
+        buffer in order to make it available for another use.
+       </para>
+       <para>
+        In <varname>io_context</varname> <literal>normal</literal>, this counts
+        the number of times a block was evicted from a buffer and replaced with
+        another block. In <varname>io_context</varname>s
+        <literal>bulkwrite</literal>, <literal>bulkread</literal>, and
+        <literal>vacuum</literal>, this counts the number of times a block was
+        evicted from shared buffers in order to add the shared buffer to a
+        separate, size-limited ring buffer for use in a bulk I/O operation.
+        </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>reuses</structfield> <type>bigint</type>
+       </para>
+       <para>
+        The number of times an existing buffer in a size-limited ring buffer
+        outside of shared buffers was reused as part of an I/O operation in the
+        <literal>bulkread</literal>, <literal>bulkwrite</literal>, or
+        <literal>vacuum</literal> <varname>io_context</varname>s.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>fsyncs</structfield> <type>bigint</type>
+       </para>
+       <para>
+        Number of <literal>fsync</literal> calls. These are only tracked in
+        <varname>io_context</varname> <literal>normal</literal>.
+       </para>
+      </entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry">
+       <para role="column_definition">
+        <structfield>stats_reset</structfield> <type>timestamp with time zone</type>
+       </para>
+       <para>
+        Time at which these statistics were last reset.
+       </para>
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+  </table>
+
+  <para>
+   Some backend types never perform I/O operations in some I/O contexts and/or
+   on some I/O objects. These rows are omitted from the view. For example, the
+   checkpointer does not checkpoint temporary tables, so there will be no rows
+   for <varname>backend_type</varname> <literal>checkpointer</literal> and
+   <varname>io_object</varname> <literal>temp relation</literal>.
+  </para>
+
+  <para>
+   In addition, some I/O operations will never be performed either by certain
+   backend types or in certain I/O contexts or on certain I/O objects. These
+   cells will be NULL. For example, temporary tables are not
+   <literal>fsync</literal>ed, so <varname>fsyncs</varname> will be NULL for
+   <varname>io_object</varname> <literal>temp relation</literal>. Also, the
+   background writer does not perform reads, so <varname>reads</varname> will
+   be NULL in rows for <varname>backend_type</varname> <literal>background
+   writer</literal>.
+  </para>
+
+  <para>
+   <structname>pg_stat_io</structname> can be used to inform database tuning.
+   For example:
+   <itemizedlist>
+    <listitem>
+     <para>
+      A high <varname>evictions</varname> count can indicate that shared
+      buffers should be increased.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Client backends rely on the checkpointer to ensure data is persisted to
+      permanent storage. Large numbers of <varname>fsyncs</varname> by
+      <literal>client backend</literal>s could indicate a misconfiguration of
+      shared buffers or of the checkpointer. More information on configuring
+      the checkpointer can be found in <xref linkend="wal-configuration"/>.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Normally, client backends should be able to rely on auxiliary processes
+      like the checkpointer and the background writer to write out dirty data
+      as much as possible. Large numbers of writes by client backends could
+      indicate a misconfiguration of shared buffers or of the checkpointer.
+      More information on configuring the checkpointer can be found in <xref
+      linkend="wal-configuration"/>.
+     </para>
+    </listitem>
+   </itemizedlist>
+  </para>
+
 
  </sect2>
 
-- 
2.34.1

---

From: Andres Freund <[email-masked]>
Date: Fri, 13 Jan 2023 15:36:00 -0800

Hi,

> > what anyone else will say, but I'd prefer if it started at "0".

The reason for using FIRST is to be able to define the loop variable as the
enum type, without assigning numeric values to an enum var. I prefer it
slightly.

> Subject: [PATCH v47 1/5] pgindent and some manual cleanup in pgstat related

Applied.

> +++ b/src/backend/utils/activity/pgstat.c

Reminder to self: Need to bump PGSTAT_FILE_FORMAT_ID before commit.

Perhaps you could add a note about that to the commit message?

>  		.snapshot_cb = pgstat_checkpointer_snapshot_cb,

That should be "io" now I think?

> + * PgStat_BackendIO, if needed.

Other PgStat_Backend* structs are just for pending data. Perhaps we could
rename it slightly to make that clearer? PgStat_BktypeIO?
PgStat_IOForBackendType? or a similar variation?

> +					backend_io->data[io_context][io_object][io_op] != 0)

Hm, perhaps this could be broken up into multiple lines? Something like

    /* no stats, so nothing to validate */
    if (backend_io->data[io_context][io_object][io_op] == 0)
        continue;

    /* something went wrong if have stats for something not tracked */
    if (!bktype_tracked ||
        !pgstat_tracks_io_op(bktype, io_context, io_object, io_op))
        return false;

> +	PgStat_Counter data[IOCONTEXT_NUM_TYPES][IOOBJECT_NUM_TYPES][IOOP_NUM_TYPES];

Would it bother you if we swapped the order of iocontext and iobject here and
related places? It makes more sense to me semantically, and should now be
pretty easy, code wise.

> +typedef struct PgStatShared_IO

Maybe /* PgStat_IO in shared memory */?

> Subject: [PATCH v47 3/5] pgstat: Count IO for relations

Nearly happy with this now. See one minor nit below.

I don't love the counting in register_dirty_segment() and mdsyncfiletag(), but
I don't have a better idea, and it doesn't seem too horrible.

>  		BufTableDelete(&oldTag, oldHash);

There's no reason to do this while we still hold the buffer partition lock,
right? That's a highly contended lock, and we can just move the counting a few
lines down.

> +		pgstat_count_io_op(IOOP_FSYNC, IOOBJECT_RELATION, IOCONTEXT_NORMAL);

I'd lean towards doing this unconditionally, it's still an fsync if it
failed... Not that it matters.

> Subject: [PATCH v47 4/5] Add system view tracking IO ops per backend type

Note to self + commit message: Remember the need to do a catversion bump.

> +-- pg_stat_io test:
> +-- verify_heapam always uses a BAS_BULKREAD BufferAccessStrategy.

Maybe add that "whereas a sequential scan does not, see ..."?

> This allows

CREATE / DROP TABLESPACE is also pretty expensive, but I don't have a better
idea.

> +-- Create an alternative tablespace and move the heaptest table to it, causing

IIRC the point of that is that it reliably evicts all the buffers from s_b,
correct? If so, mention that?

> +		bktype_tracked = pgstat_tracks_io_bktype(bktype);

How about instead just doing Assert(pgstat_bktype_io_stats_valid(...))? That
deduplicates the logic for the asserts, and avoids doing the full loop when
assertions aren't enabled anyway?

Otherwise, see also the suggestion aout formatting the assertions as I
suggested for 0002.

> +-- checkpoint in the test.

There's a comment about the subsequent checkpoints earlier in the file, and I
think the comment is slightly more precise. Mybe just reference the earlier comment?

> +CREATE TABLESPACE regress_io_stats_tblspc LOCATION '';

Perhaps worth doing this in tablespace.sql, to avoid the additional
checkpoints done as part of CREATE/DROP TABLESPACE?

Or, at least combine this with the CHECKPOINTs above?

> +SET temp_buffers TO '1MB';

I'd set it to the actual minimum '100' (in pages). Perhaps that'd allow to
make test_io_local a bit smaller?

> +  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset

Maybe add something like

SELECT pg_relation_size('test_io_local') / current_setting('block_size')::int8 > 100;

Better toast compression or such could easily make test_io_local smaller than
it's today. Seeing that it's too small would make it easier to understand the
failure.

> +  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset

This could just be one select with multiple columns?

I think if you use something like \gset io_sum_local_after_ you can also avoid
the need to repeat "io_sum_local_" so many times.

> +SELECT :io_sum_local_evictions_after > :io_sum_local_evictions_before;

Similar.

> +SELECT sum(reads) AS io_sum_vac_strategy_reads_before FROM pg_stat_io WHERE io_context = 'vacuum' \gset

There's quite a few more instances of this, so I'll now omit further mentions.

Greetings,

Andres Freund

---

From: Maciek Sakrejda <[email-masked]>
Date: Mon, 16 Jan 2023 13:41:49 -0800

> Attached is v47.

I missed a couple of versions, but I think the docs are clearer now.
I'm torn on losing some of the detail, but overall I do think it's a
good trade-off. Moving some details out to after the table does keep
the bulk of the view documentation more readable, and the "inform
database tuning" part is great. I really like the idea of a separate
Interpreting Statistics section, but for now this works.

>+          buffers while vacuuming and analyzing permanent relations.

Why only permanent relations? Are temporary relations treated
differently? I imagine if someone has a temp-table-heavy workload that
requires regularly vacuuming and analyzing those relations, this point
may be confusing without some additional explanation.

Other than that, this looks great.

Thanks,
Maciek

---

From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 12:22:14 -0500

v48 attached.

> Perhaps you could add a note about that to the commit message?

done

> That should be "io" now I think?

Oh no! I didn't notice this was broken. I've added pg_stat_have_stats()
to the IO stats tests now.

It would be nice if pgstat_get_kind_from_str() could be used in
pg_stat_reset_shared() to avoid having to remember to change both. It
doesn't really work because we want to be able to throw the error
message in pg_stat_reset_shared() when the user input is wrong -- not
the one in pgstat_get_kind_from_str().
Also:
- Since recovery_prefetch doesn't have a statistic kind, it doesn't fit
  well into this paradigm
- Only a subset of the statistics kinds are reset through this function
- bgwriter and checkpointer share a reset target
I added a comment -- perhaps that's all I can do?

On a separate note, should we be setting have_[io/slru/etc]stats to
false in the reset all functions?

> PgStat_IOForBackendType? or a similar variation?

I've done this.

>         !pgstat_tracks_io_op(bktype, io_context, io_object, io_op))

I've done this.

> related places? It makes more sense to me semantically, and should now be

So, thinking about this I started noticing inconsistencies in other
areas around this order:
For example: ordering of objects mentioned in commit messages and comments,
ordering of parameters (like in pgstat_count_io_op() [currently in
reverse order]).

I think we should make a final decision about this ordering and then
make everywhere consistent (including ordering in the view).

Currently the order is:
BackendType
  IOContext
    IOObject
      IOOp

You are suggesting this order:
BackendType
  IOObject
    IOContext
      IOOp

Could you explain what you find more natural about this ordering (as I
find the other more natural)?

This is one possible natural sentence with these objects:

During COPY, a client backend may read in data from a permanent
relation.
This order is:
IOContext
  BackendType
    IOOp
      IOObject

I think English sentences are often structured subject, verb, object --
but in our case, we have an extra thing that doesn't fit neatly
(IOContext). Also, IOOp in a sentence would be in the middle (as the
verb). I made it last because a) it feels like the smallest unit b) it
would make the code a lot more annoying if it wasn't last.

WRT IOObject and IOContext, is there a future case for which having
IOObject first will be better or lead to fewer mistakes?

I actually see loads of places where this needs to be made consistent.

> Maybe /* PgStat_IO in shared memory */?

updated.

> I don't have a better idea, and it doesn't seem too horrible.

You don't like it because such things shouldn't be in md.c -- since we
went to the trouble of having function pointers and making it general?

> right? That's a highly contended lock, and we can just move the counting a few

Thanks, I've done this.

> failed... Not that it matters.

Good point. We still incurred the costs if not benefited from the
effects. I've updated this.

> Note to self + commit message: Remember the need to do a catversion bump.

Noted.

> Maybe add that "whereas a sequential scan does not, see ..."?

Updated.

> CREATE / DROP TABLESPACE is also pretty expensive, but I don't have a better

I've added a comment.

> correct? If so, mention that?

Done.

> assertions aren't enabled anyway?

I've done this and added a comment.

> Or, at least combine this with the CHECKPOINTs above?

I see a checkpoint is requested when dropping the tablespace if not all
the files in it are deleted. It seems like if the DROP TABLE for the
permanent table is before the explicit checkpoints in the test, then the
DROP TABLESPACE will not cause an additional checkpoint. Is this what
you are suggesting? Dropping the temporary table should not have an
effect on this.

> make test_io_local a bit smaller?

I've done this.

> it's today. Seeing that it's too small would make it easier to understand the

Good idea. So, I used pg_table_size() because it seems like
pg_relation_size() does not include the toast relations. However, I'm
not sure this is a good idea, because pg_table_size() includes FSM and
visibility map. Should I write a query to get the toast relation name
and add pg_relation_size() of that relation and the main relation?

> the need to repeat "io_sum_local_" so many times.

Thanks. I didn't realize. I've fixed this throughout the test file.

> may be confusing without some additional explanation.

Ah, yes. This is a bit confusing. We don't use buffer access strategies
when operating on temp relations, so vacuuming them is counted in IO
Context normal. I've added this information to the docs but now that
definition is a bit long. Perhaps it should be a note? That seems like
it would draw too much attention to this detail, though...

- Melanie
From 7a7a254c218df04f513cef4b7b2c38725d58a8a4 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 17:20:50 -0500
Subject: [PATCH v48 2/4] pgstat: Count IO for relations

 src/backend/storage/smgr/md.c         |  24 ++++++
 6 files changed, 183 insertions(+), 36 deletions(-)

index 8075828e8a..484f422b72 100644

 	if (isLocalBuf)
 	{

 	if (isExtend)
 	{

 

 

 
 				ScheduleBufferTagForWriteback(&BackendWritebackContext,
@@ -1450,6 +1483,28 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,

 	 * Buffer contents are currently invalid.  Try to obtain the right to
 	 * start I/O.  If StartBufferIO returns false, then someone else managed

 
 
+	/*

 
 
 /*
 
+#include "pgstat.h"
 
-	/*
 
+	*from_ring = false;
+
 	/*
 
 	/*
 
 /*
 
 	/*

+/*

 
+	/*

 
index 60c9905eff..58ae2af55a 100644
 
+		/*

@@ -1410,6 +1432,8 @@ mdsyncfiletag(const FileTag *ftag, char *path)
 	if (need_to_close)
 		FileClose(file);

+#include "pgstat.h"
 
 /* freelist.c */

 typedef void *Block;

From 879cce732348d599445802f00e8650ee521fb239 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 9 Jan 2023 14:42:25 -0500
Subject: [PATCH v48 3/4] Add system view tracking IO ops per backend type

Catalog version should be bumped.

Author: Melanie Plageman <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 contrib/amcheck/expected/check_heap.out |  34 ++++
 contrib/amcheck/sql/check_heap.sql      |  27 +++
 src/backend/catalog/system_views.sql    |  15 ++
 src/backend/utils/adt/pgstatfuncs.c     | 140 ++++++++++++++
 src/include/catalog/pg_proc.dat         |   9 +
 src/test/regress/expected/rules.out     |  12 ++
 src/test/regress/expected/stats.out     | 234 ++++++++++++++++++++++++
 src/test/regress/sql/stats.sql          | 148 +++++++++++++++
 src/tools/pgindent/typedefs.list        |   1 +
 9 files changed, 620 insertions(+)

diff --git a/contrib/amcheck/expected/check_heap.out b/contrib/amcheck/expected/check_heap.out
index c010361025..e4785141a6 100644
--- a/contrib/amcheck/expected/check_heap.out
+++ b/contrib/amcheck/expected/check_heap.out
@@ -66,6 +66,22 @@ SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'ALL-VISIBLE');
 INSERT INTO heaptest (a, b)
 	(SELECT gs, repeat('x', gs)
 		FROM generate_series(1,50) gs);
+-- pg_stat_io test:
+-- verify_heapam always uses a BAS_BULKREAD BufferAccessStrategy, whereas a
+-- sequential scan does so only if the table is large enough when compared to
+-- shared buffers (see initscan()). CREATE DATABASE ... also unconditionally
+-- uses a BAS_BULKREAD strategy, but we have chosen to use a tablespace and
+-- verify_heapam to provide coverage instead of adding another expensive
+-- operation to the main regression test suite.
+--
+-- Create an alternative tablespace and move the heaptest table to it, causing
+-- it to be rewritten and all the blocks to reliably evicted from shared
+-- buffers -- guaranteeing actual reads when we next select from it.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_test_stats_tblspc LOCATION '';
+SELECT sum(reads) AS stats_bulkreads_before
+  FROM pg_stat_io WHERE io_context = 'bulkread' \gset
+ALTER TABLE heaptest SET TABLESPACE regress_test_stats_tblspc;
 -- Check that valid options are not rejected nor corruption reported
 -- for a non-empty table
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'none');
@@ -88,6 +104,23 @@ SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 0, endblock :=
 -------+--------+--------+-----
 (0 rows)
 
@@ -195,6 +228,7 @@ ERROR:  cannot check relation "test_foreign_table"
index 298de6886a..6794ca4eb0 100644
@@ -20,11 +20,29 @@ SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'NONE');
 
+
 
+-- pg_stat_io test:
+-- verify_heapam always uses a BAS_BULKREAD BufferAccessStrategy, whereas a
+-- sequential scan does so only if the table is large enough when compared to
+-- shared buffers (see initscan()). CREATE DATABASE ... also unconditionally
+-- uses a BAS_BULKREAD strategy, but we have chosen to use a tablespace and
+-- verify_heapam to provide coverage instead of adding another expensive
+-- operation to the main regression test suite.
+--
+-- Create an alternative tablespace and move the heaptest table to it, causing
+-- it to be rewritten and all the blocks to reliably evicted from shared
+-- buffers -- guaranteeing actual reads when we next select from it.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_test_stats_tblspc LOCATION '';
+SELECT sum(reads) AS stats_bulkreads_before
+  FROM pg_stat_io WHERE io_context = 'bulkread' \gset
+ALTER TABLE heaptest SET TABLESPACE regress_test_stats_tblspc;
+
 -- Check that valid options are not rejected nor corruption reported
 -- for a non-empty table
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'none');
@@ -32,6 +50,14 @@ SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'all-frozen');
 SELECT * FROM verify_heapam(relation := 'heaptest', skip := 'all-visible');
 SELECT * FROM verify_heapam(relation := 'heaptest', startblock := 0, endblock := 0);

 -- verify permissions are checked (error due to function not callable)
@@ -110,6 +136,7 @@ SELECT * FROM verify_heapam('test_foreign_table',
 
 -- cleanup
 DROP TABLE heaptest;
 
+CREATE VIEW pg_stat_io AS
+SELECT
+       b.backend_type,
+       b.io_context,
+       b.io_object,
+       b.reads,
+       b.writes,
+       b.extends,
+       b.op_bytes,
+       b.evictions,
+       b.reuses,
+       b.fsyncs,
+       b.stats_reset
+FROM pg_stat_get_io() b;
+
 CREATE VIEW pg_stat_wal AS
     SELECT
         w.wal_records,
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 6df9f06a20..284bb2a698 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1234,6 +1234,146 @@ pg_stat_get_buf_alloc(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_bgwriter()->buf_alloc);
 }
 
+/*
+		PgStat_BktypeIO *bktype_stats = &backends_io_stats->stats[bktype];
+		 * In Assert builds, we can afford an extra loop through all of the
+		 * counters checking that only expected stats are non-zero, since it
+		 * keeps the non-Assert code cleaner.
+		Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+		 * representing them in the view altogether.
+				if (!pgstat_tracks_io_object(bktype, io_context, io_obj))
+					 * Some combinations of BackendType and IOOp, of IOContext
+					 * and IOOp, and of IOObject and IOOp are not tracked. Set
+					 * these cells in the view NULL.
+					if (nulls[col_idx])
 
index 1d84407a03..3bd4e66fa8 100644
@@ -1126,4 +1126,238 @@ SELECT pg_stat_get_subscription_stats(NULL);
  
 (1 row)
 
+-- Test that the following operations are tracked in pg_stat_io:
+-- - reads of target blocks into shared buffers
+-- - writes of shared buffers to permanent storage
+-- - extends of relations using shared buffers
+-- - fsyncs done to ensure the durability of data dirtying shared buffers
+-- There is no test for blocks evicted from shared buffers, because we cannot
+-- be sure of the state of shared buffers at the point the test is run.
+-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
+-- extends.
+SELECT sum(extends) AS io_sum_shared_before_extends
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+CREATE TABLE test_io_shared(a int);
+INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(extends) AS io_sum_shared_after_extends
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
+-- and fsyncs.
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'relation' \gset io_sum_shared_before_
+-- See comment above for rationale for two explicit CHECKPOINTs.
+CHECKPOINT;
+CHECKPOINT;
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'relation' \gset io_sum_shared_after_
+SELECT :io_sum_shared_after_writes > :io_sum_shared_before_writes;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT current_setting('fsync') = 'off'
+  OR :io_sum_shared_after_fsyncs > :io_sum_shared_before_fsyncs;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Change the tablespace so that the table is rewritten directly, then SELECT
+-- from it to cause it to be read back into shared buffers.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_io_stats_tblspc LOCATION '';
+SELECT sum(reads) AS io_sum_shared_before_reads
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+ALTER TABLE test_io_shared SET TABLESPACE regress_io_stats_tblspc;
+-- SELECT from the table so that the data is read into shared buffers and
+-- io_context 'normal', io_object 'relation' reads are counted.
+SELECT COUNT(*) FROM test_io_shared;
+ count 
+-------
+   100
+(1 row)
+
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(reads) AS io_sum_shared_after_reads
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_after_reads > :io_sum_shared_before_reads;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_shared;
+-- Test that the follow IOCONTEXT_LOCAL IOOps are tracked in pg_stat_io:
+-- - eviction of local buffers in order to reuse them
+-- - reads of temporary table blocks into local buffers
+-- - writes of local buffers to permanent storage
+-- - extends of temporary tables
+-- Set temp_buffers to its minimum so that we can trigger writes with fewer
+-- inserted tuples. Do so in a new session in case temporary tables have been
+-- accessed by previous tests in this session.
+\c
+SET temp_buffers TO 100;
+CREATE TEMPORARY TABLE test_io_local(a int, b TEXT);
+SELECT sum(extends) AS extends, sum(evictions) AS evictions, sum(writes) AS writes
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'temp relation' \gset io_sum_local_before_
+-- Insert tuples into the temporary table, generating extends in the stats.
+-- Insert enough values that we need to reuse and write out dirty local
+-- buffers, generating evictions and writes.
+INSERT INTO test_io_local SELECT generate_series(1, 5000) as id, repeat('a', 200);
+-- Ensure the table is large enough to exceed our temp_buffers setting.
+SELECT pg_table_size('test_io_local') / current_setting('block_size')::int8 > 100;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT sum(reads) AS io_sum_local_before_reads
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+-- Read in evicted buffers, generating reads.
+SELECT COUNT(*) FROM test_io_local;
+ count 
+-------
+  5000
+(1 row)
+
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(evictions) AS evictions,
+       sum(reads) AS reads,
+       sum(writes) AS writes,
+       sum(extends) AS extends
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset io_sum_local_after_
+SELECT :io_sum_local_after_evictions > :io_sum_local_before_evictions,
+       :io_sum_local_after_reads > :io_sum_local_before_reads,
+       :io_sum_local_after_writes > :io_sum_local_before_writes,
+       :io_sum_local_after_extends > :io_sum_local_before_extends;
+ ?column? | ?column? | ?column? | ?column? 
+----------+----------+----------+----------
+ t        | t        | t        | t
+(1 row)
+
+-- Change the tablespaces so that the temporary table is rewritten to other
+-- local buffers, exercising a different codepath than standard local buffer
+-- writes.
+ALTER TABLE test_io_local SET TABLESPACE regress_io_stats_tblspc;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(writes) AS io_sum_local_new_tblspc_writes
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT :io_sum_local_new_tblspc_writes > :io_sum_local_after_writes;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_local;
+RESET temp_buffers;
+DROP TABLESPACE regress_io_stats_tblspc;
+-- Test that reuse of strategy buffers and reads of blocks into these reused
+-- buffers while VACUUMing are tracked in pg_stat_io.
+-- Set wal_skip_threshold smaller than the expected size of
+-- test_io_vac_strategy so that, even if wal_level is minimal, VACUUM FULL will
+-- fsync the newly rewritten test_io_vac_strategy instead of writing it to WAL.
+-- Writing it to WAL will result in the newly written relation pages being in
+-- shared buffers -- preventing us from testing BAS_VACUUM BufferAccessStrategy
+-- reads.
+SET wal_skip_threshold = '1 kB';
+SELECT sum(reuses) AS reuses, sum(reads) AS reads
+  FROM pg_stat_io WHERE io_context = 'vacuum' \gset io_sum_vac_strategy_before_
+CREATE TABLE test_io_vac_strategy(a int, b int) WITH (autovacuum_enabled = 'false');
+INSERT INTO test_io_vac_strategy SELECT i, i from generate_series(1, 8000)i;
+-- Ensure that the next VACUUM will need to perform IO by rewriting the table
+-- first with VACUUM (FULL).
+VACUUM (FULL) test_io_vac_strategy;
+VACUUM (PARALLEL 0) test_io_vac_strategy;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(reuses) AS reuses, sum(reads) AS reads
+  FROM pg_stat_io WHERE io_context = 'vacuum' \gset io_sum_vac_strategy_after_
+SELECT :io_sum_vac_strategy_after_reads > :io_sum_vac_strategy_before_reads,
+       :io_sum_vac_strategy_after_reuses > :io_sum_vac_strategy_before_reuses;
+ ?column? | ?column? 
+----------+----------
+ t        | t
+(1 row)
+
+RESET wal_skip_threshold;
+-- Test that extends done by a CTAS, which uses a BAS_BULKWRITE
+-- BufferAccessStrategy, are tracked in pg_stat_io.
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_before
+  FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+CREATE TABLE test_io_bulkwrite_strategy AS SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+ pg_stat_force_next_flush 
+--------------------------
+ 
+(1 row)
+
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_after
+  FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_extends_before;
+ ?column? 
+----------
+ t
+(1 row)
+
+-- Test IO stats reset
+SELECT pg_stat_have_stats('io', 0, 0);
+ pg_stat_have_stats 
+--------------------
+ t
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_pre_reset
+  FROM pg_stat_io \gset
+SELECT pg_stat_reset_shared('io');
+ pg_stat_reset_shared 
+----------------------
+ 
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_post_reset
+  FROM pg_stat_io \gset
+SELECT :io_stats_post_reset < :io_stats_pre_reset;
+ ?column? 
+----------
+ t
+(1 row)
+
 -- End of Stats Test
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index b4d6753c71..163ed38faf 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -536,4 +536,152 @@ SELECT pg_stat_get_replication_slot(NULL);
 SELECT pg_stat_get_subscription_stats(NULL);

+-- Test that the following operations are tracked in pg_stat_io:
+-- - reads of target blocks into shared buffers
+-- - writes of shared buffers to permanent storage
+-- - extends of relations using shared buffers
+-- - fsyncs done to ensure the durability of data dirtying shared buffers
+
+-- There is no test for blocks evicted from shared buffers, because we cannot
+-- be sure of the state of shared buffers at the point the test is run.
+
+-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
+-- extends.
+SELECT sum(extends) AS io_sum_shared_before_extends
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+CREATE TABLE test_io_shared(a int);
+INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+SELECT sum(extends) AS io_sum_shared_after_extends
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
+
+-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
+-- and fsyncs.
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'relation' \gset io_sum_shared_before_
+-- See comment above for rationale for two explicit CHECKPOINTs.
+CHECKPOINT;
+CHECKPOINT;
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'relation' \gset io_sum_shared_after_
+
+SELECT :io_sum_shared_after_writes > :io_sum_shared_before_writes;
+SELECT current_setting('fsync') = 'off'
+  OR :io_sum_shared_after_fsyncs > :io_sum_shared_before_fsyncs;
+
+-- Change the tablespace so that the table is rewritten directly, then SELECT
+-- from it to cause it to be read back into shared buffers.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_io_stats_tblspc LOCATION '';
+SELECT sum(reads) AS io_sum_shared_before_reads
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
+ALTER TABLE test_io_shared SET TABLESPACE regress_io_stats_tblspc;
+-- SELECT from the table so that the data is read into shared buffers and
+-- io_context 'normal', io_object 'relation' reads are counted.
+SELECT COUNT(*) FROM test_io_shared;
+SELECT pg_stat_force_next_flush();
+SELECT sum(reads) AS io_sum_shared_after_reads
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  \gset
+SELECT :io_sum_shared_after_reads > :io_sum_shared_before_reads;
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_shared;
+
+-- Test that the follow IOCONTEXT_LOCAL IOOps are tracked in pg_stat_io:
+-- - eviction of local buffers in order to reuse them
+-- - reads of temporary table blocks into local buffers
+-- - writes of local buffers to permanent storage
+-- - extends of temporary tables
+
+-- Set temp_buffers to its minimum so that we can trigger writes with fewer
+-- inserted tuples. Do so in a new session in case temporary tables have been
+-- accessed by previous tests in this session.
+\c
+SET temp_buffers TO 100;
+CREATE TEMPORARY TABLE test_io_local(a int, b TEXT);
+SELECT sum(extends) AS extends, sum(evictions) AS evictions, sum(writes) AS writes
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'temp relation' \gset io_sum_local_before_
+-- Insert tuples into the temporary table, generating extends in the stats.
+-- Insert enough values that we need to reuse and write out dirty local
+-- buffers, generating evictions and writes.
+INSERT INTO test_io_local SELECT generate_series(1, 5000) as id, repeat('a', 200);
+-- Ensure the table is large enough to exceed our temp_buffers setting.
+SELECT pg_table_size('test_io_local') / current_setting('block_size')::int8 > 100;
+
+SELECT sum(reads) AS io_sum_local_before_reads
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation' \gset
+-- Read in evicted buffers, generating reads.
+SELECT COUNT(*) FROM test_io_local;
+SELECT pg_stat_force_next_flush();
+SELECT sum(evictions) AS evictions,
+       sum(reads) AS reads,
+       sum(writes) AS writes,
+       sum(extends) AS extends
+  FROM pg_stat_io
+  WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset io_sum_local_after_
+SELECT :io_sum_local_after_evictions > :io_sum_local_before_evictions,
+       :io_sum_local_after_reads > :io_sum_local_before_reads,
+       :io_sum_local_after_writes > :io_sum_local_before_writes,
+       :io_sum_local_after_extends > :io_sum_local_before_extends;
+
+-- Change the tablespaces so that the temporary table is rewritten to other
+-- local buffers, exercising a different codepath than standard local buffer
+-- writes.
+ALTER TABLE test_io_local SET TABLESPACE regress_io_stats_tblspc;
+SELECT pg_stat_force_next_flush();
+SELECT sum(writes) AS io_sum_local_new_tblspc_writes
+  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'temp relation'  \gset
+SELECT :io_sum_local_new_tblspc_writes > :io_sum_local_after_writes;
+-- Drop the table so we can drop the tablespace later.
+DROP TABLE test_io_local;
+RESET temp_buffers;
+DROP TABLESPACE regress_io_stats_tblspc;
+
+-- Test that reuse of strategy buffers and reads of blocks into these reused
+-- buffers while VACUUMing are tracked in pg_stat_io.
+
+-- Set wal_skip_threshold smaller than the expected size of
+-- test_io_vac_strategy so that, even if wal_level is minimal, VACUUM FULL will
+-- fsync the newly rewritten test_io_vac_strategy instead of writing it to WAL.
+-- Writing it to WAL will result in the newly written relation pages being in
+-- shared buffers -- preventing us from testing BAS_VACUUM BufferAccessStrategy
+-- reads.
+SET wal_skip_threshold = '1 kB';
+SELECT sum(reuses) AS reuses, sum(reads) AS reads
+  FROM pg_stat_io WHERE io_context = 'vacuum' \gset io_sum_vac_strategy_before_
+CREATE TABLE test_io_vac_strategy(a int, b int) WITH (autovacuum_enabled = 'false');
+INSERT INTO test_io_vac_strategy SELECT i, i from generate_series(1, 8000)i;
+-- Ensure that the next VACUUM will need to perform IO by rewriting the table
+-- first with VACUUM (FULL).
+VACUUM (FULL) test_io_vac_strategy;
+VACUUM (PARALLEL 0) test_io_vac_strategy;
+SELECT pg_stat_force_next_flush();
+SELECT sum(reuses) AS reuses, sum(reads) AS reads
+  FROM pg_stat_io WHERE io_context = 'vacuum' \gset io_sum_vac_strategy_after_
+SELECT :io_sum_vac_strategy_after_reads > :io_sum_vac_strategy_before_reads,
+       :io_sum_vac_strategy_after_reuses > :io_sum_vac_strategy_before_reuses;
+RESET wal_skip_threshold;
+
+-- Test that extends done by a CTAS, which uses a BAS_BULKWRITE
+-- BufferAccessStrategy, are tracked in pg_stat_io.
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_before
+  FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+CREATE TABLE test_io_bulkwrite_strategy AS SELECT i FROM generate_series(1,100)i;
+SELECT pg_stat_force_next_flush();
+SELECT sum(extends) AS io_sum_bulkwrite_strategy_extends_after
+  FROM pg_stat_io WHERE io_context = 'bulkwrite' \gset
+SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_extends_before;
+
+-- Test IO stats reset
+SELECT pg_stat_have_stats('io', 0, 0);
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_pre_reset
+  FROM pg_stat_io \gset
+SELECT pg_stat_reset_shared('io');
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) AS io_stats_post_reset
+  FROM pg_stat_io \gset
+SELECT :io_stats_post_reset < :io_stats_pre_reset;
+
 -- End of Stats Test
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 1be6e07980..a399e0a5e4 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -3377,6 +3377,7 @@ intset_internal_node
 intset_leaf_node
 intset_node
 intvKEY
+io_stat_col
 itemIdCompact
 itemIdCompactData
 iterator
-- 
2.34.1

From 21d63527d9e62afb5649bcbf162c3be860408f66 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 9 Jan 2023 14:42:53 -0500
Subject: [PATCH v48 4/4] pg_stat_io documentation

Author: Melanie Plageman <[email-masked]>
Author: Samay Sharma <[email-masked]>
Reviewed-by: Maciek Sakrejda <[email-masked]>
Reviewed-by: Lukas Fittl <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Reviewed-by: Justin Pryzby <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 doc/src/sgml/monitoring.sgml | 321 +++++++++++++++++++++++++++++++++--
 1 file changed, 307 insertions(+), 14 deletions(-)

index 8d51ca3773..d0ca41e204 100644
 
+     <row>
 
   <para>
 
  </sect2>
@@ -3643,6 +3649,293 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
+          buffers while vacuuming and analyzing permanent relations. Temporary
+          table vacuums use the same local buffer pool as other temporary table
+          IO operations and are tracked in <varname>io_context</varname>
 
  </sect2>
 
-- 
2.34.1

From f77f8e0eeb4377cbffd2d52c9455f05e41468bec Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Wed, 4 Jan 2023 17:20:41 -0500
Subject: [PATCH v48 1/4] pgstat: Infrastructure to track IO operations

PGSTAT_FILE_FORMAT_ID should be bumped with this commit.

Author: Melanie Plageman <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Reviewed-by: Justin Pryzby <[email-masked]>
Reviewed-by: Kyotaro Horiguchi <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 doc/src/sgml/monitoring.sgml                  |   2 +
 src/backend/utils/activity/Makefile           |   1 +
 src/backend/utils/activity/meson.build        |   1 +
 src/backend/utils/activity/pgstat.c           |  26 ++
 src/backend/utils/activity/pgstat_bgwriter.c  |   7 +-
 .../utils/activity/pgstat_checkpointer.c      |   7 +-
 src/backend/utils/activity/pgstat_io.c        | 385 ++++++++++++++++++
 src/backend/utils/activity/pgstat_relation.c  |  15 +-
 src/backend/utils/activity/pgstat_shmem.c     |   4 +
 src/backend/utils/activity/pgstat_wal.c       |   4 +-
 src/backend/utils/adt/pgstatfuncs.c           |  11 +-
 src/include/miscadmin.h                       |   2 +
 src/include/pgstat.h                          |  68 ++++
 src/include/utils/pgstat_internal.h           |  30 ++
 src/tools/pgindent/typedefs.list              |   6 +
 15 files changed, 562 insertions(+), 7 deletions(-)
 create mode 100644 src/backend/utils/activity/pgstat_io.c

index 0fa5370bcd..60fc4e761f 100644
 
+	[PGSTAT_KIND_IO] = {
+		.name = "io",
+
+		.fixed_amount = true,
+

 
+	/* flush IO stats */

+	/*
 
+	/*
+	 * Read IO stats struct
+	 */

 /*
 
 /*

 /*
 
 /*
index 0000000000..9859db0581
@@ -0,0 +1,385 @@
+static PgStat_BktypeIO PendingIOStats;
+ * passed-in PgStat_BktypeIO must contain stats from the BackendType specified
+ * PgStat_BktypeIO, if needed.
+pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
+				/* No stats, so nothing to validate */
+				if (backend_io->data[io_context][io_object][io_op] == 0)
+				/* There are stats and there shouldn't be */
+					!pgstat_tracks_io_op(bktype, io_context, io_object, io_op))
+	PgStat_BktypeIO *bktype_shstats;
+		PgStat_BktypeIO *bktype_shstats = &pgStatLocal.shmem->io.stats.stats[i];
+		 * Use the lock in the first BackendType's PgStat_BktypeIO to protect
+		PgStat_BktypeIO *bktype_shstats = &pgStatLocal.shmem->io.stats.stats[i];
+		PgStat_BktypeIO *bktype_snap = &pgStatLocal.snapshot.io.stats[i];
+		 * Use the lock in the first BackendType's PgStat_BktypeIO to protect
+* When adding a new BackendType, also consider adding relevant restrictions to
+* pgstat_tracks_io_object() and pgstat_tracks_io_op().
+ * occur in certain IOContexts or on certain IOObjects. Check that the given
+ * IOOp is valid for the given BackendType in the given IOContext and on the
+ * given IOObject. Note that there are currently no cases of an IOOp being
+ * invalid for a particular BackendType only within a certain IOContext and/or
+ * only on a certain IOObject.
 
 /*

 /*

 /*
 
 /*
 
 /*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 58bd1360b9..6df9f06a20 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1576,7 +1576,12 @@ pg_stat_reset(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
-/* Reset some shared cluster-wide counters */
+/*
+ * Reset some shared cluster-wide counters
+ *
+ * When adding a new reset target, ideally the name should match that in
+ * pgstat_kind_infos, if relevant.
+ */
 Datum
 pg_stat_reset_shared(PG_FUNCTION_ARGS)
 {
@@ -1593,6 +1598,8 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
 		pgstat_reset_of_kind(PGSTAT_KIND_BGWRITER);
 		pgstat_reset_of_kind(PGSTAT_KIND_CHECKPOINTER);
 	}
+	else if (strcmp(target, "io") == 0)
+		pgstat_reset_of_kind(PGSTAT_KIND_IO);
 	else if (strcmp(target, "recovery_prefetch") == 0)
 		XLogPrefetchResetStats();
 	else if (strcmp(target, "wal") == 0)
@@ -1601,7 +1608,7 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 				 errmsg("unrecognized reset target: \"%s\"", target),
-				 errhint("Target must be \"archiver\", \"bgwriter\", \"recovery_prefetch\", or \"wal\".")));
+				 errhint("Target must be \"archiver\", \"bgwriter\", \"io\", \"recovery_prefetch\", or \"wal\".")));
 
 	PG_RETURN_VOID();
 }

index 5e3326a3b9..d0fa47ec07 100644
 
+
+/*
+typedef struct PgStat_BktypeIO
+} PgStat_BktypeIO;
+	PgStat_BktypeIO stats[BACKEND_NUM_TYPES];
@@ -453,6 +503,24 @@ extern void pgstat_report_checkpointer(void);

+/*
+extern bool pgstat_bktype_io_stats_valid(PgStat_BktypeIO *context_ops,
index 12fd51f1ae..6badb2fde4 100644
 
+/* Shared-memory ready PgStat_IO */

+	PgStat_IO	io;
+
 
 	PgStat_WalStats wal;
@@ -549,6 +563,15 @@ extern void pgstat_database_reset_timestamp_cb(PgStatShared_Common *header, Time
 extern bool pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);

+/*
@@ -643,6 +666,13 @@ extern void pgstat_create_transactional(PgStat_Kind kind, Oid dboid, Oid objoid)

+/*
+ * Variables in pgstat_io.c
+ */
+
+extern PGDLLIMPORT bool have_iostats;
+
+
 /*
  * Variables in pgstat_slru.c
  */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 23bafec5f7..1be6e07980 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1106,7 +1106,10 @@ ID
 INFIX
 INT128
 INTERFACE_INFO
+IOContext
 IOFuncSelector
+IOObject
+IOOp
 IPCompareMethod
 ITEM
 IV
@@ -2016,6 +2019,7 @@ PgStatShared_Common
 PgStatShared_Database
 PgStatShared_Function
 PgStatShared_HashEntry
+PgStatShared_IO
 PgStatShared_Relation
 PgStatShared_ReplSlot
 PgStatShared_SLRU
@@ -2025,6 +2029,7 @@ PgStat_ArchiverStats
 PgStat_BackendFunctionEntry
 PgStat_BackendSubEntry
 PgStat_BgWriterStats
+PgStat_BktypeIO
 PgStat_CheckpointerStats
 PgStat_Counter
 PgStat_EntryRef
@@ -2033,6 +2038,7 @@ PgStat_FetchConsistency
 PgStat_FunctionCallUsage
 PgStat_FunctionCounts
 PgStat_HashKey
+PgStat_IO
 PgStat_Kind
 PgStat_KindInfo
 PgStat_LocalState
-- 
2.34.1

---

From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 11:12:32 -0800

Hi,

On 2023-01-17 12:22:14 -0500, Melanie Plageman wrote:
> pg_stat_reset_shared() to avoid having to remember to change both.

It's hard to make that work, because of the historical behaviour of that
function :(

> - Since recovery_prefetch doesn't have a statistic kind, it doesn't fit

I think that needs a rework anyway - it went in at about the same time as the
shared mem stats patch, so it doesn't quite cohere.

> false in the reset all functions?

That'd not work reliably, because other backends won't do the same. I don't
see a benefit in doing it differently in the local connection than the other
connections.

> find the other more natural)?

The object we're performing IO on determines more things than the context. So
it just seems like the natural hierarchical fit. The context is a sub-category
of the object. Consider how it'll look like if we also have objects for 'wal',
'temp files'. It'll make sense to group by just the object, but it won't make
sense to group by just the context.

If it were trivial to do I'd use a different IOContext for each IOObject. But
it'd make it much harder. So there'll just be a bunch of values of IOContext
that'll only be used for one or a subset of the IOObjects.

The reason to put BackendType at the top is pragmatic - one backend is of a
single type, but can do IO for all kinds of objects/contexts. So any other
hierarchy would make the locking etc much harder.

> (IOContext).

"..., to avoid polluting the buffer cache it uses the bulk (read|write)
strategy".

> Also, IOOp in a sentence would be in the middle (as the

Yea, I think pragmatically that is the right choice.

> went to the trouble of having function pointers and making it general?

It's more of a gut feeling than well reasoned ;)

> DROP TABLESPACE will not cause an additional checkpoint.

Unfortunately, that's not how it works :(. See the comment above mdunlink():

> * For regular relations, we don't unlink the first segment file of the rel,
> * but just truncate it to zero length, and record a request to unlink it after
> * the next checkpoint.  Additional segments can be unlinked immediately,
> * however.  Leaving the empty file in place prevents that relfilenumber
> * from being reused.  The scenario this protects us from is:
> ...

> Is this what you are suggesting? Dropping the temporary table should not
> have an effect on this.

I was wondering about simply moving that portion of the test to
tablespace.sql, where we already created a tablespace.

An alternative would be to propose splitting tablespace.sql into one portion
running at the start of parallel_schedule, and one at the end. Historically,
we needed tablespace.sql to be optional due to causing problems when
replicating to another instance on the same machine, but now we have
allow_in_place_tablespaces.

> and add pg_relation_size() of that relation and the main relation?

I think it's the right thing to just include the relation size. Your queries
IIRC won't use the toast table or other forks. So I'd leave it at just
pg_relation_size().

Greetings,

Andres Freund

---

From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 17:00:34 -0500

v49 attached

> Yea, I think pragmatically that is the right choice.

I have changed the order and updated all the places using
PgStat_BktypeIO as well as in all locations in which it should be
ordered for consistency (that I could find in the pass I did) -- e.g.
the view definition, function signatures, comments, commit messages,
etc.

> allow_in_place_tablespaces.

It seems like the best way would be to split up the tablespace test file
as you suggested and drop the tablespace at the end of the regression
test suite. There could be other tests that could use a tablespace.
Though what I wrote is kind of tablespace test coverage, if this
rewriting behavior no longer happened when doing alter table set
tablespace, we would want to come up with a new test which exercised
that code to count those IO stats, not simply delete it from the
tablespace tests.

> IIRC won't use the toast table or other forks. So I'd leave it at just

I did notice that this test wasn't using the toast table for the
toastable column -- but you mentioned better toast compression affecting
the future test stability, so I'm confused.

- Melanie
From 2e29ec2d41fee3fd299c271ade82f8270a16474b Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 16:10:34 -0500
Subject: [PATCH v49 1/4] pgstat: Infrastructure to track IO operations

Each BackendType counts IOOps (evict, extend, fsync, read, reuse, and
write) per IOObject (relation, temp relation) per IOContext (normal,
bulkread, bulkwrite, or vacuum) through a call to pgstat_count_io_op().

Because not all BackendType, IOObject, IOContext, IOOp combinations are
valid, the validity of the stats is checked before flushing pending
stats and before reading in the existing stats file to shared memory.

 src/backend/utils/activity/pgstat_io.c        | 386 ++++++++++++++++++
 src/include/pgstat.h                          |  68 +++
 15 files changed, 563 insertions(+), 7 deletions(-)

+	[PGSTAT_KIND_IO] = {
+		.name = "io",
+
+		.fixed_amount = true,
+

 
+	/* flush IO stats */

+	/*
 
+	/*
+	 * Read IO stats struct
+	 */

 /*
 
 /*

 /*
 
 /*
index 0000000000..b606f23eb8
@@ -0,0 +1,386 @@
+ * Check that stats have not been counted for any combination of IOObject,
+ * IOContext, and IOOp which are not tracked for the passed-in BackendType. The
+				if (backend_io->data[io_object][io_context][io_op] == 0)
+					!pgstat_tracks_io_op(bktype, io_object, io_context, io_op))
+pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op)
+	Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+	PendingIOStats.data[io_object][io_context][io_op]++;
+				bktype_shstats->data[io_object][io_context][io_op] +=
+					PendingIOStats.data[io_object][io_context][io_op];
+ * Some BackendTypes do not perform IO on certain IOObjects or in certain
+ * IOContexts. Some IOObjects are never operated on in some IOContexts. Check
+ * that the given BackendType is expected to do IO in the given IOContext and
+ * on the given IOObject and that the given IOObject is expected to be operated
+ * on in the given IOContext.
+pgstat_tracks_io_object(BackendType bktype, IOObject io_object,
+						IOContext io_context)
+pgstat_tracks_io_op(BackendType bktype, IOObject io_object,
+					IOContext io_context, IOOp io_op)
+	if (!pgstat_tracks_io_object(bktype, io_object, io_context))
 
 /*

 /*

 /*
 
 /*
 
 /*

 	PG_RETURN_VOID();
 }

index 5e3326a3b9..9f09caa05f 100644
 
+
+/*
+	PgStat_Counter data[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];

+/*
+ * Functions in pgstat_io.c
+ */
+
+extern bool pgstat_bktype_io_stats_valid(PgStat_BktypeIO *context_ops,
+										 BackendType bktype);
+extern void pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op);
+extern PgStat_IO *pgstat_fetch_stat_io(void);
+extern const char *pgstat_get_io_context_name(IOContext io_context);
+extern const char *pgstat_get_io_object_name(IOObject io_object);
+
+extern bool pgstat_tracks_io_bktype(BackendType bktype);
+extern bool pgstat_tracks_io_object(BackendType bktype,
+									IOObject io_object, IOContext io_context);
+extern bool pgstat_tracks_io_op(BackendType bktype, IOObject io_object,
+								IOContext io_context, IOOp io_op);
+
+
 /*
  * Functions in pgstat_database.c
  */
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 12fd51f1ae..6badb2fde4 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -329,6 +329,17 @@ typedef struct PgStatShared_Checkpointer
 	PgStat_CheckpointerStats reset_offset;
 } PgStatShared_Checkpointer;

 
+	PgStat_IO	io;
+
 
 	PgStat_WalStats wal;

+/*

+/*

From 86be2a8ef4e800061ca57f0ba42ac4ebc0c4ac91 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 16:34:27 -0500
Subject: [PATCH v49 4/4] pg_stat_io documentation

index 8d51ca3773..b875fc3f12 100644
 
+     <row>
 
   <para>
 
  </sect2>
+   combination of backend type, target I/O object, and I/O context, showing
+   Some backend types never perform I/O operations on some I/O objects and/or
+   in some I/O contexts. These rows are omitted from the view. For example, the
+   backend types or on certain I/O objects and/or in certain I/O contexts.
 
  </sect2>
 
-- 
2.34.1

From cb2dd852c8435537ed9a9a148c719e81c0dc22ce Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 16:25:31 -0500
Subject: [PATCH v49 2/4] pgstat: Count IO for relations

 src/backend/storage/buffer/bufmgr.c   | 111 ++++++++++++++++++++++----
 6 files changed, 185 insertions(+), 36 deletions(-)

index 8075828e8a..ff12bc2ba6 100644
+						IOObject io_object, IOContext io_context);

 	if (isLocalBuf)
 	{

 	if (isExtend)
 	{
 
+		pgstat_count_io_op(io_object, io_context, IOOP_EXTEND);
+
 		/*
 		 * NB: we're *not* doing a ScheduleBufferTagForWriteback here;
 		 * although we're essentially performing a write. At least on linux
@@ -1020,6 +1041,8 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,

+			pgstat_count_io_op(io_object, io_context, IOOP_READ);
+
 			if (track_io_timing)
 			{

 

-				FlushBuffer(buf, NULL);
+				FlushBuffer(buf, NULL, IOOBJECT_RELATION, *io_context);
 				LWLockRelease(BufferDescriptorGetContentLock(buf));

 
+		pgstat_count_io_op(IOOBJECT_RELATION, *io_context,
+						   from_ring ? IOOP_REUSE : IOOP_EVICT);
 
-	FlushBuffer(bufHdr, NULL);
+	FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);

@@ -2820,7 +2875,8 @@ BufferGetTag(Buffer buffer, RelFileLocator *rlocator, ForkNumber *forknum,
  * as the second parameter.  If not, pass NULL.
  */
 static void
-FlushBuffer(BufferDesc *buf, SMgrRelation reln)
+FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
+			IOContext io_context)
 {
 	XLogRecPtr	recptr;
 	ErrorContextCallback errcallback;
@@ -2912,6 +2968,26 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln)
 			  bufToWrite,
 			  false);
 
+	/*
+	pgstat_count_io_op(IOOBJECT_RELATION, io_context, IOOP_WRITE);
@@ -3554,6 +3630,8 @@ FlushRelationBuffers(Relation rel)
 
+				pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_WRITE);
+
 				/* Pop the error context stack */
 				error_context_stack = errcallback.previous;
 			}
@@ -3586,7 +3664,8 @@ FlushRelationBuffers(Relation rel)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, RelationGetSmgr(rel));
+			FlushBuffer(bufHdr, RelationGetSmgr(rel), IOCONTEXT_NORMAL, IOOBJECT_RELATION);
+			FlushBuffer(bufHdr, RelationGetSmgr(rel), IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3684,7 +3763,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, srelent->srel);
+			FlushBuffer(bufHdr, srelent->srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3894,7 +3973,7 @@ FlushDatabaseBuffers(Oid dbid)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, NULL);
+			FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3921,7 +4000,7 @@ FlushOneBuffer(Buffer buffer)

-	FlushBuffer(bufHdr, NULL);
+	FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 }
 
 /*
 
+#include "pgstat.h"
 
-	/*
 
+	*from_ring = false;
+
 	/*
 
 	/*
 
 /*
 
 	/*

+/*

index 8372acc383..8e286db5df 100644
 
+	/*
 
+		pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_WRITE);
 		pgBufferUsage.local_blks_written++;
 	}
 
+		pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EVICT);
 	}
 
index 60c9905eff..8da813600c 100644
 
+		/*
 
+		pgstat_count_io_op(IOOBJECT_RELATION, IOCONTEXT_NORMAL, IOOP_FSYNC);

+	pgstat_count_io_op(IOOBJECT_RELATION, IOCONTEXT_NORMAL, IOOP_FSYNC);
+
 	errno = save_errno;
 	return result;
 }
 
+#include "pgstat.h"
 
 /* freelist.c */

 typedef void *Block;

From d40934679b00fd1e157bd6942d7f3faf8be5ea8e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 16:28:27 -0500
Subject: [PATCH v49 3/4] Add system view tracking IO ops per backend type

 src/backend/utils/adt/pgstatfuncs.c     | 141 ++++++++++++++
 9 files changed, 621 insertions(+)

 
+
 
+-- pg_stat_io test:

 
 -- cleanup
 DROP TABLE heaptest;
index d2a8c82900..70699f4b85 100644
 
+CREATE VIEW pg_stat_io AS
+SELECT
+       b.backend_type,
+       b.io_object,
+       b.io_context,
+       b.reads,
+       b.writes,
+       b.extends,
+       b.op_bytes,
+       b.evictions,
+       b.reuses,
+       b.fsyncs,
+       b.stats_reset
+FROM pg_stat_get_io() b;
+
 CREATE VIEW pg_stat_wal AS
     SELECT
         w.wal_records,
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 6df9f06a20..5b79d703b7 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1234,6 +1234,147 @@ pg_stat_get_buf_alloc(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_bgwriter()->buf_alloc);
 }
 
+/*
+				 * Some combinations of BackendType, IOObject, and IOContext
+				if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
+					nulls[col_idx] = !pgstat_tracks_io_op(bktype, io_obj, io_context, io_op);
+						Int64GetDatum(bktype_stats->data[io_obj][io_context][io_op]);
index 3810de7b22..2155d93b44 100644
 
+  proargnames => '{backend_type,io_object,io_context,reads,writes,extends,op_bytes,evictions,reuses,fsyncs,stats_reset}',
index a969ae63eb..8a7ed673c2 100644
+   FROM pg_stat_get_io() b(backend_type, io_object, io_context, reads, writes, extends, op_bytes, evictions, reuses, fsyncs, stats_reset);
index 1d84407a03..46bc79e740 100644
  
 (1 row)
 
+SELECT pg_relation_size('test_io_local') / current_setting('block_size')::int8 > 100;
index b4d6753c71..4465649211 100644

+SELECT pg_relation_size('test_io_local') / current_setting('block_size')::int8 > 100;

---

From: Maciek Sakrejda <[email-masked]>
Date: Tue, 17 Jan 2023 22:10:38 -0800

> it would draw too much attention to this detail, though...

Thanks for clarifying. I think the updated definition still works:
it's still shorter than the `normal` context definition.

---

From: vignesh C <[email-masked]>
Date: Thu, 19 Jan 2023 16:47:57 +0530

> the future test stability, so I'm confused.

The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL commit ID
4f74f5641d53559ec44e74d5bf552e167fdd5d20 ===
=== applying patch
./v49-0003-Add-system-view-tracking-IO-ops-per-backend-type.patch
....
patching file src/test/regress/expected/rules.out
Hunk #1 FAILED at 1876.
1 out of 1 hunk FAILED -- saving rejects to file
src/test/regress/expected/rules.out.rej

Regards,
Vignesh

---

From: Melanie Plageman <[email-masked]>
Date: Thu, 19 Jan 2023 16:28:59 -0500

> [1] - http://cfbot.cputube.org/patch_41_3272.log

Yes, it conflicted with 47bb9db75996232. rebased v50 is attached.

> that code to count those IO stats, not simply delete it from the

I have added a patch to the set which creates the regress_tblspace
(formerly created in tablespace.sq1) in test_setup.sql. I then moved the
tablespace test to the end of the parallel schedule so that my test (and
others) could use the regress_tblspace.

I modified some of the tablespace.sql tests to be more specific in terms
of the objects they are looking for so that tests using the tablespace
are not forced to drop all of the objects they make in the tablespace.

Note that I did not proactively change all tests in tablespace.sql that
may fail in this way -- only those that failed because of the tables I
created (and did not drop) from regress_tblspace.

- Melanie
From 3976128fab1467fde8ee7e1bc1f54b023e96e35d Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Thu, 19 Jan 2023 15:45:54 -0500
Subject: [PATCH v50 1/5] Create regress_tblspc in test_setup

Other tests may want to use a tablespace. Now that we have
allow_in_place_tablespaces, move the tablespace test to the end of the
parallel schedule and create the main tablespace it uses in test setup.

Modify the tablespace tests a bit to check for specific relations in the
test and not simply for the absence or presence of any objects in the
tablespace in case other tests leave objects around in the tablespace.
---
 src/test/regress/expected/tablespace.out | 63 +++++++++++++-----------
 src/test/regress/expected/test_setup.out |  3 ++
 src/test/regress/parallel_schedule       |  9 ++--
 src/test/regress/sql/tablespace.sql      | 40 ++++++++++-----
 src/test/regress/sql/test_setup.sql      |  4 ++
 5 files changed, 73 insertions(+), 46 deletions(-)

diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index c52cf1cfcf..a2aa95bd97 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -22,8 +22,6 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
 
 -- drop the tablespace so we can re-use the location
 DROP TABLESPACE regress_tblspacewith;
--- create a tablespace we can use
-CREATE TABLESPACE regress_tblspace LOCATION '';
 -- This returns a relative path as of an effect of allow_in_place_tablespaces,
 -- masking the tablespace OID used in the path name.
 SELECT regexp_replace(pg_tablespace_location(oid), '(pg_tblspc)/(\d+)', '\1/NNN')
@@ -83,11 +81,14 @@ REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx;
 REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
 ROLLBACK;
 -- no relation moved to the new tablespace
-SELECT c.relname FROM pg_class c, pg_tablespace s
+SELECT c.relname <> 'regress_tblspace_test_tbl_idx',
+       c.relname <> 'regress_tblspace_test_tbl'
+  FROM pg_class c, pg_tablespace s
   WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace';
- relname 
----------
-(0 rows)
+ ?column? | ?column? 
+----------+----------
+ t        | t
+(1 row)
 
 -- check that all indexes are moved to a new tablespace with different
 -- relfilenode.
@@ -102,40 +103,46 @@ SELECT relfilenode as toast_filenode FROM pg_class
        WHERE i.indrelid = c.reltoastrelid AND
              c.relname = 'regress_tblspace_test_tbl') \gset
 REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
-SELECT c.relname FROM pg_class c, pg_tablespace s
-  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
-  ORDER BY c.relname;
-            relname            
--------------------------------
- regress_tblspace_test_tbl_idx
+SELECT 'regress_tblspace_test_tbl_idx' IN
+  (SELECT c.relname
+  FROM pg_class c, pg_tablespace s
+  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace');
+ ?column? 
+----------
+ t
 (1 row)
 
 ALTER TABLE regress_tblspace_test_tbl SET TABLESPACE regress_tblspace;
 ALTER TABLE regress_tblspace_test_tbl SET TABLESPACE pg_default;
-SELECT c.relname FROM pg_class c, pg_tablespace s
-  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
-  ORDER BY c.relname;
-            relname            
--------------------------------
- regress_tblspace_test_tbl_idx
+SELECT 'regress_tblspace_test_tbl_idx' IN
+  (SELECT c.relname
+  FROM pg_class c, pg_tablespace s
+  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace');
+ ?column? 
+----------
+ t
 (1 row)
 
 -- Move back to the default tablespace.
 ALTER INDEX regress_tblspace_test_tbl_idx SET TABLESPACE pg_default;
-SELECT c.relname FROM pg_class c, pg_tablespace s
+SELECT c.relname <> 'regress_tblspace_test_tbl_idx',
+       c.relname <> 'regress_tblspace_test_tbl'
+  FROM pg_class c, pg_tablespace s
   WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
   ORDER BY c.relname;
- relname 
----------
-(0 rows)
+ ?column? | ?column? 
+----------+----------
+ t        | t
+(1 row)
 
 REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE regress_tblspace_test_tbl;
-SELECT c.relname FROM pg_class c, pg_tablespace s
-  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
-  ORDER BY c.relname;
-            relname            
--------------------------------
- regress_tblspace_test_tbl_idx
+SELECT 'regress_tblspace_test_tbl_idx' IN
+  (SELECT c.relname
+  FROM pg_class c, pg_tablespace s
+  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace');
+ ?column? 
+----------
+ t
 (1 row)
 
 SELECT relfilenode = :main_filenode AS main_same FROM pg_class
diff --git a/src/test/regress/expected/test_setup.out b/src/test/regress/expected/test_setup.out
index 391b36d131..4f54fe20ec 100644
--- a/src/test/regress/expected/test_setup.out
+++ b/src/test/regress/expected/test_setup.out
@@ -18,6 +18,9 @@ SET synchronous_commit = on;
 -- and most of the core regression tests still expect that.
 --
 GRANT ALL ON SCHEMA public TO public;
+-- Create a tablespace we can use in tests.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_tblspace LOCATION '';
 --
 -- These tables have traditionally been referenced by many tests,
 -- so create and populate them.  Insert only non-error values here.
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index a930dfe48c..15e015b3d6 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -11,11 +11,6 @@
 # required setup steps
 test: test_setup
 
-# run tablespace by itself, and early, because it forces a checkpoint;
-# we'd prefer not to have checkpoints later in the tests because that
-# interferes with crash-recovery testing.
-test: tablespace
-
 # ----------
 # The first group of parallel tests
 # ----------
@@ -132,3 +127,7 @@ test: event_trigger oidjoins
 
 # this test also uses event triggers, so likewise run it by itself
 test: fast_default
+
+# run tablespace test at the end because it drops the tablespace created during
+# setup that other tests may use.
+test: tablespace
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index 21db433f2a..1e03d679b2 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -20,8 +20,6 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
 -- drop the tablespace so we can re-use the location
 DROP TABLESPACE regress_tblspacewith;
 
--- create a tablespace we can use
-CREATE TABLESPACE regress_tblspace LOCATION '';
 -- This returns a relative path as of an effect of allow_in_place_tablespaces,
 -- masking the tablespace OID used in the path name.
 SELECT regexp_replace(pg_tablespace_location(oid), '(pg_tblspc)/(\d+)', '\1/NNN')
@@ -66,7 +64,9 @@ REINDEX (TABLESPACE regress_tblspace) INDEX regress_tblspace_test_tbl_idx;
 REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
 ROLLBACK;
 -- no relation moved to the new tablespace
-SELECT c.relname FROM pg_class c, pg_tablespace s
+SELECT c.relname <> 'regress_tblspace_test_tbl_idx',
+       c.relname <> 'regress_tblspace_test_tbl'
+  FROM pg_class c, pg_tablespace s
   WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace';
 
 -- check that all indexes are moved to a new tablespace with different
@@ -74,6 +74,7 @@ SELECT c.relname FROM pg_class c, pg_tablespace s
 -- Save first the existing relfilenode for the toast and main relations.
 SELECT relfilenode as main_filenode FROM pg_class
   WHERE relname = 'regress_tblspace_test_tbl_idx' \gset
+
 SELECT relfilenode as toast_filenode FROM pg_class
   WHERE oid =
     (SELECT i.indexrelid
@@ -81,24 +82,37 @@ SELECT relfilenode as toast_filenode FROM pg_class
             pg_index i
        WHERE i.indrelid = c.reltoastrelid AND
              c.relname = 'regress_tblspace_test_tbl') \gset
+
 REINDEX (TABLESPACE regress_tblspace) TABLE regress_tblspace_test_tbl;
-SELECT c.relname FROM pg_class c, pg_tablespace s
-  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
-  ORDER BY c.relname;
+
+SELECT 'regress_tblspace_test_tbl_idx' IN
+  (SELECT c.relname
+  FROM pg_class c, pg_tablespace s
+  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace');
+
 ALTER TABLE regress_tblspace_test_tbl SET TABLESPACE regress_tblspace;
 ALTER TABLE regress_tblspace_test_tbl SET TABLESPACE pg_default;
-SELECT c.relname FROM pg_class c, pg_tablespace s
-  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
-  ORDER BY c.relname;
+
+SELECT 'regress_tblspace_test_tbl_idx' IN
+  (SELECT c.relname
+  FROM pg_class c, pg_tablespace s
+  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace');
+
 -- Move back to the default tablespace.
 ALTER INDEX regress_tblspace_test_tbl_idx SET TABLESPACE pg_default;
-SELECT c.relname FROM pg_class c, pg_tablespace s
+SELECT c.relname <> 'regress_tblspace_test_tbl_idx',
+       c.relname <> 'regress_tblspace_test_tbl'
+  FROM pg_class c, pg_tablespace s
   WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
   ORDER BY c.relname;
+
 REINDEX (TABLESPACE regress_tblspace, CONCURRENTLY) TABLE regress_tblspace_test_tbl;
-SELECT c.relname FROM pg_class c, pg_tablespace s
-  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace'
-  ORDER BY c.relname;
+
+SELECT 'regress_tblspace_test_tbl_idx' IN
+  (SELECT c.relname
+  FROM pg_class c, pg_tablespace s
+  WHERE c.reltablespace = s.oid AND s.spcname = 'regress_tblspace');
+
 SELECT relfilenode = :main_filenode AS main_same FROM pg_class
   WHERE relname = 'regress_tblspace_test_tbl_idx';
 SELECT relfilenode = :toast_filenode as toast_same FROM pg_class
diff --git a/src/test/regress/sql/test_setup.sql b/src/test/regress/sql/test_setup.sql
index 02c0c84c3a..8439b38d21 100644
--- a/src/test/regress/sql/test_setup.sql
+++ b/src/test/regress/sql/test_setup.sql
@@ -23,6 +23,10 @@ SET synchronous_commit = on;
 --
 GRANT ALL ON SCHEMA public TO public;
 
+-- Create a tablespace we can use in tests.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_tblspace LOCATION '';
+
 --
 -- These tables have traditionally been referenced by many tests,
 -- so create and populate them.  Insert only non-error values here.
-- 
2.34.1

From e01f99ceb59fbc85654cf552e4bb527f8076c07a Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 16:10:34 -0500
Subject: [PATCH v50 2/5] pgstat: Infrastructure to track IO operations

index e3a783abd0..c47d057a1d 100644
@@ -5434,6 +5434,8 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
 
+	[PGSTAT_KIND_IO] = {
+		.name = "io",
+
+		.fixed_amount = true,
+

 
+	/* flush IO stats */

+	/*
 
+	/*
+	 * Read IO stats struct
+	 */

 /*
 
 /*

 /*
 
 /*
 
 /*

 /*

 /*
 
 /*
 
 /*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 6737493402..924698e6ae 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1587,7 +1587,12 @@ pg_stat_reset(PG_FUNCTION_ARGS)
 	PG_RETURN_VOID();
 }
 
@@ -1604,6 +1609,8 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
@@ -1612,7 +1619,7 @@ pg_stat_reset_shared(PG_FUNCTION_ARGS)
 
 	PG_RETURN_VOID();
 }

 
+
+/*

+/*

 
+	PgStat_IO	io;
+
 
 	PgStat_WalStats wal;

+/*

+/*
+ * Variables in pgstat_io.c
+ */
+
+extern PGDLLIMPORT bool have_iostats;
+
+
 /*
  * Variables in pgstat_slru.c
  */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 09316039e4..65be0dea1b 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -1106,7 +1106,10 @@ ID
 INFIX
 INT128
 INTERFACE_INFO
+IOContext
 IOFuncSelector
+IOObject
+IOOp
 IPCompareMethod
 ITEM
 IV
@@ -2015,6 +2018,7 @@ PgStatShared_Common
 PgStatShared_Database
 PgStatShared_Function
 PgStatShared_HashEntry
+PgStatShared_IO
 PgStatShared_Relation
 PgStatShared_ReplSlot
 PgStatShared_SLRU
@@ -2024,6 +2028,7 @@ PgStat_ArchiverStats
 PgStat_BackendFunctionEntry
 PgStat_BackendSubEntry
 PgStat_BgWriterStats
+PgStat_BktypeIO
 PgStat_CheckpointerStats
 PgStat_Counter
 PgStat_EntryRef
@@ -2032,6 +2037,7 @@ PgStat_FetchConsistency
 PgStat_FunctionCallUsage
 PgStat_FunctionCounts
 PgStat_HashKey
+PgStat_IO
 PgStat_Kind
 PgStat_KindInfo
 PgStat_LocalState
-- 
2.34.1

From a6a90045ae3b68b0fc627e27f740e27d77ea3810 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 16:34:27 -0500
Subject: [PATCH v50 5/5] pg_stat_io documentation

index c47d057a1d..2f4e6e89bc 100644
 
+     <row>
 
   <para>
 
  </sect2>
@@ -3659,6 +3665,293 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i
 
  </sect2>
 
-- 
2.34.1

From 471d98971d39d33a9e75bc1463f8f7d6c2973dfe Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 16:28:27 -0500
Subject: [PATCH v50 4/5] Add system view tracking IO ops per backend type

Author: Melanie Plageman <[email-masked]>
Reviewed-by: Andres Freund <[email-masked]>
Discussion: https://www.postgresql.org/message-id/flat/20200124195226.lth52iydq2n2uilq%40alap3.anarazel.de
---
 contrib/amcheck/expected/check_heap.out |  34 ++++
 contrib/amcheck/sql/check_heap.sql      |  27 +++
 src/backend/catalog/system_views.sql    |  15 ++
 src/backend/utils/adt/pgstatfuncs.c     | 141 +++++++++++++++
 src/include/catalog/pg_proc.dat         |   9 +
 src/test/regress/expected/rules.out     |  12 ++
 src/test/regress/expected/stats.out     | 227 ++++++++++++++++++++++++
 src/test/regress/sql/stats.sql          | 141 +++++++++++++++
 src/tools/pgindent/typedefs.list        |   1 +
 9 files changed, 607 insertions(+)

 
+
 
+-- pg_stat_io test:

 
 -- cleanup
 DROP TABLE heaptest;
index 8608e3fa5b..34ca0e739f 100644
 
+CREATE VIEW pg_stat_io AS
+SELECT
+       b.backend_type,
+       b.io_object,
+       b.io_context,
+       b.reads,
+       b.writes,
+       b.extends,
+       b.op_bytes,
+       b.evictions,
+       b.reuses,
+       b.fsyncs,
+       b.stats_reset
+FROM pg_stat_get_io() b;
+
 CREATE VIEW pg_stat_wal AS
     SELECT
         w.wal_records,
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 924698e6ae..9d707c3521 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1245,6 +1245,147 @@ pg_stat_get_buf_alloc(PG_FUNCTION_ARGS)
 	PG_RETURN_INT64(pgstat_fetch_stat_bgwriter()->buf_alloc);
 }
 
+/*
index 86eb8e8c58..2e804c5bd4 100644
 
+{ oid => '8459', descr => 'statistics: per backend type IO statistics',
+  proname => 'pg_stat_get_io', provolatile => 'v',
+  prorows => '30', proretset => 't',
+  proparallel => 'r', prorettype => 'record', proargtypes => '',
+  proallargtypes => '{text,text,text,int8,int8,int8,int8,int8,int8,int8,timestamptz}',
+  proargmodes => '{o,o,o,o,o,o,o,o,o,o,o}',
+  proargnames => '{backend_type,io_object,io_context,reads,writes,extends,op_bytes,evictions,reuses,fsyncs,stats_reset}',
+  prosrc => 'pg_stat_get_io' },
+
 { oid => '1136', descr => 'statistics: information about WAL activity',
   proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
   proparallel => 'r', prorettype => 'record', proargtypes => '',
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index e7a2f5856a..174b725fff 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1876,6 +1876,18 @@ pg_stat_gssapi| SELECT pid,
     gss_enc AS encrypted
    FROM pg_stat_get_activity(NULL::integer) s(datid, pid, usesysid, application_name, state, query, wait_event_type, wait_event, xact_start, query_start, backend_start, state_change, client_addr, client_hostname, client_port, backend_xid, backend_xmin, backend_type, ssl, sslversion, sslcipher, sslbits, ssl_client_dn, ssl_client_serial, ssl_issuer_dn, gss_auth, gss_princ, gss_enc, leader_pid, query_id)
   WHERE (client_port IS NOT NULL);
+pg_stat_io| SELECT backend_type,
+    io_object,
+    io_context,
+    reads,
+    writes,
+    extends,
+    op_bytes,
+    evictions,
+    reuses,
+    fsyncs,
+    stats_reset
+   FROM pg_stat_get_io() b(backend_type, io_object, io_context, reads, writes, extends, op_bytes, evictions, reuses, fsyncs, stats_reset);
 pg_stat_progress_analyze| SELECT s.pid,
     s.datid,
     d.datname,
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 1d84407a03..3ad38da0dd 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1126,4 +1126,231 @@ SELECT pg_stat_get_subscription_stats(NULL);
  
 (1 row)
 
+ALTER TABLE test_io_shared SET TABLESPACE regress_tblspace;
+ALTER TABLE test_io_local SET TABLESPACE regress_tblspace;
index b4d6753c71..5badd09a1c 100644
@@ -536,4 +536,145 @@ SELECT pg_stat_get_replication_slot(NULL);

+ALTER TABLE test_io_shared SET TABLESPACE regress_tblspace;
+ALTER TABLE test_io_local SET TABLESPACE regress_tblspace;
index 65be0dea1b..970a0cfd1d 100644
@@ -3376,6 +3376,7 @@ intset_internal_node
 intset_leaf_node
 intset_node
 intvKEY
+io_stat_col
 itemIdCompact
 itemIdCompactData
 iterator
-- 
2.34.1

From 2c138faa33ff93756b6bda70e68798c6ab4afbe6 Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 16:25:31 -0500
Subject: [PATCH v50 3/5] pgstat: Count IO for relations

 
 
 	if (isLocalBuf)
 	{

 	if (isExtend)
 	{

 

 

 

 

 
+	/*

 
 
 /*
 
+#include "pgstat.h"
 
-	/*
 
+	*from_ring = false;
+
 	/*
 
 	/*
 
 /*
 
 	/*

+/*

 
+	/*

 
 
+		/*

 
 
+#include "pgstat.h"
 
 /* freelist.c */

 typedef void *Block;

---

From: Melanie Plageman <[email-masked]>
Date: Thu, 19 Jan 2023 21:15:34 -0500

> Yes, it conflicted with 47bb9db75996232. rebased v50 is attached.

Oh dear-- an extra FlushBuffer() snuck in there somehow.
Removed it in attached v51.
Also, I fixed an issue in my tablespace.sql updates

- Melanie
From f477dfb566a47350bc78dcfb925db429e79fd657 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 16:34:27 -0500
Subject: [PATCH v51 5/5] pg_stat_io documentation

 
+     <row>
 
   <para>
 
  </sect2>
 
  </sect2>
 
-- 
2.34.1

From bc2a91fbe68180d47179388b8303badcfdd5259c Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Thu, 19 Jan 2023 15:45:54 -0500
Subject: [PATCH v51 1/5] Create regress_tblspc in test_setup

Modify the tablespace tests a bit to check for specific relations in the
test and not simply for the absence or presence of any objects in the
tablespace in case other tests leave objects around in the tablespace.
---
 src/test/regress/expected/tablespace.out | 65 +++++++++++++-----------
 src/test/regress/expected/test_setup.out |  3 ++
 src/test/regress/parallel_schedule       |  9 ++--
 src/test/regress/sql/tablespace.sql      | 42 ++++++++++-----
 src/test/regress/sql/test_setup.sql      |  4 ++
 5 files changed, 75 insertions(+), 48 deletions(-)

index c52cf1cfcf..007c00bfff 100644

 

 
 SELECT relfilenode = :main_filenode AS main_same FROM pg_class
@@ -331,7 +338,7 @@ CREATE TABLE testschema.part1 PARTITION OF testschema.part FOR VALUES IN (1);
 CREATE INDEX part_a_idx ON testschema.part (a) TABLESPACE regress_tblspace;
 CREATE TABLE testschema.part2 PARTITION OF testschema.part FOR VALUES IN (2);
 SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
-    where c.reltablespace = t.oid AND c.relname LIKE 'part%_idx';
+    where c.reltablespace = t.oid AND c.relname LIKE 'part%_idx' ORDER BY relname;
    relname   |     spcname      
 -------------+------------------
  part1_a_idx | regress_tblspace
diff --git a/src/test/regress/expected/test_setup.out b/src/test/regress/expected/test_setup.out
index 391b36d131..4f54fe20ec 100644
--- a/src/test/regress/expected/test_setup.out
+++ b/src/test/regress/expected/test_setup.out
@@ -18,6 +18,9 @@ SET synchronous_commit = on;
 -- and most of the core regression tests still expect that.
 --
 GRANT ALL ON SCHEMA public TO public;
+-- Create a tablespace we can use in tests.
+SET allow_in_place_tablespaces = true;
+CREATE TABLESPACE regress_tblspace LOCATION '';
 --
 -- These tables have traditionally been referenced by many tests,
 -- so create and populate them.  Insert only non-error values here.
diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule
index a930dfe48c..15e015b3d6 100644
--- a/src/test/regress/parallel_schedule
+++ b/src/test/regress/parallel_schedule
@@ -11,11 +11,6 @@
 # required setup steps
 test: test_setup

index 21db433f2a..58a279e2f9 100644

@@ -225,7 +239,7 @@ CREATE TABLE testschema.part1 PARTITION OF testschema.part FOR VALUES IN (1);
 CREATE INDEX part_a_idx ON testschema.part (a) TABLESPACE regress_tblspace;
 CREATE TABLE testschema.part2 PARTITION OF testschema.part FOR VALUES IN (2);
 SELECT relname, spcname FROM pg_catalog.pg_tablespace t, pg_catalog.pg_class c
-    where c.reltablespace = t.oid AND c.relname LIKE 'part%_idx';
+    where c.reltablespace = t.oid AND c.relname LIKE 'part%_idx' ORDER BY relname;
 \d testschema.part
 \d+ testschema.part
 \d testschema.part1

From 1921e23c4650f497266c0f73114248a57e58778e Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 16:10:34 -0500
Subject: [PATCH v51 2/5] pgstat: Infrastructure to track IO operations

 
+	[PGSTAT_KIND_IO] = {
+		.name = "io",
+
+		.fixed_amount = true,
+

 
+	/* flush IO stats */

+	/*
 
+	/*
+	 * Read IO stats struct
+	 */

 /*
 
 /*

 /*
 
 /*
 
 /*

 /*

 /*
 
 /*
 
 /*

 	PG_RETURN_VOID();
 }

 
+
+/*

+/*

 
+	PgStat_IO	io;
+
 
 	PgStat_WalStats wal;

+/*

+/*

From 9ec02a2201ffc465be17fe0e9ee49d4e42a9d89f Mon Sep 17 00:00:00 2001
From: Andres Freund <[email-masked]>
Date: Tue, 17 Jan 2023 16:25:31 -0500
Subject: [PATCH v51 3/5] pgstat: Count IO for relations

 src/backend/storage/buffer/bufmgr.c   | 110 ++++++++++++++++++++++----

index 8075828e8a..fffd846098 100644

 	if (isLocalBuf)
 	{

 	if (isExtend)
 	{

 

 

 

 

 
+	/*
 
+				pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_WRITE);
+
 				/* Pop the error context stack */
 				error_context_stack = errcallback.previous;
 			}
@@ -3586,7 +3664,7 @@ FlushRelationBuffers(Relation rel)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, RelationGetSmgr(rel));
+			FlushBuffer(bufHdr, RelationGetSmgr(rel), IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3684,7 +3762,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, srelent->srel);
+			FlushBuffer(bufHdr, srelent->srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3894,7 +3972,7 @@ FlushDatabaseBuffers(Oid dbid)
 		{
 			PinBuffer_Locked(bufHdr);
 			LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-			FlushBuffer(bufHdr, NULL);
+			FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 			LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
 			UnpinBuffer(bufHdr);
 		}
@@ -3921,7 +3999,7 @@ FlushOneBuffer(Buffer buffer)

 
 /*
 
+#include "pgstat.h"
 
-	/*
 
+	*from_ring = false;
+
 	/*
 
 	/*
 
 /*
 
 	/*

+/*

 
+	/*

 
 
+		/*

 
 
+#include "pgstat.h"
 
 /* freelist.c */

 typedef void *Block;

From cc70c22486f3d68c70e1059547a49179f007a51a Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Tue, 17 Jan 2023 16:28:27 -0500
Subject: [PATCH v51 4/5] Add system view tracking IO ops per backend type

 
 
+
 
+-- pg_stat_io test:

 
 -- cleanup
 DROP TABLE heaptest;

+/*

 (1 row)

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Tue, 24 Jan 2023 17:22:03 +0900 (JST)

Hello.

At Thu, 19 Jan 2023 21:15:34 -0500, Melanie Plageman <[email-masked]> wrote in 
> Also, I fixed an issue in my tablespace.sql updates

I only looked 0002 and 0004.
(Sorry for the random order of the comment..)

0002:

This is relatively complex checking. We already asserts-out increments
of invalid counters. Thus this is checking if some unrelated codes
clobbered them, which we do only when consistency is critical. Is
there any needs to do that here?  I saw another occurance of the same
assertion.

I'm not sure the addition is useful..

Is there any reason for not checking the value ranges at the
bottom-most functions?  They can lead to out-of-bounds access so I
don't think we need to continue execution for such invalid values.

I'm not sure I like to omit parentheses for such a long Boolean
expression on the right side.

The names of the functions hardly make sense alone to me. How about
write_struct()/read_struct()?  (I personally prefer to use
write_chunk() directly..)

+ PgStat_BktypeIO

This patch abbreviates "backend" as "bk" but "be" is used in many
places. I think that naming should follow the predecessors.

0004:

system_views.sql:

What does the "b" stand for? (Backend? then "s" or "i" seems
straight-forward.)

This is a bit hard to read since it requires to follow the condition
flow. The following is simpler and I thhink close to our standard.

if (pgstat_tacks_io_op())
   values[col_idx] =
			Int64GetDatum(bktype_stats->data[io_obj][io_context][io_op]);
else
   nulls[col_idx]  = true;

> +        Number of read operations in units of <varname>op_bytes</varname>.

I may be the only one who see the name as umbiguous between "total
number of handled bytes" and "bytes hadled at an operation". Can't it
be op_blocksize or just block_size?

+       b.io_object,
+       b.io_context,

It's uncertain to me why only the two columns are prefixed by
"io". Don't "object_type" and just "context" work instead?

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Tue, 24 Jan 2023 17:25:38 +0900 (JST)

At Tue, 24 Jan 2023 17:22:03 +0900 (JST), Kyotaro Horiguchi <[email-masked]> wrote in 
> bottom-most functions?  They can lead to out-of-bounds access so I

To make sure, the "They" means "out-of-range io_object/context/op
values"..

regards.

---

From: Andres Freund <[email-masked]>
Date: Tue, 24 Jan 2023 14:35:12 -0800

Hi,

> there any needs to do that here?  I saw another occurance of the same

I found it useful to find problems.

> expression on the right side.

What parens would help?

> write_chunk() directly..)

That's not related to this patch - there's several existing callers for
it. And write_struct wouldn't be better imo, because it's not just for
structs.

> places. I think that naming should follow the predecessors.

The precedence aren't consistent unfortunately :)

> be op_blocksize or just block_size?

No, block wouldn't be helpful - we'd like to use this for something that isn't
uniform blocks.

Greetings,

Andres Freund

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Wed, 25 Jan 2023 16:56:17 +0900 (JST)

At Tue, 24 Jan 2023 14:35:12 -0800, Andres Freund <[email-masked]> wrote in 
> I found it useful to find problems.

Okay.

> > expression on the right side.

I thought about the following.

no_temp_rel =
   (bktype == B_AUTOVAC_LAUNCHER ||
    bktype == B_BG_WRITER ||
	bktype == B_CHECKPOINTER ||
	bktype == B_AUTOVAC_WORKER ||
	bktype == B_STANDALONE_BACKEND ||
	bktype == B_STARTUP);

> it. And write_struct wouldn't be better imo, because it's not just for

Hmm.  Then what the "_s" stands for?

> The precedence aren't consistent unfortunately :)

Uuuummmmm.  Okay, just I like "be" there!  Anyway, I don't strongly
push that.

> No, block wouldn't be helpful - we'd like to use this for something that isn't

What does the field show in that case?  The mean of operation size? Or
one row per opration size?  If the former, the name looks somewhat
wrong. If the latter, block_size seems making sense.

regards.

---

From: Andres Freund <[email-masked]>
Date: Tue, 7 Feb 2023 22:38:14 -0800

Hi,

I did another read through the series. I do have some minor changes, but
they're minor. I think this is ready for commit. I plan to start pushing
tomorrow.

The changes I made are:
- the tablespace test changes didn't quite work in isolation / needed a bit of
  polishing
- moved the tablespace changes to later in the series
- split the tests out of the commit adding the view into its own commit
- minor code formatting things (e.g. didn't like nested for()s without {})

> Hmm.  Then what the "_s" stands for?

Size. It's a macro that just forwards to read_chunk()/write_chunk().

> wrong. If the latter, block_size seems making sense.

1, so that it's clear that the rest are in bytes.

Greetings,

Andres Freund

---

From: Andres Freund <[email-masked]>
Date: Wed, 8 Feb 2023 21:03:19 -0800

Hi,

> they're minor. I think this is ready for commit. I plan to start pushing

Pushed the first (and biggest) commit. More tomorrow.

Already can't wait to see incremental improvements of this version of
pg_stat_io ;). Tracking buffer hits. Tracking Wal IO. Tracking relation IO
bypassing shared buffers. Per connection IO statistics. Tracking IO time.

Greetings,

Andres Freund

---

From: Andres Freund <[email-masked]>
Date: Sat, 11 Feb 2023 10:24:37 -0800

Hi,

> Pushed the first (and biggest) commit. More tomorrow.

Just pushed the actual pg_stat_io view, the splitting of the tablespace test,
and the pg_stat_io tests.

Yay!

Thanks all for patch and review!

> bypassing shared buffers. Per connection IO statistics. Tracking IO time.

That's still the case.

Greetings,

Andres Freund

---

From: Andres Freund <[email-masked]>
Date: Tue, 14 Feb 2023 11:08:12 -0800

Hi,

> and the pg_stat_io tests.

One thing I started to wonder about since is whether we should remove the io_
prefix from io_object, io_context. The prefixes make sense on the C level, but
it's not clear to me that that's also the case on the table level.

Greetings,

Andres Freund

---

From: Justin Pryzby <[email-masked]>
Date: Tue, 21 Feb 2023 19:50:35 -0600

> and the pg_stat_io tests.

pg_stat_io says:

         * Some BackendTypes do not currently perform any IO in certain
         * IOContexts, and, while it may not be inherently incorrect for them to
         * do so, excluding those rows from the view makes the view easier to use.

        if (bktype == B_AUTOVAC_LAUNCHER && io_context == IOCONTEXT_VACUUM)
                return false;

        if ((bktype == B_AUTOVAC_WORKER || bktype == B_AUTOVAC_LAUNCHER) &&
                io_context == IOCONTEXT_BULKWRITE)
                return false;

What about these combinations?  Aren't these also "can't happen" ?

 relation      | bulkread   | autovacuum worker
 relation      | bulkread   | autovacuum launcher
 relation      | vacuum     | startup

-- 
Justin

---

From: Justin Pryzby <[email-masked]>
Date: Tue, 21 Feb 2023 22:09:37 -0600

>  relation      | vacuum     | startup

Nevermind - at least these are possible.

(gdb) p MyBackendType
$1 = B_AUTOVAC_WORKER
(gdb) p io_object
$2 = IOOBJECT_RELATION
(gdb) p io_context
$3 = IOCONTEXT_BULKREAD
(gdb) p io_op
$4 = IOOP_EVICT
(gdb) bt
...
#9  0x0000557b2f6097a3 in ReadBufferExtended (reln=0x7ff5ccee36b8, forkNum=forkNum@entry=MAIN_FORKNUM, blockNum=blockNum@entry=16, mode=mode@entry=RBM_NORMAL, strategy=0x557b305fb568) at ../src/include/utils/rel.h:573
#10 0x0000557b2f3057c0 in heapgetpage (sscan=sscan@entry=0x557b305fb158, block=block@entry=16) at ../src/backend/access/heap/heapam.c:405
#11 0x0000557b2f305d6c in heapgettup_pagemode (scan=scan@entry=0x557b305fb158, dir=dir@entry=ForwardScanDirection, nkeys=0, key=0x0) at ../src/backend/access/heap/heapam.c:885
#12 0x0000557b2f306956 in heap_getnext (sscan=sscan@entry=0x557b305fb158, direction=direction@entry=ForwardScanDirection) at ../src/backend/access/heap/heapam.c:1122
#13 0x0000557b2f59be0c in do_autovacuum () at ../src/backend/postmaster/autovacuum.c:2061
#14 0x0000557b2f59ccf7 in AutoVacWorkerMain (argc=argc@entry=0, argv=argv@entry=0x0) at ../src/backend/postmaster/autovacuum.c:1716
#15 0x0000557b2f59cdd8 in StartAutoVacWorker () at ../src/backend/postmaster/autovacuum.c:1494
#16 0x0000557b2f5a561a in StartAutovacuumWorker () at ../src/backend/postmaster/postmaster.c:5481
#17 0x0000557b2f5a5a39 in process_pm_pmsignal () at ../src/backend/postmaster/postmaster.c:5192
#18 0x0000557b2f5a5d7e in ServerLoop () at ../src/backend/postmaster/postmaster.c:1770
#19 0x0000557b2f5a73da in PostmasterMain (argc=9, argv=<optimized out>) at ../src/backend/postmaster/postmaster.c:1463
#20 0x0000557b2f4dfc39 in main (argc=9, argv=0x557b30568f50) at ../src/backend/main/main.c:200

-- 
Justin

---

From: Tom Lane <[email-masked]>
Date: Sun, 26 Feb 2023 13:20:00 -0500

> Pushed the first (and biggest) commit. More tomorrow.

I hadn't run my buildfarm-compile-warning scraper for a little while,
but I just did, and I find that this commit is causing warnings on
no fewer than 14 buildfarm animals.  They all look like

 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:40:14: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:43:16: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:70:19: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:71:20: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:115:14: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstat_io.c:118:16: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstatfuncs.c:1329:12: warning: comparison of constant 2 with expression of type 'IOObject' (aka 'enum IOObject') is always true [-Wtautological-constant-out-of-range-compare]
 ayu           | 2023-02-25 23:02:08 | pgstatfuncs.c:1334:17: warning: comparison of constant 4 with expression of type 'IOContext' (aka 'enum IOContext') is always true [-Wtautological-constant-out-of-range-compare]

That is, these compilers think that comparisons like

	io_object < IOOBJECT_NUM_TYPES
	io_context < IOCONTEXT_NUM_TYPES

are constant-true.  This seems not good; if they were to actually
act on this observation, by removing those loop-ending tests,
we'd have a problem.

The issue seems to be that code like this:

typedef enum IOContext
{
	IOCONTEXT_BULKREAD,
	IOCONTEXT_BULKWRITE,
	IOCONTEXT_NORMAL,
	IOCONTEXT_VACUUM,
} IOContext;

#define IOCONTEXT_FIRST IOCONTEXT_BULKREAD
#define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)

is far too cute for its own good.  I'm not sure about how to fix it
either.  I thought of defining

#define IOCONTEXT_LAST IOCONTEXT_VACUUM

and make the loop conditions like "io_context <= IOCONTEXT_LAST",
but that doesn't actually fix the problem.

(Even aside from that, I do not find this coding even a little bit
mistake-proof: you still have to remember to update the #define
when adding another enum value.)

We have similar code involving enum ForkNumber but it looks to me
like the loop variables are always declared as plain "int".  That
might be the path of least resistance here.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Sun, 26 Feb 2023 13:52:47 -0500

I wrote:
> is far too cute for its own good.

Oh, there's another thing here that qualifies as too-cute: loops like

    for (IOObject io_object = IOOBJECT_FIRST;
         io_object < IOOBJECT_NUM_TYPES; io_object++)

make it look like we could define these enums as 1-based rather
than 0-based, but if we did this code would fail, because it's
confusing "the number of values" with "1 more than the last value".

Again, we could fix that with tests like "io_context <= IOCONTEXT_LAST",
but I don't see the point of adding more macros rather than removing
some.  We do need IOOBJECT_NUM_TYPES to declare array sizes with,
so I think we should nuke the "xxx_FIRST" macros as being not worth
the electrons they're written on, and write these loops like

    for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)

which is not actually adding any assumptions that you don't already
make by using io_object as a C array subscript.

			regards, tom lane

---

From: Melanie Plageman <[email-masked]>
Date: Mon, 27 Feb 2023 09:24:01 -0500

> make by using io_object as a C array subscript.

Attached is a patch to remove the *_FIRST macros.
I was going to add in code to change

    for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
    to
    for (IOObject io_object = 0; (int) io_object < IOOBJECT_NUM_TYPES;
io_object++)

but then I couldn't remember why we didn't just do

I recall that when passing that loop variable into a function I was
getting a compiler warning that required me to cast the value back to an
enum to silence it:

            pgstat_tracks_io_op(bktype, (IOObject) io_object,
io_context, io_op))

However, I am now unable to reproduce that warning.
Moreover, I see in cases like table_block_relation_size() with
ForkNumber, the variable i is passed with no cast to smgrnblocks().

- Melanie
From cce6dc75e9e4fc9adc018a1d05874be5f3be96ae Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 27 Feb 2023 08:22:53 -0500
Subject: [PATCH v1 1/2] Remove potentially misleading *_FIRST macros

28e626bde00ef introduced IO statistic enums IOOp, IOObject, and
IOContext along with macros *_FIRST intended for use when looping
through the enumerated values of each. Per discussion in [1] these
macros are confusing and error-prone. Remove them.

[1] https://www.postgresql.org/message-id/23770.1677437567%40sss.pgh.pa.us
---
 src/backend/utils/activity/pgstat_io.c | 17 ++++++-----------
 src/backend/utils/adt/pgstatfuncs.c    | 10 ++++------
 src/include/pgstat.h                   |  3 ---
 3 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index 0e07e0848d..c478b126fa 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -36,18 +36,16 @@ pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
 {
 	bool		bktype_tracked = pgstat_tracks_io_bktype(bktype);
 
-	for (IOObject io_object = IOOBJECT_FIRST;
-		 io_object < IOOBJECT_NUM_TYPES; io_object++)
+	for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
 	{
-		for (IOContext io_context = IOCONTEXT_FIRST;
-			 io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		for (IOContext io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
 		{
 			/*
 			 * Don't bother trying to skip to the next loop iteration if
 			 * pgstat_tracks_io_object() would return false here. We still
 			 * need to validate that each counter is zero anyway.
 			 */
-			for (IOOp io_op = IOOP_FIRST; io_op < IOOP_NUM_TYPES; io_op++)
+			for (IOOp io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
 			{
 				/* No stats, so nothing to validate */
 				if (backend_io->data[io_object][io_context][io_op] == 0)
@@ -111,14 +109,11 @@ pgstat_flush_io(bool nowait)
 	else if (!LWLockConditionalAcquire(bktype_lock, LW_EXCLUSIVE))
 		return true;
 
-	for (IOObject io_object = IOOBJECT_FIRST;
-		 io_object < IOOBJECT_NUM_TYPES; io_object++)
+	for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
 	{
-		for (IOContext io_context = IOCONTEXT_FIRST;
-			 io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		for (IOContext io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
 		{
-			for (IOOp io_op = IOOP_FIRST;
-				 io_op < IOOP_NUM_TYPES; io_op++)
+			for (IOOp io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
 				bktype_shstats->data[io_object][io_context][io_op] +=
 					PendingIOStats.data[io_object][io_context][io_op];
 		}
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9d707c3521..12eda4ade0 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1306,7 +1306,7 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 
 	reset_time = TimestampTzGetDatum(backends_io_stats->stat_reset_timestamp);
 
-	for (BackendType bktype = B_INVALID; bktype < BACKEND_NUM_TYPES; bktype++)
+	for (BackendType bktype = 0; bktype < BACKEND_NUM_TYPES; bktype++)
 	{
 		Datum		bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
 		PgStat_BktypeIO *bktype_stats = &backends_io_stats->stats[bktype];
@@ -1325,13 +1325,11 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 		if (!pgstat_tracks_io_bktype(bktype))
 			continue;
 
-		for (IOObject io_obj = IOOBJECT_FIRST;
-			 io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+		for (IOObject io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
 		{
 			const char *obj_name = pgstat_get_io_object_name(io_obj);
 
-			for (IOContext io_context = IOCONTEXT_FIRST;
-				 io_context < IOCONTEXT_NUM_TYPES; io_context++)
+			for (IOContext io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
 			{
 				const char *context_name = pgstat_get_io_context_name(io_context);
 
@@ -1359,7 +1357,7 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 				 */
 				values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
 
-				for (IOOp io_op = IOOP_FIRST; io_op < IOOP_NUM_TYPES; io_op++)
+				for (IOOp io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
 				{
 					int			col_idx = pgstat_get_io_op_index(io_op);
 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index db9675884f..f43fac09ed 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -287,7 +287,6 @@ typedef enum IOObject
 	IOOBJECT_TEMP_RELATION,
 } IOObject;
 
-#define IOOBJECT_FIRST IOOBJECT_RELATION
 #define IOOBJECT_NUM_TYPES (IOOBJECT_TEMP_RELATION + 1)
 
 typedef enum IOContext
@@ -298,7 +297,6 @@ typedef enum IOContext
 	IOCONTEXT_VACUUM,
 } IOContext;
 
-#define IOCONTEXT_FIRST IOCONTEXT_BULKREAD
 #define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)
 
 typedef enum IOOp
@@ -311,7 +309,6 @@ typedef enum IOOp
 	IOOP_WRITE,
 } IOOp;
 
-#define IOOP_FIRST IOOP_EVICT
 #define IOOP_NUM_TYPES (IOOP_WRITE + 1)
 
 typedef struct PgStat_BktypeIO
-- 
2.37.2

---

From: Tom Lane <[email-masked]>
Date: Mon, 27 Feb 2023 10:30:42 -0500

>     for (IOObject io_object = 0; (int) io_object < IOOBJECT_NUM_TYPES; io_object++)

I don't really like that proposal.  ISTM it's just silencing the
messenger rather than addressing the underlying problem, namely that
there's no guarantee that an IOObject variable can hold the value
IOOBJECT_NUM_TYPES, which it had better do if you want the loop to
terminate.  Admittedly it's quite unlikely that these three enums would
grow to the point that that becomes an actual hazard for them --- but
IMO it's still bad practice and a bad precedent for future code.

> ForkNumber, the variable i is passed with no cast to smgrnblocks().

Yeah, my druthers would be to just do it the way we do comparable
things with ForkNumber.  I don't feel like we need to invent a
better way here.

The risk of needing to cast when using the "int" loop variable
as an enum is obviously the downside of that approach, but we have
not seen any indication that any compilers actually do warn.
It's interesting that you did see such a warning ... I wonder which
compiler you were using at the time?

			regards, tom lane

---

From: Melanie Plageman <[email-masked]>
Date: Mon, 27 Feb 2023 14:03:16 -0500

> IMO it's still bad practice and a bad precedent for future code.

That's fair. Patch attached.

> compiler you were using at the time?

so, pretty much any version of clang I tried with
-Wsign-conversion produces a warning.

<source>:35:32: warning: implicit conversion changes signedness: 'int'
to 'IOOp' (aka 'enum IOOp') [-Wsign-conversion]

I didn't do the casts in the attached patch since they aren't done elsewhere.

- Melanie
From 34fee4a3d1d1353aa38a95b3afc2d302a5f586ff Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email-masked]>
Date: Mon, 27 Feb 2023 08:48:11 -0500
Subject: [PATCH v1 2/2] Change IO stats enum loop variables to ints

Per [1], using an enum as the loop variable with a macro-defined
termination condition of #_enum_values + 1 is not guaranteed to be safe
- as compilers are free to make enums as small as a char.

Some (older) compilers will notice this when building with
-Wtautological-constant-out-of-range-compare.

[1] https://www.postgresql.org/message-id/354645.1677511842%40sss.pgh.pa.us
---
 src/backend/utils/activity/pgstat_io.c | 12 ++++++------
 src/backend/utils/adt/pgstatfuncs.c    |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index c478b126fa..c4199d18c8 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -36,16 +36,16 @@ pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
 {
 	bool		bktype_tracked = pgstat_tracks_io_bktype(bktype);
 
-	for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
+	for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
 	{
-		for (IOContext io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
 		{
 			/*
 			 * Don't bother trying to skip to the next loop iteration if
 			 * pgstat_tracks_io_object() would return false here. We still
 			 * need to validate that each counter is zero anyway.
 			 */
-			for (IOOp io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+			for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
 			{
 				/* No stats, so nothing to validate */
 				if (backend_io->data[io_object][io_context][io_op] == 0)
@@ -109,11 +109,11 @@ pgstat_flush_io(bool nowait)
 	else if (!LWLockConditionalAcquire(bktype_lock, LW_EXCLUSIVE))
 		return true;
 
-	for (IOObject io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
+	for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
 	{
-		for (IOContext io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+		for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
 		{
-			for (IOOp io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+			for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
 				bktype_shstats->data[io_object][io_context][io_op] +=
 					PendingIOStats.data[io_object][io_context][io_op];
 		}
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 12eda4ade0..b61a12382b 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1306,7 +1306,7 @@ pg_stat_get_io(PG_FUNCTION_ARGS)

-	for (BackendType bktype = 0; bktype < BACKEND_NUM_TYPES; bktype++)
+	for (int bktype = 0; bktype < BACKEND_NUM_TYPES; bktype++)
 	{
 		Datum		bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
 		PgStat_BktypeIO *bktype_stats = &backends_io_stats->stats[bktype];
@@ -1325,11 +1325,11 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 		if (!pgstat_tracks_io_bktype(bktype))
 			continue;
 
-		for (IOObject io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+		for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
 		{
 			const char *obj_name = pgstat_get_io_object_name(io_obj);
 
-			for (IOContext io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+			for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
 			{
 				const char *context_name = pgstat_get_io_context_name(io_context);
 
@@ -1357,7 +1357,7 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
 				 */
 				values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
 
-				for (IOOp io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+				for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
 				{
 					int			col_idx = pgstat_get_io_op_index(io_op);
 
-- 
2.37.2

---

From: Tom Lane <[email-masked]>
Date: Mon, 27 Feb 2023 14:58:30 -0500

> to 'IOOp' (aka 'enum IOOp') [-Wsign-conversion]

Oh, interesting --- so it's not about the implicit conversion to enum
but just about signedness.  I bet we could silence that by making the
loop variables be "unsigned int".  I doubt it's worth any extra keystrokes
though, because we are not at all clean about sign-conversion warnings.
I tried enabling -Wsign-conversion on Apple's clang 14.0.0 just now,
and counted 13462 such warnings just in the core build :-(.  I don't
foresee anybody trying to clean that up.

> I didn't do the casts in the attached patch since they aren't done elsewhere.

Agreed.  I'll push this along with the earlier patch if there are
not objections.

			regards, tom lane

---

From: Andres Freund <[email-masked]>
Date: Mon, 27 Feb 2023 15:18:30 -0800

> Agreed.  I'll push this along with the earlier patch if there are

None here.

---

From: Tom Lane <[email-masked]>
Date: Sat, 04 Mar 2023 18:21:09 -0500

> and the pg_stat_io tests.

One of the test cases is flapping a bit:

diff -U3 /home/pg/build-farm-15/buildroot/HEAD/pgsql.build/src/test/regress/expected/stats.out /home/pg/build-farm-15/buildroot/HEAD/pgsql.build/src/test/regress/results/stats.out
--- /home/pg/build-farm-15/buildroot/HEAD/pgsql.build/src/test/regress/expected/stats.out	2023-03-04 21:30:05.891579466 +0100
+++ /home/pg/build-farm-15/buildroot/HEAD/pgsql.build/src/test/regress/results/stats.out	2023-03-04 21:34:26.745552661 +0100
@@ -1201,7 +1201,7 @@
 SELECT :io_sum_shared_after_reads > :io_sum_shared_before_reads;
  ?column? 
 ----------
- t
+ f
 (1 row)
 
 DROP TABLE test_io_shared;

There are two instances of this today [1][2], and I've seen it before
but failed to note down where.

			regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=grison&dt=2023-03-04%2021%3A19%3A39
[2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=mule&dt=2023-03-04%2020%3A30%3A05

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Mon, 06 Mar 2023 15:24:25 +0900 (JST)

At Sat, 04 Mar 2023 18:21:09 -0500, Tom Lane <[email-masked]> wrote in 
> +++ /home/pg/build-farm-15/buildroot/HEAD/pgsql.build/src/test/regress/results/stats.out	2023-03-04 21:34:26.745552661 +0100

The concurrent autoanalyze below is logged as performing at least one
page read from the table. It is unclear, however, how that analyze
operation resulted in 19 hits and 2 reads on the (I think) single-page
relation.

In any case, I think we need to avoid such concurrent autovacuum/analyze.

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=grison&dt=2023-03-04%2021%3A19%3A39

2023-03-04 22:36:27.781 CET [4073:106] pg_regress/stats LOG:  statement: ALTER TABLE test_io_shared SET TABLESPACE regress_tblspace;
2023-03-04 22:36:27.838 CET [4073:107] pg_regress/stats LOG:  statement: SELECT COUNT(*) FROM test_io_shared;
2023-03-04 22:36:27.864 CET [4255:5] LOG:  automatic analyze of table "regression.public.test_io_shared"
	avg read rate: 5.208 MB/s, avg write rate: 5.208 MB/s
	buffer usage: 17 hits, 2 misses, 2 dirtied
2023-03-04 22:36:28.024 CET [4073:108] pg_regress/stats LOG:  statement: SELECT pg_stat_force_next_flush();
2023-03-04 22:36:28.024 CET [4073:108] pg_regress/stats LOG:  statement: SELECT pg_stat_force_next_flush();
2023-03-04 22:36:28.027 CET [4073:109] pg_regress/stats LOG:  statement: SELECT sum(reads) AS io_sum_shared_after_reads
	  FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation'  

regards.

---

From: Kyotaro Horiguchi <[email-masked]>
Date: Mon, 06 Mar 2023 15:48:43 +0900 (JST)

At Mon, 06 Mar 2023 15:24:25 +0900 (JST), Kyotaro Horiguchi <[email-masked]> wrote in 
> In any case, I think we need to avoid such concurrent autovacuum/analyze.

If it is correct, I believe the attached fix works.

regads.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 937b2101b3..023ec5ecc4 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1137,7 +1137,7 @@ SELECT pg_stat_get_subscription_stats(NULL);
 -- extends.
 SELECT sum(extends) AS io_sum_shared_before_extends
   FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
-CREATE TABLE test_io_shared(a int);
+CREATE TABLE test_io_shared(a int) WITH (autovacuum_enabled = 'false');
 INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
 SELECT pg_stat_force_next_flush();
  pg_stat_force_next_flush 
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 74e592aa8a..aa6552befd 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -549,7 +549,7 @@ SELECT pg_stat_get_subscription_stats(NULL);
 -- extends.
 SELECT sum(extends) AS io_sum_shared_before_extends
   FROM pg_stat_io WHERE io_context = 'normal' AND io_object = 'relation' \gset
-CREATE TABLE test_io_shared(a int);
+CREATE TABLE test_io_shared(a int) WITH (autovacuum_enabled = 'false');
 INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
 SELECT pg_stat_force_next_flush();
 SELECT sum(extends) AS io_sum_shared_after_extends

---

From: Melanie Plageman <[email-masked]>
Date: Mon, 6 Mar 2023 10:09:24 -0500

> If it is correct, I believe the attached fix works.

Thanks for investigating this!

Yes, this fix looks correct and makes sense to me.

> operation resulted in 19 hits and 2 reads on the (I think) single-page

Yes, it is a single page.
I think there could be a few different reasons by it is 2 misses/2
dirtied, but the one that seems most likely is that I/O of other
relations done during this autovac/analyze of this relation is counted
in the same global variables (like catalog tables).

- Melanie