Re: [PATCH 1/2] nfs-iostat.py: Print Data Cache Statistics

[Chuck Lever <chuck.lever@oracle.com>]

On Apr 21, 2009, at 1:54 PM, Kevin Constantine wrote:
> Chuck Lever wrote:
>> On Apr 20, 2009, at 10:03 PM, Kevin Constantine wrote:
>>> add --data flag to print data cache statistics
>>> print read cache stats from __print_data_cache_stats
>> It's been a while since I wrote this code... comments below.
>>> Signed-off-by: Kevin Constantine <kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org 
>>> >
>>> ---
>>> tools/nfs-iostat/nfs-iostat.py |   26 ++++++++++++++++++++------
>>> 1 files changed, 20 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs- 
>>> iostat.py
>>> index 9626d42..d331a72 100644
>>> --- a/tools/nfs-iostat/nfs-iostat.py
>>> +++ b/tools/nfs-iostat/nfs-iostat.py
>>> @@ -220,14 +220,20 @@ class DeviceData:
>>>        """Print the data cache hit rate
>>>        """
>>>        nfs_stats = self.__nfs_data
>>> -        app_bytes_read = float(nfs_stats['normalreadbytes'])
>>> +        app_bytes_read = float(nfs_stats['normalreadbytes'] +  
>>> nfs_stats['directreadbytes'])
>>>        if app_bytes_read != 0:
>>> -            client_bytes_read =  
>>> float(nfs_stats['serverreadbytes'] - nfs_stats['directreadbytes'])
>>> -            ratio = ((app_bytes_read - client_bytes_read) *  
>>> 100) / app_bytes_read
>>> -
>>> +            read_bytes_from_server =  
>>> float(nfs_stats['serverreadbytes'])
>>> +            directio_bytes_from_server =  
>>> float(nfs_stats['directreadbytes'])
>>> +            standard_read_bytes_from_server =  
>>> read_bytes_from_server - directio_bytes_from_server
>>> +            cached_read_bytes = float(app_bytes_read -  
>>> read_bytes_from_server)
>> I'm not sure why you are reversing the sense of this computation.   
>> "directreadbytes" is the count of bytes that applications read with  
>> O_DIRECT.  These are never cached, but they are counted in  
>> "serverreadbytes", so my logic subtracts them before computing the  
>> hit ratio.  Was there some accounting problem you found?  (If there  
>> was, it's worth stating that explicitly in the patch description).
>
> Originally you had app_bytes_read = normalreadbytes, but that's not  
> entirely true, since an app might be reading with O_DIRECT,  
> therefore app_bytes_read should probably be normal + direct.
>
> client_bytes_read seemed ambiguous to me, is that bytes being read  
> by the host client from the server, bytes being read by the nfs  
> client process?  read_bytes_from_server seemed more logical, but in  
> the diff block below, I've removed that altogether, and am just left  
> with cached_read_bytes.

I'd rather not change the way the ratio is computed, unless you see a  
loss of precision.  The way I had it minimizes the possibility of an  
overflow during the computation.  If you want to rename the variables  
to clarify it that's OK, I guess.

>>> +            ratio = (cached_read_bytes / app_bytes_read) * 100
>> The other functions use (x * 100) / y -- I seem to recall there are  
>> rounding errors if you do it the way you've done it here, but I  
>> don't remember exactly what the issue was.  But you are changing  
>> this one so it is different than the all the other similar  
>> computations.  Can you state your reason?
>
> since cached_read_bytes = app_bytes_read - client_bytes_read, i used  
> (cached_read_bytes / app_bytes_read) * 100, but that is the same as  
> (cached_read_bytes * 100) / app_bytes_read if you'd rather see that.

The latter usually gives us better precision in the calculated result.

> new diff block below (if it all checks out, i'll send the whole  
> patch, unless you'd rather just see that.)
>
> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs- 
> iostat.py
> index 9626d42..7dae659 100644
> --- a/tools/nfs-iostat/nfs-iostat.py
> +++ b/tools/nfs-iostat/nfs-iostat.py
> @@ -220,14 +220,17 @@ class DeviceData:
>         """Print the data cache hit rate
>         """
>         nfs_stats = self.__nfs_data
> -        app_bytes_read = float(nfs_stats['normalreadbytes'])
> +        app_bytes_read = float(nfs_stats['normalreadbytes'] +  
> nfs_stats['directreadbytes'])
>         if app_bytes_read != 0:
> -            client_bytes_read = float(nfs_stats['serverreadbytes']  
> - nfs_stats['directreadbytes'])
> -            ratio = ((app_bytes_read - client_bytes_read) * 100) /  
> app_bytes_read
> -
> +            cached_read_bytes = float(app_bytes_read -  
> float(nfs_stats['serverreadbytes']))
> +            ratio = (cached_read_bytes * 100) / app_bytes_read
>             print
> -            print 'app bytes: %f  client bytes %f' %  
> (app_bytes_read, client_bytes_read)
> -            print 'Data cache hit ratio: %4.2f%%' % ratio
> +            print '%10s %15s %15s %15s %7s' % ("Data Read:", "From  
> Server", "From Cache", "Total", "Hit %")
> +            print '%10s %13.4fMB %13.4fMB %13.4fMB %6.2f%%' % ("", \
> +                                                
> float(nfs_stats['serverreadbytes']) / 1024.0 / 1024.0, \
> +                                               cached_read_bytes /  
> 1024.0 / 1024.0, \
> +                                               app_bytes_read /  
> 1024.0 / 1024.0, \
> +                                               ratio)
>
>     def __print_attr_cache_stats(self, sample_time):
>         """Print attribute cache efficiency stats

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com