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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).