Remove extra Logging_collector check before calling SysLogger_Start()

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

On Fri, Dec 03, 2021 at 12:45:34PM -0500, Tom Lane wrote:

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.

+1.
--
Michael