Remove extra Logging_collector check before calling SysLogger_Start()
Hi,
It seems like there's an extra Logging_collector check before calling
SysLogger_Start(). Note that the SysLogger_Start() has a check to
return 0 if Logging_collector is off. This change is consistent with
the other usage of SysLogger_Start().
/* If we have lost the log collector, try to start a new one */
- if (SysLoggerPID == 0 && Logging_collector)
+ if (SysLoggerPID == 0)
SysLoggerPID = SysLogger_Start();Attaching a tiny patch to fix. Thoughts?
Regards,
Bharath Rupireddy.
Attachments:
v1-0001-remove-extra-Logging_collector-check-before-SysLo.patchapplication/octet-stream; name=v1-0001-remove-extra-Logging_collector-check-before-SysLo.patchDownload
From bd37d4e3115d40837f2ba1787db2076844334294 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Fri, 3 Dec 2021 07:56:49 +0000
Subject: [PATCH v1] remove extra Logging_collector check before
SysLogger_Start()
---
src/backend/postmaster/postmaster.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 328ecafa8c..c465af08ac 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1813,7 +1813,7 @@ ServerLoop(void)
}
/* If we have lost the log collector, try to start a new one */
- if (SysLoggerPID == 0 && Logging_collector)
+ if (SysLoggerPID == 0)
SysLoggerPID = SysLogger_Start();
/*
--
2.25.1
On 3 Dec 2021, at 08:58, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
It seems like there's an extra Logging_collector check before calling
SysLogger_Start(). Note that the SysLogger_Start() has a check to
return 0 if Logging_collector is off. This change is consistent with
the other usage of SysLogger_Start()./* If we have lost the log collector, try to start a new one */ - if (SysLoggerPID == 0 && Logging_collector) + if (SysLoggerPID == 0) SysLoggerPID = SysLogger_Start();
I think the code reads clearer with the Logging_collector check left intact,
and avoiding a function call in this codepath doesn't hurt.
--
Daniel Gustafsson https://vmware.com/
On Fri, Dec 3, 2021 at 1:43 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 3 Dec 2021, at 08:58, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
It seems like there's an extra Logging_collector check before calling
SysLogger_Start(). Note that the SysLogger_Start() has a check to
return 0 if Logging_collector is off. This change is consistent with
the other usage of SysLogger_Start()./* If we have lost the log collector, try to start a new one */ - if (SysLoggerPID == 0 && Logging_collector) + if (SysLoggerPID == 0) SysLoggerPID = SysLogger_Start();I think the code reads clearer with the Logging_collector check left intact,
and avoiding a function call in this codepath doesn't hurt.
In that case, can we remove if (!Logging_collector) within
SysLogger_Start() and have the check outside? This will save function
call costs in the other places too. The pgarch_start and
AutoVacuumingActive have checks outside PgArchStartupAllowed and
AutoVacuumingActive.
Regards,
Bharath Rupireddy.
Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> writes:
On Fri, Dec 3, 2021 at 1:43 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 3 Dec 2021, at 08:58, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote:
It seems like there's an extra Logging_collector check before calling
SysLogger_Start().
I think the code reads clearer with the Logging_collector check left intact,
and avoiding a function call in this codepath doesn't hurt.
In that case, can we remove if (!Logging_collector) within
SysLogger_Start() and have the check outside?
I think it's fine as-is; good belt-and-suspenders-too programming.
It's not like the extra check inside SysLogger_Start() is costly.
regards, tom lane
