From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, rulyox(at)gmail(dot)com, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Error on pgbench logs |
Date: | 2021-06-09 07:46:10 |
Message-ID: | alpine.DEB.2.22.394.2106090820330.3410176@pseudo |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello Michael,
>> The cause is that the time unit is changed to usec but the patch
>> forgot to convert agg_interval into the same unit in doLog. I tempted
>> to change it into pg_time_usec_t but it seems that it is better that
>> the unit is same with other similar variables like duration.
>
> As the option remains in seconds, I think that it is simpler to keep
> it as an int, and do the conversion where need be. It would be good
> to document that agg_interval is in seconds where the variable is
> defined.
>
> - while (agg->start_time + agg_interval <= now)
> + while (agg->start_time + agg_interval * 1000000 <= now)
>
> In need of a cast with (int64), no?
Yes, it would be better. In practice I would not expect the interval to be
large enough to trigger an overflow (maxint µs is about 36 minutes).
> The other things are "progress" and "duration". These look correctly
> handled to me.
Hmmm… What about tests?
I'm pretty sure that I wrote a test about time sensitive features with a 2
seconds run (-T, -P, maybe these aggregates as well), but the test needed
to be quite loose so as to pass on slow/heavy loaded hosts, and was
removed at some point on the ground that it was somehow imprecise.
I'm not sure whether it is worth to try again.
--
Fabien.
From | Date | Subject | |
---|---|---|---|
Next Message | Tatsuro Yamada | 2021-06-09 07:56:14 | Re: Duplicate history file? |
Previous Message | torikoshia | 2021-06-09 07:44:55 | Re: RFC: Logging plan of the running query |