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