Re: [HACKERS] [PATCH] A hook for session start

On Mon, Nov 20, 2017 at 7:56 AM, Andrew Dunstan
<andrew(dot)dunstan(at)2ndquadrant(dot)com> wrote:
> On 11/19/2017 04:49 PM, Tom Lane wrote:
>> Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
>>> I think this:
>>> #define IsClientBackend() \
>>> (MyBackendId != InvalidBackendId && \
>>> !IsAutoVacuumLauncherProcess() && \
>>> !IsAutoVacuumWorkerProcess() && \
>>> !am_walsender && \
>>> !IsBackgroundWorker)
>>> probably belongs somewhere more central. Surely this isn't the only
>>> place that we want to be able to run such a test?
>> Hm. It also seems awfully awkward. Perhaps it's not being used anyplace
>> performance-critical, but regardless of speed it seems like a modularity
>> violation, in that client backends have to be explicitly aware of
>> everything that isn't a "client backend".
>>
>> Maybe it's time to invent something corresponding to AuxProcType
>> for non "aux" processes, or else to fold all types of Postgres
>> processes into the same enum.

I think that this is close to the concept of BackendType in pgstat.h.

> Yes, agreed, The above is pretty ugly and likely to be quite fragile.

I was the one suggesting to Fabrizio to look at how backend types are
evaluated in pgstat.c after an off-list discussion. Agreed that this
result is fragile as this makes two places dependent on the process
types. Why not simply moving all the business of pgstat_bestart()
evaluating which st_backendType to use into a common routine and have
pgstat.c and have this utility use this API? pgstat.h already includes
BackendType as an enum to use so this could live with a routine
available in pgstat.c. Or would it be cleaner to have a new thing
dedicated to process-related information, say as
src/backend/utils/misc/process_info.c? It is indeed strange to have a
session-related feature depend on something with statistics.
--
Michael