Re: Log connection establishment timings

On Wed, Mar 5, 2025 at 10:46 AM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> As such, I wonder if my PGC_SET idea is not worth the effort and I
> should revise the earlier patch version which specified the stages but
> allow for on, off, true, yes, 1 for backwards compatibility and not
> include disconnections (so we don't remove the log_disconnections GUC
> this release).

Okay! Attached v10 does this. It turns log_connections into a string
but supports all the common values of the current log_connections
boolean.

So, you can specify
log_connections = authorized,authenticated
log_connections = 'all'
log_connections = on
etc
And only 'all' has to be quoted when not specified as a startup option
(which is true for other enum gucs that have 'all' as an option
because ALL is a Postgres keyword).

I've left log_disconnections alone.

I still need to comprehensively manually test it myself (so there
might still be bugs) and write a bit of test coverage for the new
values. The docs can probably also use some more massaging.

But, this is the first version of the patch where I am happy with the
interface and with the code. It isn't a breaking change (since on,
off, true, false, etc still work), so I think it is still okay to
target 18.

I went back through the thread and tried to be sure I addressed all of
the outstanding review comments on previous versions (since I
resurrected some previously dead patch features), but I can't be sure
I caught it all. Please don't hesitate to call out anything I missed.

I didn't enumerate all the possibilities in postgresql.conf.sample as
Fujii-san recommended in a previous review, because I didn't see many
other enum GUCs doing that. I am open to changing that if folks feel
strongly, though.

In 0002, because we take the timings for all wal sender and client
connection backends, now the new log message emitted in 0002 is more
like a stage, so I've named that option "ready_for_query". As such, I
wonder if I should change the output message to reflect that. What do
you think?

- Melanie