Libguestfs - Dec 2019 - Re: [PATCH nbdkit v2 3/3] filters: stats: Add flush stats

I have pushed some parts of these patches in order to reduce the delta
between your patches and upstream.  However still some problems with
the series:

Patch 1: Same problem with scale as discussed before.

Patch 2: At least the documentation needs to be updated since it no
longer matches what is printed.  The idea of collecting the time taken
in each operation is good on its own, so I pushed that part of it
along with small const-correctness and whitespace fixes:

 
https://github.com/libguestfs/nbdkit/commit/f280530d7d042d5e8f100125ab0618515de52142

Why don't we show the total time and time / operation on each line of
output (ie. per operation), instead of synthesizing the total by
adding up reads and writes?

Patch 3: The idea of collecting flush stats is good so I pushed that
part, as well as updating the documentation:

 
https://github.com/libguestfs/nbdkit/commit/8adf601835aee3779e278e13cae0489231960aee

I omitted the "if (st->bytes > 0)" part of the patch so this
shows "0
bytes" which is of course wrong but we can fix that later.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Mon, Dec 2, 2019 at 12:28 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
>
>
> I have pushed some parts of these patches in order to reduce the delta
> between your patches and upstream.  However still some problems with
> the series:
>
> Patch 1: Same problem with scale as discussed before.
I addressed this here:
https://www.redhat.com/archives/libguestfs/2019-December/msg00011.html
> Patch 2: At least the documentation needs to be updated since it no
> longer matches what is printed.  The idea of collecting the time taken
> in each operation is good on its own, so I pushed that part of it
> along with small const-correctness and whitespace fixes:
>
>  
https://github.com/libguestfs/nbdkit/commit/f280530d7d042d5e8f100125ab0618515de52142
Thanks for pushing it.
> Why don't we show the total time and time / operation on each line of
> output (ie. per operation), instead of synthesizing the total by
> adding up reads and writes?
I think that synthesizing totals give better view on total application
throughput, and add
information that was not available before, like total number of ops.
Showing two rate
values per operation looks confusing to me.

But how about this:

----------------------------------------------
total: 2299 ops, 2.172 s, 6.00 GiB, 2.76 GiB/s
       520.73 MiB/s write, 2.23 GiB/s zero
-----------------------------------------------
write: 1271 ops, 0.356 s, 1.13 GiB, 3.19 GiB/s
zero: 1027 ops, 0.012 s, 4.86 GiB, 405.00 GiB/s
extents: 1 ops, 0.000 s, 2.00 GiB, 485.29 GiB/s
flush: 2 ops, 1.252 s
> Patch 3: The idea of collecting flush stats is good so I pushed that
> part, as well as updating the documentation:
>
>  
https://github.com/libguestfs/nbdkit/commit/8adf601835aee3779e278e13cae0489231960aee
Nice, I missed the pod file.

Nir

On Wed, Dec 04, 2019 at 01:45:54AM +0200, Nir Soffer
wrote:
> On Mon, Dec 2, 2019 at 12:28 PM Richard W.M. Jones
<rjones@redhat.com> wrote:
> >
> >
> > I have pushed some parts of these patches in order to reduce the delta
> > between your patches and upstream.  However still some problems with
> > the series:
> >
> > Patch 1: Same problem with scale as discussed before.
> 
> I addressed this here:
> https://www.redhat.com/archives/libguestfs/2019-December/msg00011.html
> 
> > Patch 2: At least the documentation needs to be updated since it no
> > longer matches what is printed.  The idea of collecting the time taken
> > in each operation is good on its own, so I pushed that part of it
> > along with small const-correctness and whitespace fixes:
> >
> >  
https://github.com/libguestfs/nbdkit/commit/f280530d7d042d5e8f100125ab0618515de52142
> 
> Thanks for pushing it.
> 
> > Why don't we show the total time and time / operation on each line
of
> > output (ie. per operation), instead of synthesizing the total by
> > adding up reads and writes?
> 
> I think that synthesizing totals give better view on total application
> throughput, and add
> information that was not available before, like total number of ops.
> Showing two rate
> values per operation looks confusing to me.
> 
> But how about this:
> 
> ----------------------------------------------
> total: 2299 ops, 2.172 s, 6.00 GiB, 2.76 GiB/s
>        520.73 MiB/s write, 2.23 GiB/s zero
> -----------------------------------------------
> write: 1271 ops, 0.356 s, 1.13 GiB, 3.19 GiB/s
> zero: 1027 ops, 0.012 s, 4.86 GiB, 405.00 GiB/s
> extents: 1 ops, 0.000 s, 2.00 GiB, 485.29 GiB/s
> flush: 2 ops, 1.252 s
Yes, better than before.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW