| Message ID | 20231207135647.24332-1-ddiss@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: scrub: improve Rate reporting for sub-second durations | expand |
On 2023/12/8 00:26, David Disseldorp wrote: > Scrubs which complete in under one second may carry a duration rounded > down to zero. This subsequently results in a bytes_per_sec value of > zero, which corresponds to the Rate metric output, causing intermittent > tests/btrfs/282 failures. > > This change ensures that Rate reflects any sub-second bytes processed. > Time left and ETA metrics are also affected by this change, in that they > increase to account for (sub-second) bytes_per_sec. > > Signed-off-by: David Disseldorp <ddiss@suse.de> Looks good to me. In the future we should also increase the resolution to proper time stamp level. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > cmds/scrub.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/cmds/scrub.c b/cmds/scrub.c > index 4a741355..72ea3b67 100644 > --- a/cmds/scrub.c > +++ b/cmds/scrub.c > @@ -152,7 +152,14 @@ static void print_scrub_summary(struct btrfs_scrub_progress *p, struct scrub_sta > time_t sec_eta; > > bytes_scrubbed = p->data_bytes_scrubbed + p->tree_bytes_scrubbed; > - if (s->duration > 0) > + /* > + * If duration is zero seconds (rounded down), then the Rate metric > + * should still reflect the amount of bytes that have been processed > + * in under a second. > + */ > + if (s->duration == 0) > + bytes_per_sec = bytes_scrubbed; > + else > bytes_per_sec = bytes_scrubbed / s->duration; > if (bytes_per_sec > 0) > sec_left = (bytes_total - bytes_scrubbed) / bytes_per_sec;
On Fri, Dec 08, 2023 at 12:56:47AM +1100, David Disseldorp wrote: > Scrubs which complete in under one second may carry a duration rounded > down to zero. This subsequently results in a bytes_per_sec value of > zero, which corresponds to the Rate metric output, causing intermittent > tests/btrfs/282 failures. > > This change ensures that Rate reflects any sub-second bytes processed. > Time left and ETA metrics are also affected by this change, in that they > increase to account for (sub-second) bytes_per_sec. > > Signed-off-by: David Disseldorp <ddiss@suse.de> Added to devel, thanks.
On Fri, Dec 08, 2023 at 06:33:25AM +1030, Qu Wenruo wrote: > > > On 2023/12/8 00:26, David Disseldorp wrote: > > Scrubs which complete in under one second may carry a duration rounded > > down to zero. This subsequently results in a bytes_per_sec value of > > zero, which corresponds to the Rate metric output, causing intermittent > > tests/btrfs/282 failures. > > > > This change ensures that Rate reflects any sub-second bytes processed. > > Time left and ETA metrics are also affected by this change, in that they > > increase to account for (sub-second) bytes_per_sec. > > > > Signed-off-by: David Disseldorp <ddiss@suse.de> > > Looks good to me. > > In the future we should also increase the resolution to proper time > stamp level. It seems possible, we have some of the timestamps as time_te which is in seconds but the end time measured in scrub_one_dev() uses gettimeofday() so we would have the sub-second precision. However I'm not sure we need this, most scrubs run for a long time, I'd be fine with the simple fix like in this patch.
diff --git a/cmds/scrub.c b/cmds/scrub.c index 4a741355..72ea3b67 100644 --- a/cmds/scrub.c +++ b/cmds/scrub.c @@ -152,7 +152,14 @@ static void print_scrub_summary(struct btrfs_scrub_progress *p, struct scrub_sta time_t sec_eta; bytes_scrubbed = p->data_bytes_scrubbed + p->tree_bytes_scrubbed; - if (s->duration > 0) + /* + * If duration is zero seconds (rounded down), then the Rate metric + * should still reflect the amount of bytes that have been processed + * in under a second. + */ + if (s->duration == 0) + bytes_per_sec = bytes_scrubbed; + else bytes_per_sec = bytes_scrubbed / s->duration; if (bytes_per_sec > 0) sec_left = (bytes_total - bytes_scrubbed) / bytes_per_sec;
Scrubs which complete in under one second may carry a duration rounded down to zero. This subsequently results in a bytes_per_sec value of zero, which corresponds to the Rate metric output, causing intermittent tests/btrfs/282 failures. This change ensures that Rate reflects any sub-second bytes processed. Time left and ETA metrics are also affected by this change, in that they increase to account for (sub-second) bytes_per_sec. Signed-off-by: David Disseldorp <ddiss@suse.de> --- cmds/scrub.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)