Small patch modifying variable name to reflect the logic involved

Started by Krishnakumar Rover 2 years ago3 messages
#1Krishnakumar R
kksrcv001@gmail.com
1 attachment(s)

Hi All,

Please find a small patch to improve code readability by modifying
variable name to reflect the logic involved - finding diff between end
and start time of WAL sync.

--
Thanks and Regards,
Krishnakumar (KK)
[Microsoft]

Attachments:

v1-0001-Improve-code-readability-by-modifying-variable-na.patchapplication/octet-stream; name=v1-0001-Improve-code-readability-by-modifying-variable-na.patchDownload
From 1eec24f5ef379e51c7680c5a9a573375cf395111 Mon Sep 17 00:00:00 2001
From: "Krishnakumar R (KK)" <kksrcv001@gmail.com>
Date: Wed, 13 Sep 2023 14:53:27 -0700
Subject: [PATCH v1] Improve code readability by modifying variable name to
 reflect the logic involved - finding diff between end and start time of wal
 sync.

---
 src/backend/access/transam/xlog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index f6f8adc72a..9987184231 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8253,10 +8253,10 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli)
 	 */
 	if (track_wal_io_timing)
 	{
-		instr_time	duration;
+		instr_time	end;
 
-		INSTR_TIME_SET_CURRENT(duration);
-		INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, duration, start);
+		INSTR_TIME_SET_CURRENT(end);
+		INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, end, start);
 	}
 
 	PendingWalStats.wal_sync++;
-- 
2.34.1

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Krishnakumar R (#1)
Re: Small patch modifying variable name to reflect the logic involved

On 14 Sep 2023, at 08:28, Krishnakumar R <kksrcv001@gmail.com> wrote:

Please find a small patch to improve code readability by modifying
variable name to reflect the logic involved - finding diff between end
and start time of WAL sync.

-	INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, duration, start);
+	INSTR_TIME_SET_CURRENT(end);
+	INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, end, start);

Agreed, the duration is the result of the INSTR_TIME_ACCUM_DIFF calculation,
not what's stored in the instr_time variable.

--
Daniel Gustafsson

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Daniel Gustafsson (#2)
Re: Small patch modifying variable name to reflect the logic involved

On 14 Sep 2023, at 11:30, Daniel Gustafsson <daniel@yesql.se> wrote:

On 14 Sep 2023, at 08:28, Krishnakumar R <kksrcv001@gmail.com> wrote:

Please find a small patch to improve code readability by modifying
variable name to reflect the logic involved - finding diff between end
and start time of WAL sync.

-	INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, duration, start);
+	INSTR_TIME_SET_CURRENT(end);
+	INSTR_TIME_ACCUM_DIFF(PendingWalStats.wal_sync_time, end, start);

Agreed, the duration is the result of the INSTR_TIME_ACCUM_DIFF calculation,
not what's stored in the instr_time variable.

And done, with a small fixup to handle another occurrence in the same file.

--
Daniel Gustafsson