Re: Log connection establishment timings - Mailing list pgsql-hackers

Hi,

On Mon, Mar 03, 2025 at 06:24:59PM -0500, Melanie Plageman wrote:
> On Mon, Mar 3, 2025 at 11:14 AM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> >
> > On Fri, Feb 28, 2025 at 05:52:35PM -0500, Melanie Plageman wrote:
> >
> > +bool
> > +check_log_connections(char **newval, void **extra, GucSource source)
> > +{
> >
> > This function is pretty close to check_debug_io_direct() for example and its
> > overall content, memory management, looks good to me. I just have a few
> > following comments about it.
> 
> So, I started thinking more about this patch. I wonder if what we
> really want to do is add a new GUC type that is somewhere between a
> list and an enum. What I'm imagining is basically a set -- you could
> specify parameters in a comma separated list like before but without
> quotes:
> 
> SET log_connections = received,authorized;
> 
> It would be for GUCs that can be any combination of a set of
> predetermined values.
> There are actually a few list gucs that would benefit from this
> structure: see log_destination, debug_io_direct,
> restrict_nonsystem_relation_kind. And I imagine there are others.
> 
> It would reduce code duplication for those (see the boiler plate
> string parsing and handling for all of these check functions). If this
> was implemented, log_connections wouldn't even need its own check
> function (I think).
> 
> It also might be easier to do tab completion (depending on how it is
> implemented).
> 
> And I think it would be even simpler to do your idea of groups and to
> have aliases for different combinations of options. And it would let
> us keep 'on' and 'off' as backwards compatibility aliases.

hm, I think that's a very interesting idea. Being able to preserve backward
compatibility is certainly a big bonus (as long as the values keep doing the
exact same thing).

> Maybe if I tried implementing it, it wouldn't work at all. But, as it
> stands, I'm feeling nervous about making two of the most common GUCs
> in the world (log_connections and log_disconnections) less structured.

I think the patch was in a good shape but yeah that might be too late in
the release cycle for such a change... And even if we were to merge it that
would not be a user friendly thing to change it in 2 releases in a row (I'm
imagining 19 implementing your new "SET" idea).

> All that being said, this is hardly a project for March 3rd.

Fully agree.

> So, in
> service of moving 0002 forward, I think I want to walk back to an idea
> everyone else on this thread was keen on: making log_connections an
> enum with 'on', 'off', and 'timings'. This moves us in the direction
> of having more granular control in log_connections without breaking
> any existing users and paving the way for a project like what I
> outlined above. What do you think?

I do agree (see above as to why). That said, it means that 18 will not "solve"
the concerns about the volume of log_connections messages that you and Andres
shared up-thread.

But given that:

- 9afffcb833d3 and e48b19c5db31 are the most recent ones that added extra
messages (so added in pre-18)
- the new 'timings' option will help to prevent to add extra message in 18 (
if not desired)

Then it's not like we will add a lot of extra messages by default on 18.

So being on the safe side of things and postpone this to 19 seems a perfectly
reasonable thing to do.

Regards,

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