Re: pgbench logging broken by time logic changes - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: pgbench logging broken by time logic changes
Date
Msg-id alpine.DEB.2.22.394.2106170917240.2693553@pseudo
Whole thread Raw
In response to Re: pgbench logging broken by time logic changes  (Thomas Munro <thomas.munro@gmail.com>)
Responses Re: pgbench logging broken by time logic changes
List 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.

pgsql-hackers by date:

Previous
From: Zhihong Yu
Date:
Subject: Re: Skip partition tuple routing with constant partition key
Next
From: Amit Langote
Date:
Subject: Re: Skip partition tuple routing with constant partition key