Re: Log connection establishment timings - Mailing list pgsql-hackers
| From | Melanie Plageman |
|---|---|
| Subject | Re: Log connection establishment timings |
| Date | |
| Msg-id | CAAKRu_bs7MP6_q5D7piX9XCZhwbdxV39Mtb+5Qgcqptx7BStvA@mail.gmail.com Whole thread Raw |
| In response to | Re: Log connection establishment timings (Guillaume Lelarge <guillaume@lelarge.info>) |
| List | pgsql-hackers |
On Tue, May 20, 2025 at 4:52 AM Peter Eisentraut <peter@eisentraut.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
pgsql-hackers by date: