Date: Fri, 28 Feb 2025 05:16:21 +0000
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>,
	Guillaume Lelarge <guillaume@lelarge.info>,
	Pg Hackers <pgsql-hackers@postgresql.org>,
	Daniel Gustafsson <daniel@yesql.se>,
	andrey.chudnovskiy@microsoft.com,
	Jelte Fennema-Nio <postgres@jeltef.nl>,
	Jacob Champion <jacob.champion@enterprisedb.com>
Subject: Re: Log connection establishment timings
Message-ID: <Z8FGpTkLIF7M3+M/@ip-10-97-1-34.eu-west-3.compute.internal>
References: <CAAKRu_b_smAHK0ZjrnL5GRxnAVWujEXQWpLXYzGbmpcZd3nLYw@mail.gmail.com>
 <CAECtzeW3bFOT8v_th9hu6O9c0p+yNkD0ceBneWOtXYcwVd73rA@mail.gmail.com>
 <Z45lUorFBcmCKxp7@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAAKRu_YrNsA7-v5L9d318XZu+tPqcxp+ctCGy2EGYrSt69ON=A@mail.gmail.com>
 <CAAKRu_aV+B2yLpkcfh06tJV7XQPodjasxQ=XKh+MjD8ixkXwrA@mail.gmail.com>
 <65fe2d68-a214-40f0-80a8-56ca7323ad5c@oss.nttdata.com>
 <Z77Fxf49c+LIW74a@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAAKRu_bWRipJ2HA8cwvxrZOhSpvpPLkvhcBHecXBx8_wS4cjQQ@mail.gmail.com>
 <Z8ALQSr1o0AoGpAo@ip-10-97-1-34.eu-west-3.compute.internal>
 <CAAKRu_Zv7e1GWHiTJ9UOZEoqs8hO8sCezZZNuPv7s5mGYe_QkA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAAKRu_Zv7e1GWHiTJ9UOZEoqs8hO8sCezZZNuPv7s5mGYe_QkA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Z8FGpTkLIF7M3%2BM/%40ip-10-97-1-34.eu-west-3.compute.internal>
Precedence: bulk

Hi,

On Thu, Feb 27, 2025 at 11:08:04AM -0500, Melanie Plageman wrote:
> I was just talking to Andres off-list and he mentioned that the volume
> of log_connections messages added in recent releases can really be a
> problem for users. He said ideally we would emit one message which
> consolidated these (and make sure we did so for failed connections too
> detailing the successfully completed stages).
> 
> However, since that is a bigger project (with more refactoring, etc),
> he suggested that we change log_connections to a GUC_LIST
> (ConfigureNamesString) with options like "none", "received,
> "authenticated", "authorized", "all".
> 
> Then we could add one like "established" for the final message and
> timings my patch set adds. I think the overhead of an additional log
> message being emitted probably outweighs the overhead of taking those
> additional timings.
> 
> String GUCs are a lot more work than enum GUCs, so I was thinking if
> there is a way to do it as an enum.
> 
> I think we want the user to be able to specify a list of all the log
> messages they want included, not just have each one include the
> previous ones. So, then it probably has to be a list right? There is
> no good design that would fit as an enum.

Interesting idea... Yeah, that would sound weird with an enum. I could think
about providing an enum per possible combination but I think that would
generate things like 2^N enum and won't be really user friendly (also that would
double each time we'd want to add a new possible "value" becoming quickly
unmanageable).

So yeah, I can't think of anything better than GUC_LIST.

> > I think that's somehow also around code maintenance (not only LOC), say for example
> > if we want to add more "child_type" in the check (no need to remember to update both
> > locations).
> 
> I didn't include checking the child_type in that function since it is
> unrelated to instr_time, so it sadly wouldn't help with that. We could
> macro-ize the child_type check were we to add another child_type.

Yup but my idea was to put all those line:

"
    if (Log_connections &&
        (child_type == B_BACKEND || child_type == B_WAL_SENDER))
    {
        instr_time  fork_time = ((BackendStartupData *) startup_data)->fork_time;

        conn_timing.fork_duration = INSTR_TIME_GET_DURATION_SINCE(fork_time);
    }
"

into a dedicated helper function.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com