Refactor pgstat_prepare_io_time() with an input argument instead of a GUC

Originally, this routine relied on track_io_timing to check if a time
interval for an I/O operation stored in pg_stat_io should be initialized
or not.  However, the addition of WAL statistics to pg_stat_io requires
that the initialization happens when track_wal_io_timing is enabled,
which is dependent on the code path where the I/O operation happens.

Author: Nazir Bilal Yavuz
Discussion: https://postgr.es/m/CAN55FZ3AiQ+ZMxUuXnBpd0Rrh1YhwJ5FudkHg=JU0P+-W8T4Vg@mail.gmail.com

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index de8b5edbfbfbeb8f5b9304cd79ba646e940b33cd..9f9d3f24ac2885cb55d82b4b51941806cf8b8ea9 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1143,7 +1143,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
                MemSet((char *) bufBlock, 0, BLCKSZ);
        else
        {
-               instr_time      io_start = pgstat_prepare_io_time();
+               instr_time      io_start = pgstat_prepare_io_time(track_io_timing);
 
                smgrread(smgr, forkNum, blockNum, bufBlock);
 
@@ -2070,7 +2070,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr,
                }
        }
 
-       io_start = pgstat_prepare_io_time();
+       io_start = pgstat_prepare_io_time(track_io_timing);
 
        /*
         * Note: if smgrzeroextend fails, we will end up with buffers that are
@@ -3523,7 +3523,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
         */
        bufToWrite = PageSetChecksumCopy((Page) bufBlock, buf->tag.blockNum);
 
-       io_start = pgstat_prepare_io_time();
+       io_start = pgstat_prepare_io_time(track_io_timing);
 
        /*
         * bufToWrite is either the shared buffer or a copy, as appropriate.
@@ -4181,7 +4181,7 @@ FlushRelationBuffers(Relation rel)
 
                                PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-                               io_start = pgstat_prepare_io_time();
+                               io_start = pgstat_prepare_io_time(track_io_timing);
 
                                smgrwrite(RelationGetSmgr(rel),
                                                  BufTagGetForkNum(&bufHdr->tag),
@@ -5614,7 +5614,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();
+       io_start = pgstat_prepare_io_time(track_io_timing);
 
        /*
         * 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 c75ed184e04434be04b61329c05a968be14ab124..d1cdd3eeb437392831f8546e4c448144d2e5d3da 100644 (file)
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -246,7 +246,7 @@ GetLocalVictimBuffer(void)
 
                PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
 
-               io_start = pgstat_prepare_io_time();
+               io_start = pgstat_prepare_io_time(track_io_timing);
 
                /* And write... */
                smgrwrite(oreln,
@@ -411,7 +411,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr,
                }
        }
 
-       io_start = pgstat_prepare_io_time();
+       io_start = pgstat_prepare_io_time(track_io_timing);
 
        /* 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 091993ea45b6e72596c96a00ed796b30abcc45a1..bbac3c874138275e5f6aa40b91f54ad6106b4697 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -1189,7 +1189,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();
+               io_start = pgstat_prepare_io_time(track_io_timing);
 
                if (FileSync(seg->mdfd_vfd, WAIT_EVENT_DATA_FILE_SYNC) < 0)
                        ereport(data_sync_elevel(ERROR),
@@ -1586,7 +1586,7 @@ mdsyncfiletag(const FileTag *ftag, char *path)
                need_to_close = true;
        }
 
-       io_start = pgstat_prepare_io_time();
+       io_start = pgstat_prepare_io_time(track_io_timing);
 
        /* 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 490d5a9ab79a0832e16f591565399ca18ee88d5c..d99ecdd4d8e5531a81b0a3c011b7a4e41727ba35 100644 (file)
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -92,15 +92,25 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
        have_iostats = true;
 }
 
+/*
+ * Initialize the internal timing for an IO operation, depending on an
+ * IO timing GUC.
+ */
 instr_time
-pgstat_prepare_io_time(void)
+pgstat_prepare_io_time(bool track_io_guc)
 {
        instr_time      io_start;
 
-       if (track_io_timing)
+       if (track_io_guc)
                INSTR_TIME_SET_CURRENT(io_start);
        else
+       {
+               /*
+                * There is no need to set io_start when an IO timing GUC is disabled,
+                * still initialize it to zero to avoid compiler warnings.
+                */
                INSTR_TIME_SET_ZERO(io_start);
+       }
 
        return io_start;
 }

diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index f95d8db0c4cc8d56dbf89b22f4bd820ca90620fb..fc93d0d731daa6c21634f800a5229f0b0663c173 100644 (file)
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -519,7 +519,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);
 extern void pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt);
-extern instr_time pgstat_prepare_io_time(void);
+extern instr_time pgstat_prepare_io_time(bool track_io_guc);
 extern void pgstat_count_io_op_time(IOObject io_object, IOContext io_context,
                                                                        IOOp io_op, instr_time start_time, uint32 cnt);