On Tue, Mar 30, 2021 at 11:15:48PM +0000, Jacob Champion wrote:
> Rather than putting Postgres log data into the Perl logs, I rotate the
> logs exactly once at the beginning -- so that there's an
> old 001_ssltests_primary.log, and a new 001_ssltests_primary_1.log --
> and then every time we truncate the logfile, I shuffle the bits from
> the new logfile into the old one. So no one has to learn to find the
> log entries in a new place, we don't get an explosion of rotated logs,
> we don't lose the log data, we don't match incorrect portions of the
> logs, and we only pay the restart price once. This is wrapped into a
> small Perl module, LogCollector.
Hmm. I have dug today into that and I am really not convinced that
this is necessary, as a connection attempt combined with the output
sent to stderr gives you the stability needed. If we were to have
anything like that, perhaps a sub-class of PostgresNode would be
adapted instead, with an internal log integration.
After thinking about it, the new wording in config.sgml looks fine
as-is.
Anyway, I have not been able to convince myself that we need those
slowdowns and that many server restarts as there is no
reload-dependent timing here, and things have been stable on
everything I have tested (including a slow RPI). I have found a
couple of things that can be simplified in the tests:
- In src/test/authentication/, except for the trust method where there
is no auth ID, all the other tests wrap a like() if $res == 0, or
unlike() otherwise. I think that it is cleaner to make the matching
pattern an argument of test_role(), and adapt the tests to that.
- src/test/ldap/ can also embed a single logic within test_access().
- src/test/ssl/ is a different beast, but I think that there is more
refactoring possible here in parallel of the recent work I have sent
to have equivalents of test_connect_ok() and test_connect_fails() in
PostgresNode.pm. For now, I think that we should just live in this
set with a small routine able to check for pattern matches in the
logs.
Attached is an updated patch, with a couple of comments tweaks, the
reworked tests and an indentation done.
--
Michael