Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 3c9d9ac

Browse files
committed
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
1 parent a6be060 commit 3c9d9ac

File tree

5 files changed

+22
-12
lines changed

5 files changed

+22
-12
lines changed

src/backend/storage/buffer/bufmgr.c

+5-5
Original file line numberDiff line numberDiff line change
@@ -1143,7 +1143,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
11431143
MemSet((char *) bufBlock, 0, BLCKSZ);
11441144
else
11451145
{
1146-
instr_time io_start = pgstat_prepare_io_time();
1146+
instr_time io_start = pgstat_prepare_io_time(track_io_timing);
11471147

11481148
smgrread(smgr, forkNum, blockNum, bufBlock);
11491149

@@ -2070,7 +2070,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr,
20702070
}
20712071
}
20722072

2073-
io_start = pgstat_prepare_io_time();
2073+
io_start = pgstat_prepare_io_time(track_io_timing);
20742074

20752075
/*
20762076
* Note: if smgrzeroextend fails, we will end up with buffers that are
@@ -3523,7 +3523,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
35233523
*/
35243524
bufToWrite = PageSetChecksumCopy((Page) bufBlock, buf->tag.blockNum);
35253525

3526-
io_start = pgstat_prepare_io_time();
3526+
io_start = pgstat_prepare_io_time(track_io_timing);
35273527

35283528
/*
35293529
* bufToWrite is either the shared buffer or a copy, as appropriate.
@@ -4181,7 +4181,7 @@ FlushRelationBuffers(Relation rel)
41814181

41824182
PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
41834183

4184-
io_start = pgstat_prepare_io_time();
4184+
io_start = pgstat_prepare_io_time(track_io_timing);
41854185

41864186
smgrwrite(RelationGetSmgr(rel),
41874187
BufTagGetForkNum(&bufHdr->tag),
@@ -5614,7 +5614,7 @@ IssuePendingWritebacks(WritebackContext *wb_context, IOContext io_context)
56145614
sort_pending_writebacks(wb_context->pending_writebacks,
56155615
wb_context->nr_pending);
56165616

5617-
io_start = pgstat_prepare_io_time();
5617+
io_start = pgstat_prepare_io_time(track_io_timing);
56185618

56195619
/*
56205620
* Coalesce neighbouring writes, but nothing else. For that we iterate

src/backend/storage/buffer/localbuf.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ GetLocalVictimBuffer(void)
246246

247247
PageSetChecksumInplace(localpage, bufHdr->tag.blockNum);
248248

249-
io_start = pgstat_prepare_io_time();
249+
io_start = pgstat_prepare_io_time(track_io_timing);
250250

251251
/* And write... */
252252
smgrwrite(oreln,
@@ -411,7 +411,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr,
411411
}
412412
}
413413

414-
io_start = pgstat_prepare_io_time();
414+
io_start = pgstat_prepare_io_time(track_io_timing);
415415

416416
/* actually extend relation */
417417
smgrzeroextend(bmr.smgr, fork, first_block, extend_by, false);

src/backend/storage/smgr/md.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ register_dirty_segment(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg)
11891189
ereport(DEBUG1,
11901190
(errmsg_internal("could not forward fsync request because request queue is full")));
11911191

1192-
io_start = pgstat_prepare_io_time();
1192+
io_start = pgstat_prepare_io_time(track_io_timing);
11931193

11941194
if (FileSync(seg->mdfd_vfd, WAIT_EVENT_DATA_FILE_SYNC) < 0)
11951195
ereport(data_sync_elevel(ERROR),
@@ -1586,7 +1586,7 @@ mdsyncfiletag(const FileTag *ftag, char *path)
15861586
need_to_close = true;
15871587
}
15881588

1589-
io_start = pgstat_prepare_io_time();
1589+
io_start = pgstat_prepare_io_time(track_io_timing);
15901590

15911591
/* Sync the file. */
15921592
result = FileSync(file, WAIT_EVENT_DATA_FILE_SYNC);

src/backend/utils/activity/pgstat_io.c

+12-2
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,25 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
9292
have_iostats = true;
9393
}
9494

95+
/*
96+
* Initialize the internal timing for an IO operation, depending on an
97+
* IO timing GUC.
98+
*/
9599
instr_time
96-
pgstat_prepare_io_time(void)
100+
pgstat_prepare_io_time(bool track_io_guc)
97101
{
98102
instr_time io_start;
99103

100-
if (track_io_timing)
104+
if (track_io_guc)
101105
INSTR_TIME_SET_CURRENT(io_start);
102106
else
107+
{
108+
/*
109+
* There is no need to set io_start when an IO timing GUC is disabled,
110+
* still initialize it to zero to avoid compiler warnings.
111+
*/
103112
INSTR_TIME_SET_ZERO(io_start);
113+
}
104114

105115
return io_start;
106116
}

src/include/pgstat.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ extern bool pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
519519
BackendType bktype);
520520
extern void pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op);
521521
extern void pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt);
522-
extern instr_time pgstat_prepare_io_time(void);
522+
extern instr_time pgstat_prepare_io_time(bool track_io_guc);
523523
extern void pgstat_count_io_op_time(IOObject io_object, IOContext io_context,
524524
IOOp io_op, instr_time start_time, uint32 cnt);
525525

0 commit comments

Comments
 (0)