stat: fix duplicate timestamps in iops/latency/bandwidth logs #982
Conversation
add_log_sample() checks if the current time is greater than the last log time
plus the sampling window duration ("log_avg_msec"). If so it records the log
entry and sets the "last log time" to the current "last log time" plus
log_avg_msec. This is incorrect in cases where more than log_avg_msec has
elapsed between calls to add_log_sample(). The effect is that the very next
time add_log_sample() is called a new log entry will be recorded, possibly with
the same timestamp as the previous log entry.
This fix sets the last log time to the current time, rounded down to the nearest
multiple of log_avg_msec (e.g. if current time is 203 and log_avg_msec is 50 the
last log time will be set to 200, so the next log entry should be recorded at
time=250). This prevents duplicate log entries.
|
I don't know if this is sufficient... Don't we have to clamp all sample period starting points downwards including the first one? I'm also wondering when fio handles its own stats whether it accounts for periods where no sampling actually took place (although this isn't an issue for the log generation)... |
I'm not sure what you mean by this? |
|
I mean, shouldn't we align the very first sample downwards relative the the starting epoch? If the first second has nothing in it then that is somehow significant... |
ghost
mentioned this pull request
|
I believe this change along with the changes I have proposed in [https://github.com//pull/1114] have resolved the issues we've been seeing with the status interval creep. Could you also review PR# 1114 and comment on the changes? |
ghost
mentioned this pull request
|
Closing because this has been superseded by #1119 . |
add_log_sample() checks if the current time is greater than the last log time
plus the sampling window duration ("log_avg_msec"). If so it records the log
entry and sets the "last log time" to the current "last log time" plus
log_avg_msec. This is incorrect in cases where more than log_avg_msec has
elapsed between calls to add_log_sample(). The effect is that the very next
time add_log_sample() is called a new log entry will be recorded, possibly with
the same timestamp as the previous log entry.
This fix sets the last log time to the current time, rounded down to the nearest
multiple of log_avg_msec (e.g. if current time is 203 and log_avg_msec is 50 the
last log time will be set to 200, so the next log entry should be recorded at
time=250). This prevents duplicate log entries.
Fixes #971