public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v46 5/7] Doc part of shared-memory based stats collector. 3+ messages / 3 participants [nested] [flat]
* [PATCH v46 5/7] Doc part of shared-memory based stats collector. @ 2020-03-19 06:11 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Kyotaro Horiguchi @ 2020-03-19 06:11 UTC (permalink / raw) --- doc/src/sgml/catalogs.sgml | 6 +- doc/src/sgml/config.sgml | 34 ++++---- doc/src/sgml/high-availability.sgml | 13 +-- doc/src/sgml/monitoring.sgml | 127 +++++++++++++--------------- doc/src/sgml/ref/pg_dump.sgml | 9 +- 5 files changed, 90 insertions(+), 99 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 3a2266526c..4d8b92df72 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -9234,9 +9234,9 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l <para> <xref linkend="view-table"/> lists the system views described here. More detailed documentation of each view follows below. - There are some additional views that provide access to the results of - the statistics collector; they are described in <xref - linkend="monitoring-stats-views-table"/>. + There are some additional views that provide access to the activity + statistics; they are described in + <xref linkend="monitoring-stats-views-table"/>. </para> <para> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 82864bbb24..b0c25c9c5c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7327,11 +7327,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; <title>Run-time Statistics</title> <sect2 id="runtime-config-statistics-collector"> - <title>Query and Index Statistics Collector</title> + <title>Query and Index Activity Statistics</title> <para> - These parameters control server-wide statistics collection features. - When statistics collection is enabled, the data that is produced can be + These parameters control server-wide activity statistics features. + When activity statistics is enabled, the data that is produced can be accessed via the <structname>pg_stat</structname> and <structname>pg_statio</structname> family of system views. Refer to <xref linkend="monitoring"/> for more information. @@ -7347,14 +7347,13 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </term> <listitem> <para> - Enables the collection of information on the currently - executing command of each session, along with the time when - that command began execution. This parameter is on by - default. Note that even when enabled, this information is not - visible to all users, only to superusers and the user owning - the session being reported on, so it should not represent a - security risk. - Only superusers can change this setting. + Enables activity tracking on the currently executing command of + each session, along with the time when that command began + execution. This parameter is on by default. Note that even when + enabled, this information is not visible to all users, only to + superusers and the user owning the session being reported on, so it + should not represent a security risk. Only superusers can change this + setting. </para> </listitem> </varlistentry> @@ -7385,9 +7384,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </term> <listitem> <para> - Enables collection of statistics on database activity. + Enables tracking of database activity. This parameter is on by default, because the autovacuum - daemon needs the collected information. + daemon needs the activity information. Only superusers can change this setting. </para> </listitem> @@ -8485,7 +8484,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; <listitem> <para> Specifies the fraction of the total number of heap tuples counted in - the previous statistics collection that can be inserted without + the previously collected statistics that can be inserted without incurring an index scan at the <command>VACUUM</command> cleanup stage. This setting currently applies to B-tree indexes only. </para> @@ -8497,9 +8496,10 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; the index contains deleted pages that can be recycled during cleanup. Index statistics are considered to be stale if the number of newly inserted tuples exceeds the <varname>vacuum_cleanup_index_scale_factor</varname> - fraction of the total number of heap tuples detected by the previous - statistics collection. The total number of heap tuples is stored in - the index meta-page. Note that the meta-page does not include this data + + fraction of the total number of heap tuples in the previously + collected statistics. The total number of heap tuples is stored in the + index meta-page. Note that the meta-page does not include this data until <command>VACUUM</command> finds no dead tuples, so B-tree index scan at the cleanup stage can only be skipped if the second and subsequent <command>VACUUM</command> cycles detect no dead tuples. diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index efc382cb8d..6c620469eb 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -2338,12 +2338,13 @@ LOG: database system is ready to accept read only connections </para> <para> - The statistics collector is active during recovery. All scans, reads, blocks, - index usage, etc., will be recorded normally on the standby. Replayed - actions will not duplicate their effects on primary, so replaying an - insert will not increment the Inserts column of pg_stat_user_tables. - The stats file is deleted at the start of recovery, so stats from primary - and standby will differ; this is considered a feature, not a bug. + The activity statistics is collected during recovery. All scans, reads, + blocks, index usage, etc., will be recorded normally on the + standby. Replayed actions will not duplicate their effects on primary, so + replaying an insert will not increment the Inserts column of + pg_stat_user_tables. The activity statistics is reset at the start of + recovery, so stats from primary and standby will differ; this is + considered a feature, not a bug. </para> <para> diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 3cdb1aff3c..afa8c35127 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -22,7 +22,7 @@ <para> Several tools are available for monitoring database activity and analyzing performance. Most of this chapter is devoted to describing - <productname>PostgreSQL</productname>'s statistics collector, + <productname>PostgreSQL</productname>'s activity statistics, but one should not neglect regular Unix monitoring programs such as <command>ps</command>, <command>top</command>, <command>iostat</command>, and <command>vmstat</command>. Also, once one has identified a @@ -53,7 +53,6 @@ postgres 15554 0.0 0.0 57536 1184 ? Ss 18:02 0:00 postgres: back postgres 15555 0.0 0.0 57536 916 ? Ss 18:02 0:00 postgres: checkpointer postgres 15556 0.0 0.0 57536 916 ? Ss 18:02 0:00 postgres: walwriter postgres 15557 0.0 0.0 58504 2244 ? Ss 18:02 0:00 postgres: autovacuum launcher -postgres 15558 0.0 0.0 17512 1068 ? Ss 18:02 0:00 postgres: stats collector postgres 15582 0.0 0.0 58772 3080 ? Ss 18:04 0:00 postgres: joe runbug 127.0.0.1 idle postgres 15606 0.0 0.0 58772 3052 ? Ss 18:07 0:00 postgres: tgl regression [local] SELECT waiting postgres 15610 0.0 0.0 58772 3056 ? Ss 18:07 0:00 postgres: tgl regression [local] idle in transaction @@ -65,9 +64,8 @@ postgres 15610 0.0 0.0 58772 3056 ? Ss 18:07 0:00 postgres: tgl primary server process. The command arguments shown for it are the same ones used when it was launched. The next five processes are background worker processes automatically launched by the - primary process. (The <quote>stats collector</quote> process will not be present - if you have set the system not to start the statistics collector; likewise - the <quote>autovacuum launcher</quote> process can be disabled.) + primary process. (The <quote>autovacuum launcher</quote> process will not + be present if you have set the system not to start it.) Each of the remaining processes is a server process handling one client connection. Each such process sets its command line display in the form @@ -130,20 +128,21 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser </sect1> <sect1 id="monitoring-stats"> - <title>The Statistics Collector</title> + <title>The Activity Statistics</title> <indexterm zone="monitoring-stats"> <primary>statistics</primary> </indexterm> <para> - <productname>PostgreSQL</productname>'s <firstterm>statistics collector</firstterm> - is a subsystem that supports collection and reporting of information about - server activity. Presently, the collector can count accesses to tables - and indexes in both disk-block and individual-row terms. It also tracks - the total number of rows in each table, and information about vacuum and - analyze actions for each table. It can also count calls to user-defined - functions and the total time spent in each one. + <productname>PostgreSQL</productname>'s <firstterm>activity + statistics</firstterm> is a subsystem that supports tracking and reporting + of information about server activity. Presently, the activity statistics + tracks the count of accesses to tables and indexes in both disk-block and + individual-row terms. It also tracks the total number of rows in each + table, and information about vacuum and analyze actions for each table. It + can also track calls to user-defined functions and the total time spent in + each one. </para> <para> @@ -151,15 +150,15 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser information about exactly what is going on in the system right now, such as the exact command currently being executed by other server processes, and which other connections exist in the system. This facility is independent - of the collector process. + of the activity statistics. </para> <sect2 id="monitoring-stats-setup"> - <title>Statistics Collection Configuration</title> + <title>Activity Statistics Configuration</title> <para> - Since collection of statistics adds some overhead to query execution, - the system can be configured to collect or not collect information. + Since tracking for the activity statistics adds some overhead to query + execution, the system can be configured to track or not track activity. This is controlled by configuration parameters that are normally set in <filename>postgresql.conf</filename>. (See <xref linkend="runtime-config"/> for details about setting configuration parameters.) @@ -172,7 +171,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser <para> The parameter <xref linkend="guc-track-counts"/> controls whether - statistics are collected about table and index accesses. + to track activity about table and index accesses. </para> <para> @@ -196,18 +195,11 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser </para> <para> - The statistics collector transmits the collected information to other - <productname>PostgreSQL</productname> processes through temporary files. - These files are stored in the directory named by the - <xref linkend="guc-stats-temp-directory"/> parameter, - <filename>pg_stat_tmp</filename> by default. - For better performance, <varname>stats_temp_directory</varname> can be - pointed at a RAM-based file system, decreasing physical I/O requirements. - When the server shuts down cleanly, a permanent copy of the statistics - data is stored in the <filename>pg_stat</filename> subdirectory, so that - statistics can be retained across server restarts. When recovery is - performed at server start (e.g., after immediate shutdown, server crash, - and point-in-time recovery), all statistics counters are reset. + down cleanly, a permanent copy of the statistics data is stored in + the <filename>pg_stat</filename> subdirectory, so that statistics can be + retained across server restarts. When recovery is performed at server + start (e.g. after immediate shutdown, server crash, and point-in-time + recovery), all statistics counters are reset. </para> </sect2> @@ -220,48 +212,46 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser linkend="monitoring-stats-dynamic-views-table"/>, are available to show the current state of the system. There are also several other views, listed in <xref - linkend="monitoring-stats-views-table"/>, available to show the results - of statistics collection. Alternatively, one can - build custom views using the underlying statistics functions, as discussed - in <xref linkend="monitoring-stats-functions"/>. + linkend="monitoring-stats-views-table"/>, available to show the activity + statistics. Alternatively, one can build custom views using the underlying + statistics functions, as discussed in + <xref linkend="monitoring-stats-functions"/>. </para> <para> - When using the statistics to monitor collected data, it is important - to realize that the information does not update instantaneously. - Each individual server process transmits new statistical counts to - the collector just before going idle; so a query or transaction still in - progress does not affect the displayed totals. Also, the collector itself - emits a new report at most once per <varname>PGSTAT_STAT_INTERVAL</varname> - milliseconds (500 ms unless altered while building the server). So the - displayed information lags behind actual activity. However, current-query - information collected by <varname>track_activities</varname> is - always up-to-date. + When using the activity statistics, it is important to realize that the + information does not update instantaneously. Each individual server writes + out new statistical counts just before going idle, not frequent than once + per <varname>PGSTAT_STAT_INTERVAL</varname> milliseconds (1 second unless + altered while building the server); so a query or transaction still in + progress does not affect the displayed totals. However, current-query + information tracked by <varname>track_activities</varname> is always + up-to-date. </para> <para> Another important point is that when a server process is asked to display - any of these statistics, it first fetches the most recent report emitted by - the collector process and then continues to use this snapshot for all - statistical views and functions until the end of its current transaction. - So the statistics will show static information as long as you continue the - current transaction. Similarly, information about the current queries of - all sessions is collected when any such information is first requested - within a transaction, and the same information will be displayed throughout - the transaction. - This is a feature, not a bug, because it allows you to perform several - queries on the statistics and correlate the results without worrying that - the numbers are changing underneath you. But if you want to see new - results with each query, be sure to do the queries outside any transaction - block. Alternatively, you can invoke + any of these statistics, it first reads the current statistics and then + continues to use this snapshot for all statistical views and functions + until the end of its current transaction. So the statistics will show + static information as long as you continue the current transaction. + Similarly, information about the current queries of all sessions is tracked + when any such information is first requested within a transaction, and the + same information will be displayed throughout the transaction. This is a + feature, not a bug, because it allows you to perform several queries on the + statistics and correlate the results without worrying that the numbers are + changing underneath you. But if you want to see new results with each + query, be sure to do the queries outside any transaction block. + Alternatively, you can invoke <function>pg_stat_clear_snapshot</function>(), which will discard the current transaction's statistics snapshot (if any). The next use of statistical information will cause a new snapshot to be fetched. </para> - + <para> - A transaction can also see its own statistics (as yet untransmitted to the - collector) in the views <structname>pg_stat_xact_all_tables</structname>, + A transaction can also see its own statistics (as yet unwritten to the + server-wide activity statistics) in the + views <structname>pg_stat_xact_all_tables</structname>, <structname>pg_stat_xact_sys_tables</structname>, <structname>pg_stat_xact_user_tables</structname>, and <structname>pg_stat_xact_user_functions</structname>. These numbers do not act as @@ -643,7 +633,7 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser 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 collector + advised to use the <productname>PostgreSQL</productname> activity statistics in combination with operating system utilities that allow insight into the kernel's handling of I/O. </para> @@ -1080,10 +1070,6 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser <entry><literal>LogicalLauncherMain</literal></entry> <entry>Waiting in main loop of logical replication launcher process.</entry> </row> - <row> - <entry><literal>PgStatMain</literal></entry> - <entry>Waiting in main loop of statistics collector process.</entry> - </row> <row> <entry><literal>RecoveryWalStream</literal></entry> <entry>Waiting in main loop of startup process for WAL to arrive, during @@ -1838,6 +1824,10 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser </thead> <tbody> + <row> + <entry><literal>ActivityStatistics</literal></entry> + <entry>Waiting to write out activity statistics to shared memory.</entry> + </row> <row> <entry><literal>AddinShmemInit</literal></entry> <entry>Waiting to manage an extension's space allocation in shared @@ -5996,9 +5986,10 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid, <entry><literal>performing final cleanup</literal></entry> <entry> <command>VACUUM</command> is performing final cleanup. During this phase, - <command>VACUUM</command> will vacuum the free space map, update statistics - in <literal>pg_class</literal>, and report statistics to the statistics - collector. When this phase is completed, <command>VACUUM</command> will end. + <command>VACUUM</command> will vacuum the free space map, update + statistics in <literal>pg_class</literal>, and system-wide activity + statistics. When this phase is completed, <command>VACUUM</command> + will end. </entry> </row> </tbody> diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml index bcbb7a25fb..1fa59a2fdf 100644 --- a/doc/src/sgml/ref/pg_dump.sgml +++ b/doc/src/sgml/ref/pg_dump.sgml @@ -1280,11 +1280,10 @@ PostgreSQL documentation </para> <para> - The database activity of <application>pg_dump</application> is - normally collected by the statistics collector. If this is - undesirable, you can set parameter <varname>track_counts</varname> - to false via <envar>PGOPTIONS</envar> or the <literal>ALTER - USER</literal> command. + The database activity of <application>pg_dump</application> is normally + collected. If this is undesirable, you can set + parameter <varname>track_counts</varname> to false + via <envar>PGOPTIONS</envar> or the <literal>ALTER USER</literal> command. </para> </refsect1> -- 2.27.0 ----Next_Part(Thu_Jan_14_15_14_25_2021_903)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v46-0006-Remove-the-GUC-stats_temp_directory.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing @ 2025-02-18 10:43 Bertrand Drouvot <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Bertrand Drouvot @ 2025-02-18 10:43 UTC (permalink / raw) To: [email protected] Hi hackers, a051e71e28a added the WAL writes and fsyncs information (and their related timing) into the pg_stat_io view so that one can now find the same information in 2 places: pg_stat_wal and pg_stat_io (for the wal object). This lead to a discussion in [1] to remove this information from the pg_stat_wal view. === Let's sump up in this dedicated thread what has been discussed in [1] 1. The pg_stat_io's writes and fsyncs (and their relative timing) are incremented at the exact same places as their pg_stat_wal counterparts. 2. pgstat_tracks_io_bktype() returns false for backend's that do not generate WAL (so that we don't lost WAL information in pg_stat_io). 3. pg_stat_stat.wal_write is the same value as "select sum(writes) from pg_stat_io where object = 'wal' and context = 'normal'" as these are incremented in XLogWrite(). 4. Same argument about pg_stat_wal.wal_write_time with pg_stat_io.write_time. 5. issue_xlog_fsync() tells that pg_stat_wal.wal_sync_time and sum(pg_stat_io.fsync_time) under object=wal and context=normal are the same values. 6. Same argument with the fsync counters pg_stat_wal.wal_sync and pg_stat_io.fsyncs. 7. This will encourage monitoring pull to move to pg_stat_io, where there is much more context and granularity of the stats data. 8. That will simplify the work done for the per backend WAL statistics ([1]) === Remarks R1. The "bytes" fields differ (as pg_stat_wal.wal_bytes somehow "focus" on the wal records size while the pg_stat_io's unit is the wal_block_size) so we keep them in both places. R2. track_wal_io_timing becomes useless once those fields are gone from pg_stat_wal R3. PendingWalStats becomes empty, so removes it. === Patch structure PFA 3 sub patches: - 0001: Add details in the pg_stat_io doc about the wal object. That's basically more or less a copy/paste from the pg_stat_wal fields description that will be removed in 0002. As we already track them in pg_stat_io, it's done in a dedicated patch. - 0002: Remove wal_[sync|write][_time] from pg_stat_wal, PendingWalStats and track_wal_io_timing. That does not make sense to split this work in sub-patches so that all of this in done in 0002. - 0003: remove the pgstat_prepare_io_time() parameter. Now that track_wal_io_timing is gone there is no need for pgstat_prepare_io_time() to get a parameter. [1]: https://www.postgresql.org/message-id/Z7NSc5j6M4g2r1HD%40ip-10-97-1-34.eu-west-3.compute.internal Looking forward to your feedback, Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] v1-0001-Add-details-in-the-pg_stat_io-doc-about-the-wal-o.patch (2.9K, ../../Z7RkQ0EfYaqqjgz%[email protected]/2-v1-0001-Add-details-in-the-pg_stat_io-doc-about-the-wal-o.patch) download | inline diff: From bc9dec707e2b12a6c6e7d1f654fbe41c8e5bbb65 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Tue, 18 Feb 2025 07:14:05 +0000 Subject: [PATCH v1 1/3] Add details in the pg_stat_io doc about the wal object Adding details about writes, fsyncs, write_time and sync_time when linked to the wal object. Same level of details as for wal_write, wal_sync, wal_write_time and wal_sync_time in pg_stat_wal. --- doc/src/sgml/monitoring.sgml | 42 ++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) 100.0% doc/src/sgml/ diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 928a6eb64b0..2c61c0d1e50 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -2930,6 +2930,48 @@ description | Waiting for a newly initialized WAL file to reach durable storage writer</literal>. </para> + <para> + For the <literal>wal</literal> <structfield>object</structfield>: + <itemizedlist> + <listitem> + <para> + <structfield>writes</structfield> is the number of times WAL buffers were + written out to disk via <function>XLogWrite</function> request. + See <xref linkend="wal-configuration"/> for more information about the internal + WAL function <function>XLogWrite</function>. + </para> + </listitem> + <listitem> + <para> + <structfield>fsyncs</structfield> is the number of times WAL files were + synced to disk via <function>issue_xlog_fsync</function> request (if <xref linkend="guc-fsync"/> + is <literal>on</literal> and <xref linkend="guc-wal-sync-method"/> is either + <literal>fdatasync</literal>, <literal>fsync</literal> or <literal>fsync_writethrough</literal>, + otherwise zero). See <xref linkend="wal-configuration"/> for more information + about the internal WAL function <function>issue_xlog_fsync</function>. + </para> + </listitem> + <listitem> + <para> + <structfield>write_time</structfield> is the total amount of time spent writing + WAL buffers to disk via <function>XLogWrite</function> request. + This includes the sync time when <varname>wal_sync_method</varname> + is either <literal>open_datasync</literal> or <literal>open_sync</literal>. + </para> + </listitem> + <listitem> + <para> + <structfield>sync_time</structfield> is the total amount of time spent syncing + WAL files to disk via <function>issue_xlog_fsync</function> request (if + <varname>fsync</varname> is <literal>on</literal>, + and <varname>wal_sync_method</varname> is either <literal>fdatasync</literal>, + <literal>fsync</literal> or <literal>fsync_writethrough</literal>, + otherwise zero). + </para> + </listitem> + </itemizedlist> + </para> + <para> <structname>pg_stat_io</structname> can be used to inform database tuning. For example: -- 2.34.1 [text/x-diff] v1-0002-Remove-wal_-sync-write-_time-from-pg_stat_wal-and.patch (20.1K, ../../Z7RkQ0EfYaqqjgz%[email protected]/3-v1-0002-Remove-wal_-sync-write-_time-from-pg_stat_wal-and.patch) download | inline diff: From 42b2d46d80f082829c91f9c91f844ab750f2cafd Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Tue, 18 Feb 2025 07:58:24 +0000 Subject: [PATCH v1 2/3] Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing a051e71e28a added this information into pg_stat_io (with more details and granularity), so there is no need to keep it in pg_stat_wal. This also allows to remove PendingWalStats and simplifies up coming commits related to per backend WAL statistics. Once done, track_wal_io_timing becomes useless so it is also removed. --- doc/src/sgml/config.sgml | 22 ------- doc/src/sgml/monitoring.sgml | 62 ------------------- doc/src/sgml/wal.sgml | 12 ++-- src/backend/access/transam/xlog.c | 38 ++---------- src/backend/catalog/system_views.sql | 4 -- src/backend/utils/activity/pgstat_wal.c | 24 +------ src/backend/utils/adt/pgstatfuncs.c | 20 +----- src/backend/utils/misc/guc_tables.c | 9 --- src/backend/utils/misc/postgresql.conf.sample | 1 - src/include/access/xlog.h | 1 - src/include/catalog/pg_proc.dat | 6 +- src/include/pgstat.h | 28 --------- src/test/regress/expected/rules.out | 6 +- src/tools/pgindent/typedefs.list | 1 - 14 files changed, 19 insertions(+), 215 deletions(-) 48.6% doc/src/sgml/ 12.7% src/backend/access/transam/ 11.1% src/backend/utils/activity/ 10.6% src/backend/utils/adt/ 3.4% src/backend/ 3.2% src/include/catalog/ 7.9% src/include/ diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 336630ce417..05760241174 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8327,28 +8327,6 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; </listitem> </varlistentry> - <varlistentry id="guc-track-wal-io-timing" xreflabel="track_wal_io_timing"> - <term><varname>track_wal_io_timing</varname> (<type>boolean</type>) - <indexterm> - <primary><varname>track_wal_io_timing</varname> configuration parameter</primary> - </indexterm> - </term> - <listitem> - <para> - Enables timing of WAL I/O calls. This parameter is off by default, - as it will repeatedly query the operating system for the current time, - which may cause significant overhead on some platforms. - You can use the <application>pg_test_timing</application> tool to - measure the overhead of timing on your system. - I/O timing information is - displayed in <link linkend="monitoring-pg-stat-wal-view"> - <structname>pg_stat_wal</structname></link>. - Only superusers and users with the appropriate <literal>SET</literal> - privilege can change this setting. - </para> - </listitem> - </varlistentry> - <varlistentry id="guc-track-functions" xreflabel="track_functions"> <term><varname>track_functions</varname> (<type>enum</type>) <indexterm> diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 2c61c0d1e50..8cbc89aa797 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -183,11 +183,6 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser of block read, write, extend, and fsync times. </para> - <para> - The parameter <xref linkend="guc-track-wal-io-timing"/> enables monitoring - of WAL write and fsync times. - </para> - <para> Normally these parameters are set in <filename>postgresql.conf</filename> so that they apply to all server processes, but it is possible to turn @@ -3297,63 +3292,6 @@ description | Waiting for a newly initialized WAL file to reach durable storage </para></entry> </row> - <row> - <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>wal_write</structfield> <type>bigint</type> - </para> - <para> - Number of times WAL buffers were written out to disk via - <function>XLogWrite</function> request. - See <xref linkend="wal-configuration"/> for more information about - the internal WAL function <function>XLogWrite</function>. - </para></entry> - </row> - - <row> - <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>wal_sync</structfield> <type>bigint</type> - </para> - <para> - Number of times WAL files were synced to disk via - <function>issue_xlog_fsync</function> request - (if <xref linkend="guc-fsync"/> is <literal>on</literal> and - <xref linkend="guc-wal-sync-method"/> is either - <literal>fdatasync</literal>, <literal>fsync</literal> or - <literal>fsync_writethrough</literal>, otherwise zero). - See <xref linkend="wal-configuration"/> for more information about - the internal WAL function <function>issue_xlog_fsync</function>. - </para></entry> - </row> - - <row> - <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>wal_write_time</structfield> <type>double precision</type> - </para> - <para> - Total amount of time spent writing WAL buffers to disk via - <function>XLogWrite</function> request, in milliseconds - (if <xref linkend="guc-track-wal-io-timing"/> is enabled, - otherwise zero). This includes the sync time when - <varname>wal_sync_method</varname> is either - <literal>open_datasync</literal> or <literal>open_sync</literal>. - </para></entry> - </row> - - <row> - <entry role="catalog_table_entry"><para role="column_definition"> - <structfield>wal_sync_time</structfield> <type>double precision</type> - </para> - <para> - Total amount of time spent syncing WAL files to disk via - <function>issue_xlog_fsync</function> request, in milliseconds - (if <varname>track_wal_io_timing</varname> is enabled, - <varname>fsync</varname> is <literal>on</literal>, and - <varname>wal_sync_method</varname> is either - <literal>fdatasync</literal>, <literal>fsync</literal> or - <literal>fsync_writethrough</literal>, otherwise zero). - </para></entry> - </row> - <row> <entry role="catalog_table_entry"><para role="column_definition"> <structfield>stats_reset</structfield> <type>timestamp with time zone</type> diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml index b908720adea..8f3ac982b0e 100644 --- a/doc/src/sgml/wal.sgml +++ b/doc/src/sgml/wal.sgml @@ -810,11 +810,11 @@ <para> There are two internal functions to write WAL data to disk: <function>XLogWrite</function> and <function>issue_xlog_fsync</function>. - When <xref linkend="guc-track-wal-io-timing"/> is enabled, the total + When <xref linkend="guc-track-io-timing"/> is enabled, the total amounts of time <function>XLogWrite</function> writes and <function>issue_xlog_fsync</function> syncs WAL data to disk are counted as - <literal>wal_write_time</literal> and <literal>wal_sync_time</literal> in - <xref linkend="pg-stat-wal-view"/>, respectively. + <literal>write_time</literal> and <literal>sync_time</literal> in + <xref linkend="pg-stat-io-view"/> for the wal <literal>object</literal>, respectively. <function>XLogWrite</function> is normally called by <function>XLogInsertRecord</function> (when there is no space for the new record in WAL buffers), <function>XLogFlush</function> and the WAL writer, @@ -829,11 +829,11 @@ <literal>fsync</literal>, or <literal>fsync_writethrough</literal>, the write operation moves WAL buffers to kernel cache and <function>issue_xlog_fsync</function> syncs them to disk. Regardless - of the setting of <varname>track_wal_io_timing</varname>, the number + of the setting of <varname>track_io_timing</varname>, the number of times <function>XLogWrite</function> writes and <function>issue_xlog_fsync</function> syncs WAL data to disk are also - counted as <literal>wal_write</literal> and <literal>wal_sync</literal> - in <structname>pg_stat_wal</structname>, respectively. + counted as <literal>writes</literal> and <literal>fsyncs</literal> + in <structname>pg_stat_io</structname> for the wal <literal>object</literal>, respectively. </para> <para> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 25a5c605404..5a1940a42f5 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -134,7 +134,6 @@ int CommitSiblings = 5; /* # concurrent xacts needed to sleep */ int wal_retrieve_retry_interval = 5000; int max_slot_wal_keep_size_mb = -1; int wal_decode_buffer_size = 512 * 1024; -bool track_wal_io_timing = false; #ifdef WAL_DEBUG bool XLOG_DEBUG = false; @@ -2436,10 +2435,9 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible) errno = 0; /* - * Measure I/O timing to write WAL data, for pg_stat_io and/or - * pg_stat_wal. + * Measure I/O timing to write WAL data, for pg_stat_io. */ - start = pgstat_prepare_io_time(track_io_timing || track_wal_io_timing); + start = pgstat_prepare_io_time(track_io_timing); pgstat_report_wait_start(WAIT_EVENT_WAL_WRITE); written = pg_pwrite(openLogFile, from, nleft, startoffset); @@ -2448,20 +2446,6 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible) pgstat_count_io_op_time(IOOBJECT_WAL, IOCONTEXT_NORMAL, IOOP_WRITE, start, 1, written); - /* - * Increment the I/O timing and the number of times WAL data - * were written out to disk. - */ - if (track_wal_io_timing) - { - instr_time end; - - INSTR_TIME_SET_CURRENT(end); - INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_write_time, end, start); - } - - PendingWalStats.wal_write++; - if (written <= 0) { char xlogfname[MAXFNAMELEN]; @@ -8718,10 +8702,9 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli) return; /* - * Measure I/O timing to sync the WAL file for pg_stat_io and/or - * pg_stat_wal. + * Measure I/O timing to sync the WAL file for pg_stat_io. */ - start = pgstat_prepare_io_time(track_io_timing || track_wal_io_timing); + start = pgstat_prepare_io_time(track_io_timing); pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC); switch (wal_sync_method) @@ -8767,21 +8750,8 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli) pgstat_report_wait_end(); - /* - * Increment the I/O timing and the number of times WAL files were synced. - */ - if (track_wal_io_timing) - { - instr_time end; - - INSTR_TIME_SET_CURRENT(end); - INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, end, start); - } - pgstat_count_io_op_time(IOOBJECT_WAL, IOCONTEXT_NORMAL, IOOP_FSYNC, start, 1, 0); - - PendingWalStats.wal_sync++; } /* diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql index eff0990957e..a4d2cfdcaf5 100644 --- a/src/backend/catalog/system_views.sql +++ b/src/backend/catalog/system_views.sql @@ -1189,10 +1189,6 @@ CREATE VIEW pg_stat_wal AS w.wal_fpi, w.wal_bytes, w.wal_buffers_full, - w.wal_write, - w.wal_sync, - w.wal_write_time, - w.wal_sync_time, w.stats_reset FROM pg_stat_get_wal() w; diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/activity/pgstat_wal.c index c1ca65eb991..4dc41a4a590 100644 --- a/src/backend/utils/activity/pgstat_wal.c +++ b/src/backend/utils/activity/pgstat_wal.c @@ -21,8 +21,6 @@ #include "utils/pgstat_internal.h" -PgStat_PendingWalStats PendingWalStats = {0}; - /* * WAL usage counters saved from pgWalUsage at the previous call to * pgstat_report_wal(). This is used to calculate how much WAL usage @@ -118,17 +116,10 @@ pgstat_wal_flush_cb(bool nowait) #define WALSTAT_ACC(fld, var_to_add) \ (stats_shmem->stats.fld += var_to_add.fld) -#define WALSTAT_ACC_INSTR_TIME(fld) \ - (stats_shmem->stats.fld += INSTR_TIME_GET_MICROSEC(PendingWalStats.fld)) WALSTAT_ACC(wal_records, wal_usage_diff); WALSTAT_ACC(wal_fpi, wal_usage_diff); WALSTAT_ACC(wal_bytes, wal_usage_diff); WALSTAT_ACC(wal_buffers_full, wal_usage_diff); - WALSTAT_ACC(wal_write, PendingWalStats); - WALSTAT_ACC(wal_sync, PendingWalStats); - WALSTAT_ACC_INSTR_TIME(wal_write_time); - WALSTAT_ACC_INSTR_TIME(wal_sync_time); -#undef WALSTAT_ACC_INSTR_TIME #undef WALSTAT_ACC LWLockRelease(&stats_shmem->lock); @@ -138,11 +129,6 @@ pgstat_wal_flush_cb(bool nowait) */ prevWalUsage = pgWalUsage; - /* - * Clear out the statistics buffer, so it can be re-used. - */ - MemSet(&PendingWalStats, 0, sizeof(PendingWalStats)); - return false; } @@ -158,18 +144,12 @@ pgstat_wal_init_backend_cb(void) } /* - * To determine whether any WAL activity has occurred since last time, not - * only the number of generated WAL records but also the numbers of WAL - * writes and syncs need to be checked. Because even transaction that - * generates no WAL records can write or sync WAL data when flushing the - * data pages. + * To determine whether WAL usage happened. */ bool pgstat_wal_have_pending_cb(void) { - return pgWalUsage.wal_records != prevWalUsage.wal_records || - PendingWalStats.wal_write != 0 || - PendingWalStats.wal_sync != 0; + return pgWalUsage.wal_records != prevWalUsage.wal_records; } void diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c index e9096a88492..68e16e52ab6 100644 --- a/src/backend/utils/adt/pgstatfuncs.c +++ b/src/backend/utils/adt/pgstatfuncs.c @@ -1637,7 +1637,7 @@ pg_stat_get_backend_io(PG_FUNCTION_ARGS) Datum pg_stat_get_wal(PG_FUNCTION_ARGS) { -#define PG_STAT_GET_WAL_COLS 9 +#define PG_STAT_GET_WAL_COLS 5 TupleDesc tupdesc; Datum values[PG_STAT_GET_WAL_COLS] = {0}; bool nulls[PG_STAT_GET_WAL_COLS] = {0}; @@ -1654,15 +1654,7 @@ pg_stat_get_wal(PG_FUNCTION_ARGS) NUMERICOID, -1, 0); TupleDescInitEntry(tupdesc, (AttrNumber) 4, "wal_buffers_full", INT8OID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 5, "wal_write", - INT8OID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 6, "wal_sync", - INT8OID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 7, "wal_write_time", - FLOAT8OID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 8, "wal_sync_time", - FLOAT8OID, -1, 0); - TupleDescInitEntry(tupdesc, (AttrNumber) 9, "stats_reset", + TupleDescInitEntry(tupdesc, (AttrNumber) 5, "stats_reset", TIMESTAMPTZOID, -1, 0); BlessTupleDesc(tupdesc); @@ -1682,14 +1674,8 @@ pg_stat_get_wal(PG_FUNCTION_ARGS) Int32GetDatum(-1)); values[3] = Int64GetDatum(wal_stats->wal_buffers_full); - values[4] = Int64GetDatum(wal_stats->wal_write); - values[5] = Int64GetDatum(wal_stats->wal_sync); - - /* Convert counters from microsec to millisec for display */ - values[6] = Float8GetDatum(((double) wal_stats->wal_write_time) / 1000.0); - values[7] = Float8GetDatum(((double) wal_stats->wal_sync_time) / 1000.0); - values[8] = TimestampTzGetDatum(wal_stats->stat_reset_timestamp); + values[4] = TimestampTzGetDatum(wal_stats->stat_reset_timestamp); /* Returns the record as Datum */ PG_RETURN_DATUM(HeapTupleGetDatum(heap_form_tuple(tupdesc, values, nulls))); diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index cce73314609..6d4cf7609a7 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -1499,15 +1499,6 @@ struct config_bool ConfigureNamesBool[] = false, NULL, NULL, NULL }, - { - {"track_wal_io_timing", PGC_SUSET, STATS_CUMULATIVE, - gettext_noop("Collects timing statistics for WAL I/O activity."), - NULL - }, - &track_wal_io_timing, - false, - NULL, NULL, NULL - }, { {"update_process_title", PGC_SUSET, PROCESS_TITLE, diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index d472987ed46..c64b3e8a0e6 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -637,7 +637,6 @@ #track_counts = on #track_cost_delay_timing = off #track_io_timing = off -#track_wal_io_timing = off #track_functions = none # none, pl, all #stats_fetch_consistency = cache # cache, none, snapshot diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 4411c1468ac..bd18d95a6cf 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -54,7 +54,6 @@ extern PGDLLIMPORT char *wal_consistency_checking_string; extern PGDLLIMPORT bool log_checkpoints; extern PGDLLIMPORT int CommitDelay; extern PGDLLIMPORT int CommitSiblings; -extern PGDLLIMPORT bool track_wal_io_timing; extern PGDLLIMPORT int wal_decode_buffer_size; extern PGDLLIMPORT int CheckPointSegments; diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat index 9e803d610d7..1e1626964e3 100644 --- a/src/include/catalog/pg_proc.dat +++ b/src/include/catalog/pg_proc.dat @@ -5950,9 +5950,9 @@ { oid => '1136', descr => 'statistics: information about WAL activity', proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's', proparallel => 'r', prorettype => 'record', proargtypes => '', - proallargtypes => '{int8,int8,numeric,int8,int8,int8,float8,float8,timestamptz}', - proargmodes => '{o,o,o,o,o,o,o,o,o}', - proargnames => '{wal_records,wal_fpi,wal_bytes,wal_buffers_full,wal_write,wal_sync,wal_write_time,wal_sync_time,stats_reset}', + proallargtypes => '{int8,int8,numeric,int8,timestamptz}', + proargmodes => '{o,o,o,o,o}', + proargnames => '{wal_records,wal_fpi,wal_bytes,wal_buffers_full,stats_reset}', prosrc => 'pg_stat_get_wal' }, { oid => '6248', descr => 'statistics: information about WAL prefetching', proname => 'pg_stat_get_recovery_prefetch', prorows => '1', proretset => 't', diff --git a/src/include/pgstat.h b/src/include/pgstat.h index 53f2a8458e6..a3a341cc604 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -480,28 +480,9 @@ typedef struct PgStat_WalStats PgStat_Counter wal_fpi; uint64 wal_bytes; PgStat_Counter wal_buffers_full; - PgStat_Counter wal_write; - PgStat_Counter wal_sync; - PgStat_Counter wal_write_time; - PgStat_Counter wal_sync_time; TimestampTz stat_reset_timestamp; } PgStat_WalStats; -/* - * This struct stores wal-related durations as instr_time, which makes it - * cheaper and easier to accumulate them, by not requiring type - * conversions. During stats flush instr_time will be converted into - * microseconds. - */ -typedef struct PgStat_PendingWalStats -{ - PgStat_Counter wal_write; - PgStat_Counter wal_sync; - instr_time wal_write_time; - instr_time wal_sync_time; -} PgStat_PendingWalStats; - - /* * Functions in pgstat.c */ @@ -834,13 +815,4 @@ extern PGDLLIMPORT PgStat_Counter pgStatTransactionIdleTime; /* updated by the traffic cop and in errfinish() */ extern PGDLLIMPORT SessionEndType pgStatSessionEndCause; - -/* - * Variables in pgstat_wal.c - */ - -/* updated directly by backends and background processes */ -extern PGDLLIMPORT PgStat_PendingWalStats PendingWalStats; - - #endif /* PGSTAT_H */ diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 5baba8d39ff..62f69ac20b2 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2259,12 +2259,8 @@ pg_stat_wal| SELECT wal_records, wal_fpi, wal_bytes, wal_buffers_full, - wal_write, - wal_sync, - wal_write_time, - wal_sync_time, stats_reset - FROM pg_stat_get_wal() w(wal_records, wal_fpi, wal_bytes, wal_buffers_full, wal_write, wal_sync, wal_write_time, wal_sync_time, stats_reset); + FROM pg_stat_get_wal() w(wal_records, wal_fpi, wal_bytes, wal_buffers_full, stats_reset); pg_stat_wal_receiver| SELECT pid, status, receive_start_lsn, diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index bce4214503d..740777127e9 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2163,7 +2163,6 @@ PgStat_KindInfo PgStat_LocalState PgStat_PendingDroppedStatsItem PgStat_PendingIO -PgStat_PendingWalStats PgStat_SLRUStats PgStat_ShmemControl PgStat_Snapshot -- 2.34.1 [text/x-diff] v1-0003-remove-the-pgstat_prepare_io_time-parameter.patch (8.3K, ../../Z7RkQ0EfYaqqjgz%[email protected]/4-v1-0003-remove-the-pgstat_prepare_io_time-parameter.patch) download | inline diff: From 6f878f1bbfb119b1dbe95b4cb26b30c97b7a047e Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Tue, 18 Feb 2025 08:52:41 +0000 Subject: [PATCH v1 3/3] remove the pgstat_prepare_io_time() parameter Now that track_wal_io_timing is gone there is no need for pgstat_prepare_io_time() to get a parameter. --- src/backend/access/transam/xlog.c | 8 ++++---- src/backend/access/transam/xlogreader.c | 2 +- src/backend/access/transam/xlogrecovery.c | 2 +- src/backend/storage/buffer/bufmgr.c | 10 +++++----- src/backend/storage/buffer/localbuf.c | 4 ++-- src/backend/storage/smgr/md.c | 4 ++-- src/backend/utils/activity/pgstat_io.c | 7 +++---- src/include/pgstat.h | 2 +- 8 files changed, 19 insertions(+), 20 deletions(-) 31.1% src/backend/access/transam/ 37.2% src/backend/storage/buffer/ 10.5% src/backend/storage/smgr/ 14.7% src/backend/utils/activity/ 6.2% src/include/ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 5a1940a42f5..24624c84748 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -2437,7 +2437,7 @@ XLogWrite(XLogwrtRqst WriteRqst, TimeLineID tli, bool flexible) /* * Measure I/O timing to write WAL data, for pg_stat_io. */ - start = pgstat_prepare_io_time(track_io_timing); + start = pgstat_prepare_io_time(); pgstat_report_wait_start(WAIT_EVENT_WAL_WRITE); written = pg_pwrite(openLogFile, from, nleft, startoffset); @@ -3248,7 +3248,7 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli, errmsg("could not create file \"%s\": %m", tmppath))); /* Measure I/O timing when initializing segment */ - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_WRITE); save_errno = 0; @@ -3310,7 +3310,7 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli, } /* Measure I/O timing when flushing segment */ - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); pgstat_report_wait_start(WAIT_EVENT_WAL_INIT_SYNC); if (pg_fsync(fd) != 0) @@ -8704,7 +8704,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli) /* * Measure I/O timing to sync the WAL file for pg_stat_io. */ - start = pgstat_prepare_io_time(track_io_timing); + start = pgstat_prepare_io_time(); pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC); switch (wal_sync_method) diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 31bffc6f501..995e4f7c26f 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1557,7 +1557,7 @@ WALRead(XLogReaderState *state, #ifndef FRONTEND /* Measure I/O timing when reading segment */ - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); pgstat_report_wait_start(WAIT_EVENT_WAL_READ); #endif diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 473de6710d7..ac70b73749c 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -3401,7 +3401,7 @@ retry: readOff = targetPageOff; /* Measure I/O timing when reading segment */ - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); pgstat_report_wait_start(WAIT_EVENT_WAL_READ); r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 80b0d0c5ded..e880bed4627 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1509,7 +1509,7 @@ WaitReadBuffers(ReadBuffersOperation *operation) io_pages[io_buffers_len++] = BufferGetBlock(buffers[i]); } - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); smgrreadv(operation->smgr, forknum, io_first_block, io_pages, io_buffers_len); pgstat_count_io_op_time(io_object, io_context, IOOP_READ, io_start, 1, io_buffers_len * BLCKSZ); @@ -2401,7 +2401,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, } } - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); /* * Note: if smgrzeroextend fails, we will end up with buffers that are @@ -3858,7 +3858,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, */ bufToWrite = PageSetChecksumCopy((Page) bufBlock, buf->tag.blockNum); - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); /* * bufToWrite is either the shared buffer or a copy, as appropriate. @@ -4459,7 +4459,7 @@ FlushRelationBuffers(Relation rel) PageSetChecksumInplace(localpage, bufHdr->tag.blockNum); - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); smgrwrite(srel, BufTagGetForkNum(&bufHdr->tag), @@ -5916,7 +5916,7 @@ IssuePendingWritebacks(WritebackContext *wb_context, IOContext io_context) sort_pending_writebacks(wb_context->pending_writebacks, wb_context->nr_pending); - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); /* * Coalesce neighbouring writes, but nothing else. For that we iterate diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 64931efaa75..78b65fb160a 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -244,7 +244,7 @@ GetLocalVictimBuffer(void) PageSetChecksumInplace(localpage, bufHdr->tag.blockNum); - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); /* And write... */ smgrwrite(oreln, @@ -414,7 +414,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr, } } - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); /* actually extend relation */ smgrzeroextend(bmr.smgr, fork, first_block, extend_by, false); diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index 7bf0b45e2c3..298754d1b64 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -1381,7 +1381,7 @@ register_dirty_segment(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg) ereport(DEBUG1, (errmsg_internal("could not forward fsync request because request queue is full"))); - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); if (FileSync(seg->mdfd_vfd, WAIT_EVENT_DATA_FILE_SYNC) < 0) ereport(data_sync_elevel(ERROR), @@ -1786,7 +1786,7 @@ mdsyncfiletag(const FileTag *ftag, char *path) need_to_close = true; } - io_start = pgstat_prepare_io_time(track_io_timing); + io_start = pgstat_prepare_io_time(); /* Sync the file. */ result = FileSync(file, WAIT_EVENT_DATA_FILE_SYNC); diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c index 28a431084b8..e7362b52a37 100644 --- a/src/backend/utils/activity/pgstat_io.c +++ b/src/backend/utils/activity/pgstat_io.c @@ -83,15 +83,14 @@ pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op, } /* - * Initialize the internal timing for an IO operation, depending on an - * IO timing GUC. + * Initialize the internal timing for an IO operation. */ instr_time -pgstat_prepare_io_time(bool track_io_guc) +pgstat_prepare_io_time(void) { instr_time io_start; - if (track_io_guc) + if (track_io_timing) INSTR_TIME_SET_CURRENT(io_start); else { diff --git a/src/include/pgstat.h b/src/include/pgstat.h index a3a341cc604..030b101028f 100644 --- a/src/include/pgstat.h +++ b/src/include/pgstat.h @@ -564,7 +564,7 @@ extern bool pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io, BackendType bktype); extern void pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt, uint64 bytes); -extern instr_time pgstat_prepare_io_time(bool track_io_guc); +extern instr_time pgstat_prepare_io_time(void); extern void pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op, instr_time start_time, uint32 cnt, uint64 bytes); -- 2.34.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing @ 2025-02-19 04:32 Michael Paquier <[email protected]> parent: Bertrand Drouvot <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Michael Paquier @ 2025-02-19 04:32 UTC (permalink / raw) To: Bertrand Drouvot <[email protected]>; +Cc: [email protected] On Tue, Feb 18, 2025 at 10:43:15AM +0000, Bertrand Drouvot wrote: > === Remarks As mentioned on a previous thread, I am siding with the removal of these fields. There are more benefits on terms of granularity of this data to encourage users to move away from pg_stat_wal and stick to pg_stat_io. Reasons are posted here: https://www.postgresql.org/message-id/[email protected] > R1. The "bytes" fields differ (as pg_stat_wal.wal_bytes somehow "focus" on the wal > records size while the pg_stat_io's unit is the wal_block_size) so we keep them > in both places. Yes, I don't think we should drop that anyway. There is no equivalent mapping and pg_stat_wal.bytes is not I/O. This is also useful in the WAL stats for the backends. > R2. track_wal_io_timing becomes useless once those fields are gone > from pg_stat_wal > > R3. PendingWalStats becomes empty, so removes it. Sounds fine to me. The simplifications are in pgstat_wal.c, where we can now depend only on the diffs in the WalUsage, which we rely on for EXPLAIN, VACUUM/ANALYZE and pgss. This makes the implementation of the backend WAL stats much simpler. > === Patch structure > > PFA 3 sub patches: > > - 0001: Add details in the pg_stat_io doc about the wal object. That's basically > more or less a copy/paste from the pg_stat_wal fields description that will be > removed in 0002. As we already track them in pg_stat_io, it's done in a dedicated > patch. These additions feel unbalanced with the existing contents, and overlap with the follow-up paragraph about the tuning that can be guessed from the contents of pg_stat_io because the new content refers twice to the section "WAL Configuration". Perhaps this could be simpler, with one sentence in the tuning part? I would propose something like: For the WAL object, "fsyncs" and "sync_time" track the sync activity of WAL files via issue_xlog_fsync(). "writes" and "write_time" track the write activity of WAL files via XLogWrite(). See <xref linkend="wal-configuration"/> for more information. My point is that the "WAL configuration" section already provides all the details that were in pg_stat_wal removed in 0002 and added in 0001, leading to duplicated contents. > - 0002: Remove wal_[sync|write][_time] from pg_stat_wal, PendingWalStats and > track_wal_io_timing. That does not make sense to split this work in sub-patches > so that all of this in done in 0002. > - 0003: remove the pgstat_prepare_io_time() parameter. Now that track_wal_io_timing > is gone there is no need for pgstat_prepare_io_time() to get a parameter. 0002 and 0003 can be grouped in the same commit, IMO. - When <xref linkend="guc-track-wal-io-timing"/> is enabled, the total + When <xref linkend="guc-track-io-timing"/> is enabled, the total amounts of time <function>XLogWrite</function> writes and <function>issue_xlog_fsync</function> syncs WAL data to disk are counted as - <literal>wal_write_time</literal> and <literal>wal_sync_time</literal> in - <xref linkend="pg-stat-wal-view"/>, respectively. + <literal>write_time</literal> and <literal>sync_time</literal> in + <xref linkend="pg-stat-io-view"/> for the wal <literal>object</literal>, respectively. Okay to update these are part of 0002 that removes the fields, but there is also an argument about updating that on its own because this is actually the case on HEAD? (No need to post an updated patch for this remark.) The removal with PgStat_PendingWalStats and the GUC gone looks clean as presented. -- Michael Attachments: [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc) download ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-02-19 04:32 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-03-19 06:11 [PATCH v46 5/7] Doc part of shared-memory based stats collector. Kyotaro Horiguchi <[email protected]> 2025-02-18 10:43 Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing Bertrand Drouvot <[email protected]> 2025-02-19 04:32 ` Re: Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox