Re: Log connection establishment timings

On Tue, May 20, 2025 at 4:52 AM Peter Eisentraut <peter(at)eisentraut(dot)org>
wrote:

> log_connections has been changed from a Boolean parameter to a string
> one, but a number of places in the documentation and in various pieces
> of test code still use the old values. I think it would be better if
> they were adjusted to the new style.
>
> There are two places in doc/src/sgml/config.sgml where
> log_connections=yes is used as an example. This is a relatively
> prominent place, so it should not use deprecated values.
>
>
In earlier versions of my patch, I played around with replacing these
references in the docs. I ended up not doing it because I wasn't sure we
had consensus on deprecating the "on", "true", "yes" options and that we
would continue to support them indefinitely. Thinking about it now, by no
longer documenting "on" and "off", I was obviously deprecating them (not to
mention removing support for log_connections = "y", "ye", etc).

I'll write a patch to change these.

> In src/backend/tcop/postgres.c, there is a call
>
> SetConfigOption("log_connections", "true", context, source);
>
> that could be adjusted.
>

Do you think the debug option should be 'all' or a list of the options
covered by "true" (which is a subset of 'all')?

> Various uses in test code:
>
> src/interfaces/libpq/t/005_negotiate_encryption.pl
> src/test/authentication/t/001_password.pl
> src/test/authentication/t/003_peer.pl
> src/test/authentication/t/005_sspi.pl
> src/test/authentication/t/007_pre_auth.pl
> src/test/kerberos/t/001_auth.pl
> src/test/ldap/t/001_auth.pl
> src/test/ldap/t/002_bindpasswd.pl
> src/test/modules/ldap_password_func/t/001_mutated_bindpasswd.pl
> src/test/modules/oauth_validator/t/001_server.pl
> src/test/modules/oauth_validator/t/002_client.pl
> src/test/postmaster/t/002_connection_limits.pl
> src/test/postmaster/t/003_start_stop.pl
> src/test/recovery/t/013_crash_restart.pl
> src/test/recovery/t/022_crash_temp_files.pl
> src/test/recovery/t/032_relfilenode_reuse.pl
> src/test/recovery/t/037_invalid_database.pl
> src/test/ssl/t/SSL/Server.pm
> src/tools/ci/pg_ci_base.conf
>
> I suspect in some of these cases using one of the new more granular
> values would be appropriate? This could also serve as examples and
> testing of the parameter itself.

Yep, for these, some of them specifically parse and test output generated
by one or more of the log_connections options. In those cases, obviously
those options should be provided at a minimum. However, I assume, based on
how I use the logs from TAP tests, that when a test fails, folks use the
logging to understand more about what went wrong even when the log output
is not specifically parsed and tested against.
The reason I didn't change these was that I was unsure which of these tests
would want which options.

And I didn't want to risk increasing the volume of logging output by
replacing any of them with "all".

I can, of course, go through and make my best guess. Or I could just
replace them with the equivalent subset of options.

In the absence of clarity on this, I did add tests to 001_password.pl
specifically exercising and testing combinations of log_connections options.

- Melanie