Re: pgbench logging broken by time logic changes
Fabien COELHO <coelho@cri.ensmp.fr>
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Gregory Smith <gregsmithpgsql@gmail.com>,
PostgreSQL-development <pgsql-hackers@postgresql.org>,
david.christensen@crunchydata.com
Date: 2021-06-17T07:24:37Z
Lists: pgsql-hackers
Hello Thomas, >> Before I could get to startup timing I noticed the pgbench logging >> output was broken via commit 547f04e7 "Improve time logic": >> https://www.postgresql.org/message-id/E1lJqpF-00064e-C6%40gemulon.postgresql.org > > It does suck that we broke the logging and that it took 3 months for > anyone to notice and report it to the list. Indeed. Well, it also demonstrates that beta are useful. > Seems like it should be straightforward to fix, though, with fixes > already proposed (though I haven't studied them yet, will do). I think that fixing logging is simple enough, thus a revert is not necessary. >> I have a lot of community oriented work backed up behind this right >> now, so I'm gonna be really honest. This time rework commit in its >> current form makes me uncomfortable at this point in the release >> schedule. The commit has already fought through two rounds of platform >> specific bug fixes. But since the buildfarm doesn't test the logging >> feature, that whole process is suspect. > > It's true that this work produced a few rounds of small portability > follow-ups: c427de42 (work around strange hacks elsewhere in the tree > for AIX), 68b34b23 (missing calling convention specifier on Windows), > and de91c3b9 (adjust pthread missing-function code for threadless > builds). These were problems that didn't show up on developer or CI > systems (including threadless and Windows), and IMHO are typical sorts > of problems you expect to have to work through when stuff hits the > build farm, especially when using new system interfaces. So I don't > think any of that, on its own, supports reverting anything here. Yep, the buildfarm is here to catch portability issues, and it does its job:-) There is no doubt that logging is has been broken because of lack of tests in this area, shame on us. I think it is easy to fix. > [...] For that reason, I'm not super attached to that new pg_time_usec_t > stuff at all, and wouldn't be sad if we reverted that piece. Well, I was sooo happy to get rid of INSTR_TIME ugly and inefficient macros in pgbench… so anything looks better to me. Note that Michaël is having a look at fixing pgbench logging issues. -- Fabien.
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