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.
