[CF 5579] v3 - Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing

This branch was automatically generated by a robot using es from an email thread registered at: https://commitfest.postgresql.org//5579 The branch will be overwritten each time a new version is posted to the thread, and also periodically to check for bitrot caused by changes on the master branch. (es): https://www.postgresql.org/message-id/[email protected] Author(s): Bertrand Drouvot

Author: Commitfest Bot

doc/src/sgml/config.sgml

@@ -8393,28 +8393,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>

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
@@ -3265,63 +3260,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>

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();
 
 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];
@@ -3264,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;
@@ -3326,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)
@@ -8758,10 +8742,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();
 
 pgstat_report_wait_start(WAIT_EVENT_WAL_SYNC);
 switch (wal_sync_method)
@@ -8807,21 +8790,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++;
 }
 
 /*

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

src/backend/access/transam/xlogrecovery.c

@@ -3401,7 +3401,7 @@ XLogPageRead(XLogReaderState *xlogreader, XLogRecPtr targetPagePtr, int reqLen,
 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);

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;
 

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.
@@ -4460,7 +4460,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),
@@ -5917,7 +5917,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 neiguring writes, but nothing else. For that we iterate

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);

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);

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
 {

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