Re: pgbench logging broken by time logic changes
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: Andrew Dunstan <andrew@dunslane.net>, Thomas Munro <thomas.munro@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Gregory Smith <gregsmithpgsql@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, david.christensen@crunchydata.com
Date: 2021-06-23T08:06:18Z
Lists: pgsql-hackers
On Wed, Jun 23, 2021 at 08:26:32AM +0200, Fabien COELHO wrote:
>> Could we make that shorter at 1s? That will shorten the duration of
>> the test run. It is easy to miss that this test is for
>> --aggregate-interval (aka the logAgg() path) without a comment.
>
> It is for -T, -P and --aggregate-interval. The units of all these is
> seconds, the minimum is 1, I put 2 so that It is pretty sure to get some
> output. We could try 1, but I'm less confident that an output is ensured in
> all cases on a very slow host which may decide to shorten the run before
> having shown a progress for instance.
Could it be possible to document the intention of the test and its
coverage then? With the current patch, one has to guess what's the
intention behind this case.
>> +# $nthreads threads, 2 seconds, but due to timing imprecision we might get
>> +# only 1 or as many as 3 progress reports per thread.
>> +check_pgbench_logs($bdir, '001_pgbench_log_1', $nthreads, 1, 3,
>> + qr{^\d{10,} \d{1,2} \d+ \d+ \d+ \d+ \d+ \d+ \d+ \d+ \d+$});
>> +
>> Now this one is good and actually stable thanks to the fact that we'd
>> get at least the final logs, and the main complain we got to discuss
>> about on this thread was the format of the logs.
>
> Yep, this test would have probably detected the epoch issue reported by
> Greg.
(Sorry I missed the use of throttle_delay that would generate 10
fields in the log file)
Hm.. Could it be possible to tighten a bit the regex used here then?
I was playing with it and it is not really picky in terms of patterns
allowed or rejected. The follow-up checks for check_pgbench_logs()
could be a bit more restrictive as well, but my regex-fu is bad.
>> I would say to give up on the first test, and keep the second.
>
> You mean remove the time check and keep the log check. I'd like to keep a
> time check, or at least have it in comment so that I can enable it simply.
I'd rather avoid tests that tend to fail on slow machines. There is a
flotilla in the buildfarm.
--
Michael
Commits
-
Fix pgbench timestamp bugs.
- 5614a0f78eaa 14.0 landed
- 0e39a608ed55 15.0 landed
-
Fix pattern matching logic for logs in TAP tests of pgbench
- 0efd2a1a66dd 11.13 landed
- 79ff96aa9d9a 12.8 landed
- 7a9eaf111ac0 13.4 landed
- c13585fe9e55 14.0 landed
-
pgbench: Improve time logic.
- 547f04e7348b 14.0 cited
-
pgbench: Synchronize client threads.
- aeb57af8e640 14.0 cited
-
pgbench: refactor handling of stats tracking
- b60376649600 9.6.0 cited