From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Gregory Smith <gregsmithpgsql@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: david.christensen@crunchydata.com
Subject: Re: pgbench logging broken by time logic changes
Date: Thu, 17 Jun 2021 09:24:37 +0200 (CEST)
Message-ID: <alpine.DEB.2.22.394.2106170917240.2693553@pseudo> (raw)
In-Reply-To: <CA+hUKG+jhatozihmqy4=Ufd0OHwZadEd_vg-_+26hE3f4C3TgQ@mail.gmail.com>
References: <CAHLJuCW_8Vpcr0=t6O_gozrg3wXXWXZXDioYJd3NhvKriqgpfQ@mail.gmail.com>
<CA+hUKG+jhatozihmqy4=Ufd0OHwZadEd_vg-_+26hE3f4C3TgQ@mail.gmail.com>
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.
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: coelho@cri.ensmp.fr, thomas.munro@gmail.com, gregsmithpgsql@gmail.com, david.christensen@crunchydata.com
Subject: Re: pgbench logging broken by time logic changes
In-Reply-To: <alpine.DEB.2.22.394.2106170917240.2693553@pseudo>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox