Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tkL4W-002xAn-Tl for pgsql-hackers@arkaria.postgresql.org; Tue, 18 Feb 2025 10:43:25 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1tkL4V-0093PS-LZ for pgsql-hackers@arkaria.postgresql.org; Tue, 18 Feb 2025 10:43:23 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1tkL4V-0093Lk-3E for pgsql-hackers@lists.postgresql.org; Tue, 18 Feb 2025 10:43:23 +0000 Received: from mail-wr1-x436.google.com ([2a00:1450:4864:20::436]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.96) (envelope-from ) id 1tkL4R-001Uwd-2f for pgsql-hackers@lists.postgresql.org; Tue, 18 Feb 2025 10:43:22 +0000 Received: by mail-wr1-x436.google.com with SMTP id ffacd0b85a97d-38f406e9f80so1877041f8f.2 for ; Tue, 18 Feb 2025 02:43:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1739875398; x=1740480198; darn=lists.postgresql.org; h=content-disposition:mime-version:message-id:subject:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=ecMZEacqqlO4dPLEdsjgZNjkoPREHbD6/mU6KiRpj9k=; b=aUnm+QZQQB7cQEyN6i+Tq2TdM35YhcUYtN+wGG1Phzb9qY5MyvPLUr+k01qIKVAUmU DMf8yktEOgY0JgmyeODU+ALQHOi+f2PpT3QPUU6DLj/Lt7t7zUiCAtiZry82vlPDrvL4 x98BNkhEj69ZfXOXmAo4uAmBoRZ6lrzUKpczYWKs/1V10lHI9Mn3aByD4gxnss5WY+4k ByV00nOgME23tGjU0JCj45rLWzqnzNbEIh0UeOq0ekRrP5vk3TFk05arH8ZL0cRrf0i5 5B4OTq/duWIqbENICxzFFNYuy71JV/9XRCU7HllYhGAjHCUIFarNW5Do+97Ek+oABUYt gZ1w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1739875398; x=1740480198; h=content-disposition:mime-version:message-id:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ecMZEacqqlO4dPLEdsjgZNjkoPREHbD6/mU6KiRpj9k=; b=HsWKzHHXZ7DpoSF7xLte9NvP0qZx49zhFYlFMVXY9wNKSzMPYGL/QVnjkISv22IjL6 a5ML/FSvMhVeFymVApBaTdAwJfXPI06KkLSAi4r+YXn99iggG6NT+pKaMAS/s6plnud1 RN+LIHeMp3MyfDhPf8iZvHgAxVRtGRVVEDzE6WOGswq4QQPErixrHQGsLl0Vz/zTeiUZ jMrtdPSlP688Nb/J7YfwGeQDqM7J37WeAokdp/Cr/ps5h3YAzHB/COuipEjJRk7MOuIp 5CMrrXW6Kt8qmjNrx4tblIvcfIzCQw7zmyZqklg2rQmPsxcpg711ZXR6XLLFL7PfxrIp q01Q== X-Gm-Message-State: AOJu0Yx/YQ04DHN1pgbHauylVGvWdzh2ilyKXduuZKf98AqLE90d520C 0FXAe9nWEGnNLrkKZmm1TuKTPpg2JNwVQmHIc93qKVB2HclXvLwjGP6YZQ== X-Gm-Gg: ASbGncsJ3JDoOVo9pAS55LNdwLujQ9y9/SyhMLves0Bvk/Dnq0wSa6gvUpJLvd8SMa3 qKv0JwZJcEnZuTEIYAUUGrhnWMP6+lY+ED/ZwCtoZUX70UmzZ9e70LgewjfLI/6oSSTaRq4iG8h F7UqzJv36uTDAV0o2b9PD7hQ7i/8IHIP0RXKf9HJMGC6tACGlfWoNEOvZCliPX9KphT52SAwnIp M+Ba4JgZCSKvAqoMDK92FuKGdh0KFjyCiP7PIryyieDHX8LSfG0XifKQE5NPXb/l82FG7YR+jXq 1jvzLWCRAhAo1QGbZLhZDAfLu+6ND7q7ziHqmL9412ioX+ZnuLJY1sw+sPtrvUCI6+kyhwdfual 2bYOIxIqKmQo/MpIPJwGToYifyv3CANTtf/YfvUUhRmDi4vo= X-Google-Smtp-Source: AGHT+IHH2qDm08lTCjRHrOfc8J9YLMBN2lqGTv4vSdfbeL/LsWRgIT7fdfuXCvilSi1kesB3xTh7kg== X-Received: by 2002:a05:6000:2c3:b0:38f:2224:738e with SMTP id ffacd0b85a97d-38f33f105a1mr13373263f8f.1.1739875398123; Tue, 18 Feb 2025 02:43:18 -0800 (PST) Received: from ip-10-97-1-34.eu-west-3.compute.internal (ec2-15-237-181-182.eu-west-3.compute.amazonaws.com. [15.237.181.182]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38f25915688sm14850409f8f.57.2025.02.18.02.43.17 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Feb 2025 02:43:17 -0800 (PST) Date: Tue, 18 Feb 2025 10:43:15 +0000 From: Bertrand Drouvot To: pgsql-hackers@lists.postgresql.org Subject: Remove wal_[sync|write][_time] from pg_stat_wal and track_wal_io_timing Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="cVb/prkuj9W16mit" Content-Disposition: inline List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --cVb/prkuj9W16mit Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --cVb/prkuj9W16mit Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0001-Add-details-in-the-pg_stat_io-doc-about-the-wal-o.patch" From bc9dec707e2b12a6c6e7d1f654fbe41c8e5bbb65 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot 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. + + For the wal object: + + + + writes is the number of times WAL buffers were + written out to disk via XLogWrite request. + See for more information about the internal + WAL function XLogWrite. + + + + + fsyncs is the number of times WAL files were + synced to disk via issue_xlog_fsync request (if + is on and is either + fdatasync, fsync or fsync_writethrough, + otherwise zero). See for more information + about the internal WAL function issue_xlog_fsync. + + + + + write_time is the total amount of time spent writing + WAL buffers to disk via XLogWrite request. + This includes the sync time when wal_sync_method + is either open_datasync or open_sync. + + + + + sync_time is the total amount of time spent syncing + WAL files to disk via issue_xlog_fsync request (if + fsync is on, + and wal_sync_method is either fdatasync, + fsync or fsync_writethrough, + otherwise zero). + + + + + pg_stat_io can be used to inform database tuning. For example: -- 2.34.1 --cVb/prkuj9W16mit Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0002-Remove-wal_-sync-write-_time-from-pg_stat_wal-and.patch" From 42b2d46d80f082829c91f9c91f844ab750f2cafd Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot 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; - - track_wal_io_timing (boolean) - - track_wal_io_timing configuration parameter - - - - - 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 pg_test_timing tool to - measure the overhead of timing on your system. - I/O timing information is - displayed in - pg_stat_wal. - Only superusers and users with the appropriate SET - privilege can change this setting. - - - - track_functions (enum) 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. - - The parameter enables monitoring - of WAL write and fsync times. - - Normally these parameters are set in postgresql.conf 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 - - - wal_write bigint - - - Number of times WAL buffers were written out to disk via - XLogWrite request. - See for more information about - the internal WAL function XLogWrite. - - - - - - wal_sync bigint - - - Number of times WAL files were synced to disk via - issue_xlog_fsync request - (if is on and - is either - fdatasync, fsync or - fsync_writethrough, otherwise zero). - See for more information about - the internal WAL function issue_xlog_fsync. - - - - - - wal_write_time double precision - - - Total amount of time spent writing WAL buffers to disk via - XLogWrite request, in milliseconds - (if is enabled, - otherwise zero). This includes the sync time when - wal_sync_method is either - open_datasync or open_sync. - - - - - - wal_sync_time double precision - - - Total amount of time spent syncing WAL files to disk via - issue_xlog_fsync request, in milliseconds - (if track_wal_io_timing is enabled, - fsync is on, and - wal_sync_method is either - fdatasync, fsync or - fsync_writethrough, otherwise zero). - - - stats_reset timestamp with time zone 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 @@ There are two internal functions to write WAL data to disk: XLogWrite and issue_xlog_fsync. - When is enabled, the total + When is enabled, the total amounts of time XLogWrite writes and issue_xlog_fsync syncs WAL data to disk are counted as - wal_write_time and wal_sync_time in - , respectively. + write_time and sync_time in + for the wal object, respectively. XLogWrite is normally called by XLogInsertRecord (when there is no space for the new record in WAL buffers), XLogFlush and the WAL writer, @@ -829,11 +829,11 @@ fsync, or fsync_writethrough, the write operation moves WAL buffers to kernel cache and issue_xlog_fsync syncs them to disk. Regardless - of the setting of track_wal_io_timing, the number + of the setting of track_io_timing, the number of times XLogWrite writes and issue_xlog_fsync syncs WAL data to disk are also - counted as wal_write and wal_sync - in pg_stat_wal, respectively. + counted as writes and fsyncs + in pg_stat_io for the wal object, respectively. 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 --cVb/prkuj9W16mit Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v1-0003-remove-the-pgstat_prepare_io_time-parameter.patch" From 6f878f1bbfb119b1dbe95b4cb26b30c97b7a047e Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot 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 --cVb/prkuj9W16mit--