From: Chuck Lever <chuck.lever@oracle.com>
To: Kevin Constantine <Kevin.Constantine@disneyanimation.com>
Cc: linux-nfs@vger.kernel.org, steved@redhat.com
Subject: Re: [PATCH 1/1] nfs-iostat.py: Fixes several Divide by Zero errors
Date: Fri, 28 May 2010 13:00:24 -0400 [thread overview]
Message-ID: <4BFFF6A8.2090103@oracle.com> (raw)
In-Reply-To: <4BFF1345.9070306@disney.com>
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.
> -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:
>>>
>>> Traceback (most recent call last):
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 628, in ?
>>> iostat_command(prog)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 610, in iostat_command
>>> print_iostat_summary(old_mountstats, mountstats, devices, sample_time,
>>> options)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 462, in print_iostat_summary
>>> stats[device].display_iostats(time, options.which)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 370, in display_iostats
>>> backlog = (float(self.__rpc_data['backlogutil']) / sends) / sample_time
>>> ZeroDivisionError: float division
>>>
>>> Traceback (most recent call last):
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 628, in ?
>>> iostat_command(prog)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 610, in iostat_command
>>> print_iostat_summary(old_mountstats, mountstats, devices, sample_time,
>>> options)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 462, in print_iostat_summary
>>> stats[device].display_iostats(time, options.which)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 380, in display_iostats
>>> print '%7.2f' % (sends / sample_time),
>>> ZeroDivisionError: float division
>>>
>>> Traceback (most recent call last):
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 630, in ?
>>> iostat_command(prog)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 612, in iostat_command
>>> print_iostat_summary(old_mountstats, mountstats, devices, sample_time,
>>> options)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 464, in print_iostat_summary
>>> stats[device].display_iostats(time, options.which)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 386, in display_iostats
>>> self.__print_rpc_op_stats('READ', sample_time)
>>> File
>>> "/home/fahome/kconstan/repos/nfs-utils.upstream/tools/nfs-iostat/nfs-iostat.py",
>>>
>>> line 350, in __print_rpc_op_stats
>>> print '\t\t%7.3f' % (ops / sample_time),
>>> ZeroDivisionError: float division
>>>
>>> Signed-off-by: Kevin Constantine<kevin.constantine@disneyanimation.com>
>>> ---
>>> tools/nfs-iostat/nfs-iostat.py | 20 +++++++++++++++-----
>>> 1 files changed, 15 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/nfs-iostat/nfs-iostat.py
>>> b/tools/nfs-iostat/nfs-iostat.py
>>> index 2d0b143..4d6b295 100644
>>> --- a/tools/nfs-iostat/nfs-iostat.py
>>> +++ b/tools/nfs-iostat/nfs-iostat.py
>>> @@ -342,13 +342,19 @@ class DeviceData:
>>> retrans_percent = 0.0
>>> rtt_per_op = 0.0
>>> exe_per_op = 0.0
>>> + if sample_time != 0:
>>> + ops_per_sample_time = ops / sample_time
>>> + kb_per_sample_time = kilobytes / sample_time
>>> + else:
>>> + ops_per_sample_time = 0.0
>>> + kb_per_sample_time = 0.0
>>>
>>> op += ':'
>>> print '%s' % op.lower().ljust(15),
>>> print ' ops/s\t\t kB/s\t\t kB/op\t\tretrans\t\tavg RTT (ms)\tavg exe
>>> (ms)'
>>>
>>> - print '\t\t%7.3f' % (ops / sample_time),
>>> - print '\t%7.3f' % (kilobytes / sample_time),
>>> + print '\t\t%7.3f' % (ops_per_sample_time),
>>> + print '\t%7.3f' % (kb_per_sample_time),
>>> print '\t%7.3f' % kb_per_op,
>>> print ' %7d (%3.1f%%)' % (retrans, retrans_percent),
>>> print '\t%7.3f' % rtt_per_op,
>>> @@ -358,7 +364,9 @@ class DeviceData:
>>> sends = float(self.__rpc_data['rpcsends'])
>>> if sample_time == 0:
>>> sample_time = float(self.__nfs_data['age'])
>>> - return (sends / sample_time)
>>> + return (sends / sample_time)
>>> + else:
>>> + return(0.0)
>>>
>>> def display_iostats(self, sample_time, which):
>>> """Display NFS and RPC stats in an iostat-like way
>>> @@ -366,10 +374,12 @@ class DeviceData:
>>> sends = float(self.__rpc_data['rpcsends'])
>>> if sample_time == 0:
>>> sample_time = float(self.__nfs_data['age'])
>>> - if sends != 0:
>>> + if sends != 0 and sample_time != 0:
>>> backlog = (float(self.__rpc_data['backlogutil']) / sends) / sample_time
>>> + sends_per_sample_time = sends / sample_time
>>> else:
>>> backlog = 0.0
>>> + sends_per_sample_time = 0.0
>>>
>>> print
>>> print '%s mounted on %s:' % \
>>> @@ -377,7 +387,7 @@ class DeviceData:
>>> print
>>>
>>> print ' op/s\t\trpc bklog'
>>> - print '%7.2f' % (sends / sample_time),
>>> + print '%7.2f' % (sends_per_sample_time),
>>> print '\t%7.2f' % backlog
>>>
>>> if which == 0:
>>
>>
next prev parent reply other threads:[~2010-05-28 17:02 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-27 23:58 [PATCH 1/1] nfs-iostat.py: Fixes several Divide by Zero errors Kevin Constantine
[not found] ` <1275004718-1802-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2010-05-28 0:22 ` Chuck Lever
2010-05-28 0:50 ` Kevin Constantine
2010-05-28 17:00 ` Chuck Lever [this message]
2010-05-28 22:51 ` Kevin Constantine
2010-06-01 15:37 ` Chuck Lever
2010-06-02 17:13 ` Kevin Constantine
2010-06-07 22:40 ` [PATCH 1/1] nfs-iostat.py: divide by zero with fresh mount Kevin Constantine
[not found] ` <1275950427-10200-1-git-send-email-kevin.constantine-FfNkGbSheRGpB8w63BLUukEOCMrvLtNR@public.gmane.org>
2010-06-07 23:12 ` Chuck Lever
2010-06-22 21:52 ` Steve Dickson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BFFF6A8.2090103@oracle.com \
--to=chuck.lever@oracle.com \
--cc=Kevin.Constantine@disneyanimation.com \
--cc=linux-nfs@vger.kernel.org \
--cc=steved@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.