Re: [PATCH 1/1] nfs-iostat.py: Fixes several Divide by Zero errors

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

On 05/28/10 06:51 PM, Kevin Constantine wrote:
> On 05/28/2010 10:00 AM, Chuck Lever wrote:
>> On 05/27/10 08:50 PM, Kevin Constantine wrote:
>>> The first time through the code sample_time is set to 0.0 on line 588.
>>> Eventually we call display_iostats() and check if sample_time == 0. It
>>> does, so we set sample_time equal to the mount age on line 368. I'm
>>> seeing instances where the age of a mount is 0.
>>>
>>> cat /proc/self/mountstats | egrep "device|age"
>>> device fashome-n1:/vol/home/fahome mounted on /home/fahome with fstype
>>> nfs statvers=1.0
>>> age: 0
>>>
>>> All of our storage is automounted, so volumes are frequently getting
>>> unmounted and remounted.
>>
>> Makes sense. When I wrote the script, I wasn't using automounter at all.
>> It might be simpler to have one check for a zero age at 368.
>>
>
> My only concern with checking and just returning (which is certainly
> easier and a smaller change) is that we end up not printing anything for
> a volume that is technically mounted. I figured it was better to print
> the volume info and all zeroes than to not print anything.

Unfortunately not printing anything in this case might be considered 
"following the precedent" of Unix tools -- don't print anything in error 
cases, or when there is nothing to show.  I think that's the rule I 
followed for RPC procedures that have a zero op count.  But it would 
still be useful to see other relevant information for that mount, as you 
point out.

What would happen if, at line 368, you set sample_time to 1 if the 
mount's age is zero?

> I'll defer to your opinion on which of those two is best.
>
> -kevin
>
>>> On 05/27/2010 05:22 PM, Chuck Lever wrote:
>>>> On 05/27/2010 07:58 PM, Kevin Constantine wrote:
>>>>> There was no check to see if sample_time was zero before dividing by
>>>>> it.
>>>>
>>>> I haven't looked at this code in a very long time. Why was sample_time
>>>> zero? That seems wrong.
>>>>
>>>>> This was causing ZeroDivisionError's:
>>>>>