commit 062577877bd3f1d8da1edeb889a09d380da83364
Author: Lans Carstensen <[email protected]>
Date: Tue Aug 25 21:53:54 2009 -0700
Adds --sort option to display mount point stats sorted by ops/s
Adds -<n> option to only display stats for first <n> mount points
E.g. the use of "--sort -1" should be useful in seeing stats for
only the mountpoint with the highest ops/s.
diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
index 9f3b3eb..5df9bfb 100644
--- a/tools/nfs-iostat/nfs-iostat.py
+++ b/tools/nfs-iostat/nfs-iostat.py
@@ -20,7 +20,7 @@ along with this program; if not, write to the Free
Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
"""
-import sys, os, time
+import sys, os, time, re
Iostats_version = '0.3'
@@ -353,6 +353,12 @@ class DeviceData:
print '\t%7.3f' % rtt_per_op,
print '\t%7.3f' % exe_per_op
+ def ops(self, sample_time):
+ sends = float(self.__rpc_data['rpcsends'])
+ if sample_time == 0:
+ sample_time = float(self.__nfs_data['age'])
+ return (sends / sample_time)
+
def display_iostats(self, sample_time, which):
"""Display NFS and RPC stats in an iostat-like way
"""
@@ -421,6 +427,11 @@ def print_iostat_help(name):
print ' If one or more <mount point> names are specified,
statistics for only these'
print ' mount points will be displayed. Otherwise, all NFS mount
points on the'
print ' client are listed.'
+ print
+ print ' You can also specify "--sort" to sort the NFS mount points
by ops/second,'
+ print ' and specify a number of mount points to return with -<num>,
e.g. -1.'
+ print ' For example, use of "--sort -1" will iterate only showing
you the stats'
+ print ' for the mount point with the highest ops/second.'
def parse_stats_file(filename):
"""pop the contents of a mountstats file into a dictionary,
@@ -446,7 +457,10 @@ def parse_stats_file(filename):
return ms_dict
-def print_iostat_summary(old, new, devices, time, ac):
+def print_iostat_summary(old, new, devices, time, ac, sortbyops,
entrycount):
+ diff_stats = {}
+ count = 1
+
if old:
# Trim device list to only include intersection of old and new
data,
# this addresses umounts due to autofs mountpoints
@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices, time, ac):
devicelist = devices
for device in devicelist:
+ count += 1
stats = DeviceData()
stats.parse_stats(new[device])
if not old:
stats.display_iostats(time, ac)
+ if (count>entrycount):
+ return
else:
old_stats = DeviceData()
old_stats.parse_stats(old[device])
- diff_stats = stats.compare_iostats(old_stats)
- diff_stats.display_iostats(time, ac)
+ diff_stats[device] = stats.compare_iostats(old_stats)
+ if not sortbyops:
+ diff_stats[device].display_iostats(time, ac)
+ if (count>entrycount):
+ return
+
+ if old and sortbyops:
+ # We now have compared data and can print a comparison
+ # ordered by mountpoint ops per second
+ count = 1
+
+ devicelist.sort(key=lambda x: diff_stats[x].ops(time),
reverse=True)
+
+ for device in devicelist:
+ count += 1
+ diff_stats[device].display_iostats(time, ac)
+ if (count>entrycount):
+ return
def list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate or
@@ -497,6 +530,8 @@ def iostat_command(name):
which = 0
interval_seen = False
count_seen = False
+ sortbyops = False
+ entrycount = sys.maxint
for arg in sys.argv:
if arg in ['-h', '--help', 'help', 'usage']:
@@ -507,6 +542,19 @@ def iostat_command(name):
print '%s version %s' % (name, Iostats_version)
return
+ if arg in ['-s', '--sort', 'sort']:
+ sortbyops = True
+ # sorted display infers a loop, default to 1 second
+ if not interval_seen:
+ interval = 1
+ interval_seen = True
+ continue
+
+ stop_re = re.compile('-[0-9]+')
+ if stop_re.match(arg):
+ entrycount = int(arg.lstrip('-'))
+ continue
+
if arg in ['-a', '--attr']:
which = 1
continue
@@ -546,12 +594,12 @@ def iostat_command(name):
sample_time = 0.0
if not interval_seen:
- print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
+ print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which, sortbyops, entrycount)
return
if count_seen:
while count != 0:
- print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
+ print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which, sortbyops, entrycount)
old_mountstats = mountstats
time.sleep(interval)
sample_time = interval
@@ -562,7 +610,7 @@ def iostat_command(name):
count -= 1
else:
while True:
- print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which)
+ print_iostat_summary(old_mountstats, mountstats, devices,
sample_time, which, sortbyops, entrycount)
old_mountstats = mountstats
time.sleep(interval)
sample_time = interval
Greetings! Comments inline. Chuck Lever wrote:
> Hi Lans-
>
> I was thinking about this feature a little more. Would it be
> interesting to allow sorting by other metrics besides ops/s ? That's
> another feature that the "top" command has that this new option doesn't.
>
> I also don't like the bare integer options. What if, at some point, we
> want "-4" and "-6" for IPv6 support (not that we do, but what if)?
> Plus, having the bare integers doesn't give any clues about what the
> number means.
>
> In which case I propose:
>
> --sort=ops --list=<n>
>
> which provides more flexibility and helps make the options more
> self-documenting.
Again, thanks for looking this over.
I had thought about the sorting a little bit, but decided that every
case we cared about devolved to sorting by ops/s. We'd use a tool like
this in an operational group where they've lost control of a particular
rogue workload in the mix of thousands of other workloads, and the
operational group is trying to figure out what workload is generating a
disproportionate amount of traffic (ops/s). If someone wanted to extend
the implementation further where leaving "--sort" unadorned sorts by ops
and using "--sort=readdirplus" or somesuch, I'd be all for it - but it
didn't seem worth the work for our use. So I'd encourage that to be
adopted as-is and if someone wants to put the extra effort into it I'm
all for it.
For the bare integer options I was thinking in terms of the existing
"tail" and "head" implementations - that's what made the most sense to
me and the couple of others looking over this here. Changing it to
"--list=<n>" is fine if that suites you and others more. I can't quite
fathom going after IPv4 or IPv6 stats with this, as
/proc/self/mountstats a) doesn't expose any relevant data that would be
IPv4/IPv6 oriented, and b) really never would as that's at an
implementation layer below NFS ops. So the existing patch makes most
sense to me, but if you and others want it changed I'd be fine w/ doing it.
> On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
>
>> commit 062577877bd3f1d8da1edeb889a09d380da83364
>> Author: Lans Carstensen <[email protected]>
>> Date: Tue Aug 25 21:53:54 2009 -0700
>>
>> Adds --sort option to display mount point stats sorted by ops/s
>> Adds -<n> option to only display stats for first <n> mount points
>> E.g. the use of "--sort -1" should be useful in seeing stats for
>> only the mountpoint with the highest ops/s.
>>
>> diff --git a/tools/nfs-iostat/nfs-iostat.py
>> b/tools/nfs-iostat/nfs-iostat.py
>> index 9f3b3eb..5df9bfb 100644
>> --- a/tools/nfs-iostat/nfs-iostat.py
>> +++ b/tools/nfs-iostat/nfs-iostat.py
>> @@ -20,7 +20,7 @@ along with this program; if not, write to the Free
>> Software
>> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>> """
>>
>> -import sys, os, time
>> +import sys, os, time, re
>>
>> Iostats_version = '0.3'
>>
>> @@ -353,6 +353,12 @@ class DeviceData:
>> print '\t%7.3f' % rtt_per_op,
>> print '\t%7.3f' % exe_per_op
>>
>> + def ops(self, sample_time):
>> + sends = float(self.__rpc_data['rpcsends'])
>> + if sample_time == 0:
>> + sample_time = float(self.__nfs_data['age'])
>> + return (sends / sample_time)
>> +
>> def display_iostats(self, sample_time, which):
>> """Display NFS and RPC stats in an iostat-like way
>> """
>> @@ -421,6 +427,11 @@ def print_iostat_help(name):
>> print ' If one or more <mount point> names are specified,
>> statistics for only these'
>> print ' mount points will be displayed. Otherwise, all NFS mount
>> points on the'
>> print ' client are listed.'
>> + print
>> + print ' You can also specify "--sort" to sort the NFS mount
>> points by ops/second,'
>> + print ' and specify a number of mount points to return with
>> -<num>, e.g. -1.'
>> + print ' For example, use of "--sort -1" will iterate only showing
>> you the stats'
>> + print ' for the mount point with the highest ops/second.'
>>
>> def parse_stats_file(filename):
>> """pop the contents of a mountstats file into a dictionary,
>> @@ -446,7 +457,10 @@ def parse_stats_file(filename):
>>
>> return ms_dict
>>
>> -def print_iostat_summary(old, new, devices, time, ac):
>> +def print_iostat_summary(old, new, devices, time, ac, sortbyops,
>> entrycount):
>> + diff_stats = {}
>> + count = 1
>> +
>> if old:
>> # Trim device list to only include intersection of old and new
>> data,
>> # this addresses umounts due to autofs mountpoints
>> @@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,
>> time, ac):
>> devicelist = devices
>>
>> for device in devicelist:
>> + count += 1
>> stats = DeviceData()
>> stats.parse_stats(new[device])
>> if not old:
>> stats.display_iostats(time, ac)
>> + if (count>entrycount):
>> + return
>> else:
>> old_stats = DeviceData()
>> old_stats.parse_stats(old[device])
>> - diff_stats = stats.compare_iostats(old_stats)
>> - diff_stats.display_iostats(time, ac)
>> + diff_stats[device] = stats.compare_iostats(old_stats)
>> + if not sortbyops:
>> + diff_stats[device].display_iostats(time, ac)
>> + if (count>entrycount):
>> + return
>> +
>> + if old and sortbyops:
>> + # We now have compared data and can print a comparison
>> + # ordered by mountpoint ops per second
>> + count = 1
>> +
>> + devicelist.sort(key=lambda x: diff_stats[x].ops(time),
>> reverse=True)
>> +
>> + for device in devicelist:
>> + count += 1
>> + diff_stats[device].display_iostats(time, ac)
>> + if (count>entrycount):
>> + return
>>
>> def list_nfs_mounts(givenlist, mountstats):
>> """return a list of NFS mounts given a list to validate or
>> @@ -497,6 +530,8 @@ def iostat_command(name):
>> which = 0
>> interval_seen = False
>> count_seen = False
>> + sortbyops = False
>> + entrycount = sys.maxint
>>
>> for arg in sys.argv:
>> if arg in ['-h', '--help', 'help', 'usage']:
>> @@ -507,6 +542,19 @@ def iostat_command(name):
>> print '%s version %s' % (name, Iostats_version)
>> return
>>
>> + if arg in ['-s', '--sort', 'sort']:
>> + sortbyops = True
>> + # sorted display infers a loop, default to 1 second
>> + if not interval_seen:
>> + interval = 1
>> + interval_seen = True
>> + continue
>> +
>> + stop_re = re.compile('-[0-9]+')
>> + if stop_re.match(arg):
>> + entrycount = int(arg.lstrip('-'))
>> + continue
>> +
>> if arg in ['-a', '--attr']:
>> which = 1
>> continue
>> @@ -546,12 +594,12 @@ def iostat_command(name):
>> sample_time = 0.0
>>
>> if not interval_seen:
>> - print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which)
>> + print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which, sortbyops, entrycount)
>> return
>>
>> if count_seen:
>> while count != 0:
>> - print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which)
>> + print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which, sortbyops, entrycount)
>> old_mountstats = mountstats
>> time.sleep(interval)
>> sample_time = interval
>> @@ -562,7 +610,7 @@ def iostat_command(name):
>> count -= 1
>> else:
>> while True:
>> - print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which)
>> + print_iostat_summary(old_mountstats, mountstats, devices,
>> sample_time, which, sortbyops, entrycount)
>> old_mountstats = mountstats
>> time.sleep(interval)
>> sample_time = interval
>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com
>
>
>
On Aug 26, 2009, at 12:57 PM, Trond Myklebust wrote:
> On Wed, 2009-08-26 at 11:47 -0400, Chuck Lever wrote:
>> On Aug 26, 2009, at 10:53 AM, Lans Carstensen wrote:
>>
>>> Greetings! Comments inline. Chuck Lever wrote:
>>>> Hi Lans-
>>>> I was thinking about this feature a little more. Would it be
>>>> interesting to allow sorting by other metrics besides ops/s ?
>>>> That's another feature that the "top" command has that this new
>>>> option doesn't.
>>>> I also don't like the bare integer options. What if, at some
>>>> point, we want "-4" and "-6" for IPv6 support (not that we do, but
>>>> what if)? Plus, having the bare integers doesn't give any clues
>>>> about what the number means.
>>>> In which case I propose:
>>>> --sort=ops --list=<n>
>>>> which provides more flexibility and helps make the options more
>>>> self-documenting.
>>>
>>> Again, thanks for looking this over.
>>>
>>> I had thought about the sorting a little bit, but decided that every
>>> case we cared about devolved to sorting by ops/s. We'd use a tool
>>> like this in an operational group where they've lost control of a
>>> particular rogue workload in the mix of thousands of other
>>> workloads, and the operational group is trying to figure out what
>>> workload is generating a disproportionate amount of traffic (ops/
>>> s). If someone wanted to extend the implementation further where
>>> leaving "--sort" unadorned sorts by ops and using "--
>>> sort=readdirplus" or somesuch, I'd be all for it - but it didn't
>>> seem worth the work for our use. So I'd encourage that to be
>>> adopted as-is and if someone wants to put the extra effort into it
>>> I'm all for it.
>>
>> Well, the problem is once we choose a command line option, it's hard
>> to change it later. I think having two choices, like --sort=ops
>> and --
>> sort=def[ault] would be fine for now, and give us room to grow later.
>
> There is precedent for having both an option "--sort" with no
> arguments,
> and "--sort=". See for instance 'ls --color'/'ls --color=WHEN'.
>
> We could therefore just add '--sort' for now, and then add a --
> sort=KEY
> type option when someone decides they need it.
If we are confident that having a "--sort" today will not limit
options later, then let's go for it.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Wed, 2009-08-26 at 11:47 -0400, Chuck Lever wrote:
> On Aug 26, 2009, at 10:53 AM, Lans Carstensen wrote:
>
> > Greetings! Comments inline. Chuck Lever wrote:
> >> Hi Lans-
> >> I was thinking about this feature a little more. Would it be
> >> interesting to allow sorting by other metrics besides ops/s ?
> >> That's another feature that the "top" command has that this new
> >> option doesn't.
> >> I also don't like the bare integer options. What if, at some
> >> point, we want "-4" and "-6" for IPv6 support (not that we do, but
> >> what if)? Plus, having the bare integers doesn't give any clues
> >> about what the number means.
> >> In which case I propose:
> >> --sort=ops --list=<n>
> >> which provides more flexibility and helps make the options more
> >> self-documenting.
> >
> > Again, thanks for looking this over.
> >
> > I had thought about the sorting a little bit, but decided that every
> > case we cared about devolved to sorting by ops/s. We'd use a tool
> > like this in an operational group where they've lost control of a
> > particular rogue workload in the mix of thousands of other
> > workloads, and the operational group is trying to figure out what
> > workload is generating a disproportionate amount of traffic (ops/
> > s). If someone wanted to extend the implementation further where
> > leaving "--sort" unadorned sorts by ops and using "--
> > sort=readdirplus" or somesuch, I'd be all for it - but it didn't
> > seem worth the work for our use. So I'd encourage that to be
> > adopted as-is and if someone wants to put the extra effort into it
> > I'm all for it.
>
> Well, the problem is once we choose a command line option, it's hard
> to change it later. I think having two choices, like --sort=ops and --
> sort=def[ault] would be fine for now, and give us room to grow later.
There is precedent for having both an option "--sort" with no arguments,
and "--sort=". See for instance 'ls --color'/'ls --color=WHEN'.
We could therefore just add '--sort' for now, and then add a --sort=KEY
type option when someone decides they need it.
Cheers
Trond
Hi Lans-
I was thinking about this feature a little more. Would it be
interesting to allow sorting by other metrics besides ops/s ? That's
another feature that the "top" command has that this new option doesn't.
I also don't like the bare integer options. What if, at some point,
we want "-4" and "-6" for IPv6 support (not that we do, but what if)?
Plus, having the bare integers doesn't give any clues about what the
number means.
In which case I propose:
--sort=ops --list=<n>
which provides more flexibility and helps make the options more self-
documenting.
On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
> commit 062577877bd3f1d8da1edeb889a09d380da83364
> Author: Lans Carstensen <[email protected]>
> Date: Tue Aug 25 21:53:54 2009 -0700
>
> Adds --sort option to display mount point stats sorted by ops/s
> Adds -<n> option to only display stats for first <n> mount points
> E.g. the use of "--sort -1" should be useful in seeing stats for
> only the mountpoint with the highest ops/s.
>
> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-
> iostat.py
> index 9f3b3eb..5df9bfb 100644
> --- a/tools/nfs-iostat/nfs-iostat.py
> +++ b/tools/nfs-iostat/nfs-iostat.py
> @@ -20,7 +20,7 @@ along with this program; if not, write to the Free
> Software
> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> USA
> """
>
> -import sys, os, time
> +import sys, os, time, re
>
> Iostats_version = '0.3'
>
> @@ -353,6 +353,12 @@ class DeviceData:
> print '\t%7.3f' % rtt_per_op,
> print '\t%7.3f' % exe_per_op
>
> + def ops(self, sample_time):
> + sends = float(self.__rpc_data['rpcsends'])
> + if sample_time == 0:
> + sample_time = float(self.__nfs_data['age'])
> + return (sends / sample_time)
> +
> def display_iostats(self, sample_time, which):
> """Display NFS and RPC stats in an iostat-like way
> """
> @@ -421,6 +427,11 @@ def print_iostat_help(name):
> print ' If one or more <mount point> names are specified,
> statistics for only these'
> print ' mount points will be displayed. Otherwise, all NFS
> mount points on the'
> print ' client are listed.'
> + print
> + print ' You can also specify "--sort" to sort the NFS mount
> points by ops/second,'
> + print ' and specify a number of mount points to return with -
> <num>, e.g. -1.'
> + print ' For example, use of "--sort -1" will iterate only
> showing you the stats'
> + print ' for the mount point with the highest ops/second.'
>
> def parse_stats_file(filename):
> """pop the contents of a mountstats file into a dictionary,
> @@ -446,7 +457,10 @@ def parse_stats_file(filename):
>
> return ms_dict
>
> -def print_iostat_summary(old, new, devices, time, ac):
> +def print_iostat_summary(old, new, devices, time, ac, sortbyops,
> entrycount):
> + diff_stats = {}
> + count = 1
> +
> if old:
> # Trim device list to only include intersection of old and
> new data,
> # this addresses umounts due to autofs mountpoints
> @@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,
> time, ac):
> devicelist = devices
>
> for device in devicelist:
> + count += 1
> stats = DeviceData()
> stats.parse_stats(new[device])
> if not old:
> stats.display_iostats(time, ac)
> + if (count>entrycount):
> + return
> else:
> old_stats = DeviceData()
> old_stats.parse_stats(old[device])
> - diff_stats = stats.compare_iostats(old_stats)
> - diff_stats.display_iostats(time, ac)
> + diff_stats[device] = stats.compare_iostats(old_stats)
> + if not sortbyops:
> + diff_stats[device].display_iostats(time, ac)
> + if (count>entrycount):
> + return
> +
> + if old and sortbyops:
> + # We now have compared data and can print a comparison
> + # ordered by mountpoint ops per second
> + count = 1
> +
> + devicelist.sort(key=lambda x: diff_stats[x].ops(time),
> reverse=True)
> +
> + for device in devicelist:
> + count += 1
> + diff_stats[device].display_iostats(time, ac)
> + if (count>entrycount):
> + return
>
> def list_nfs_mounts(givenlist, mountstats):
> """return a list of NFS mounts given a list to validate or
> @@ -497,6 +530,8 @@ def iostat_command(name):
> which = 0
> interval_seen = False
> count_seen = False
> + sortbyops = False
> + entrycount = sys.maxint
>
> for arg in sys.argv:
> if arg in ['-h', '--help', 'help', 'usage']:
> @@ -507,6 +542,19 @@ def iostat_command(name):
> print '%s version %s' % (name, Iostats_version)
> return
>
> + if arg in ['-s', '--sort', 'sort']:
> + sortbyops = True
> + # sorted display infers a loop, default to 1 second
> + if not interval_seen:
> + interval = 1
> + interval_seen = True
> + continue
> +
> + stop_re = re.compile('-[0-9]+')
> + if stop_re.match(arg):
> + entrycount = int(arg.lstrip('-'))
> + continue
> +
> if arg in ['-a', '--attr']:
> which = 1
> continue
> @@ -546,12 +594,12 @@ def iostat_command(name):
> sample_time = 0.0
>
> if not interval_seen:
> - print_iostat_summary(old_mountstats, mountstats, devices,
> sample_time, which)
> + print_iostat_summary(old_mountstats, mountstats, devices,
> sample_time, which, sortbyops, entrycount)
> return
>
> if count_seen:
> while count != 0:
> - print_iostat_summary(old_mountstats, mountstats,
> devices, sample_time, which)
> + print_iostat_summary(old_mountstats, mountstats,
> devices, sample_time, which, sortbyops, entrycount)
> old_mountstats = mountstats
> time.sleep(interval)
> sample_time = interval
> @@ -562,7 +610,7 @@ def iostat_command(name):
> count -= 1
> else:
> while True:
> - print_iostat_summary(old_mountstats, mountstats,
> devices, sample_time, which)
> + print_iostat_summary(old_mountstats, mountstats,
> devices, sample_time, which, sortbyops, entrycount)
> old_mountstats = mountstats
> time.sleep(interval)
> sample_time = interval
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"
> in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
On Aug 26, 2009, at 10:53 AM, Lans Carstensen wrote:
> Greetings! Comments inline. Chuck Lever wrote:
>> Hi Lans-
>> I was thinking about this feature a little more. Would it be
>> interesting to allow sorting by other metrics besides ops/s ?
>> That's another feature that the "top" command has that this new
>> option doesn't.
>> I also don't like the bare integer options. What if, at some
>> point, we want "-4" and "-6" for IPv6 support (not that we do, but
>> what if)? Plus, having the bare integers doesn't give any clues
>> about what the number means.
>> In which case I propose:
>> --sort=ops --list=<n>
>> which provides more flexibility and helps make the options more
>> self-documenting.
>
> Again, thanks for looking this over.
>
> I had thought about the sorting a little bit, but decided that every
> case we cared about devolved to sorting by ops/s. We'd use a tool
> like this in an operational group where they've lost control of a
> particular rogue workload in the mix of thousands of other
> workloads, and the operational group is trying to figure out what
> workload is generating a disproportionate amount of traffic (ops/
> s). If someone wanted to extend the implementation further where
> leaving "--sort" unadorned sorts by ops and using "--
> sort=readdirplus" or somesuch, I'd be all for it - but it didn't
> seem worth the work for our use. So I'd encourage that to be
> adopted as-is and if someone wants to put the extra effort into it
> I'm all for it.
Well, the problem is once we choose a command line option, it's hard
to change it later. I think having two choices, like --sort=ops and --
sort=def[ault] would be fine for now, and give us room to grow later.
> For the bare integer options I was thinking in terms of the existing
> "tail" and "head" implementations - that's what made the most sense
> to me and the couple of others looking over this here. Changing it
> to "--list=<n>" is fine if that suites you and others more. I can't
> quite fathom going after IPv4 or IPv6 stats with this, as /proc/self/
> mountstats a) doesn't expose any relevant data that would be IPv4/
> IPv6 oriented, and b) really never would as that's at an
> implementation layer below NFS ops. So the existing patch makes
> most sense to me, but if you and others want it changed I'd be fine
> w/ doing it.
The modern command line option on head/tail is --lines=<n>, which is
also an OK name for this option.
IPv4/IPv6 was just an example... some applications, like ssh, use "-4"
and "-6" already, which makes the use of -<n> ambiguous, I think.
>> On Aug 26, 2009, at 12:59 AM, Lans Carstensen wrote:
>>> commit 062577877bd3f1d8da1edeb889a09d380da83364
>>> Author: Lans Carstensen <[email protected]>
>>> Date: Tue Aug 25 21:53:54 2009 -0700
>>>
>>> Adds --sort option to display mount point stats sorted by ops/s
>>> Adds -<n> option to only display stats for first <n> mount points
>>> E.g. the use of "--sort -1" should be useful in seeing stats for
>>> only the mountpoint with the highest ops/s.
>>>
>>> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-
>>> iostat.py
>>> index 9f3b3eb..5df9bfb 100644
>>> --- a/tools/nfs-iostat/nfs-iostat.py
>>> +++ b/tools/nfs-iostat/nfs-iostat.py
>>> @@ -20,7 +20,7 @@ along with this program; if not, write to the
>>> Free Software
>>> Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>>> 02111-1307 USA
>>> """
>>>
>>> -import sys, os, time
>>> +import sys, os, time, re
>>>
>>> Iostats_version = '0.3'
>>>
>>> @@ -353,6 +353,12 @@ class DeviceData:
>>> print '\t%7.3f' % rtt_per_op,
>>> print '\t%7.3f' % exe_per_op
>>>
>>> + def ops(self, sample_time):
>>> + sends = float(self.__rpc_data['rpcsends'])
>>> + if sample_time == 0:
>>> + sample_time = float(self.__nfs_data['age'])
>>> + return (sends / sample_time)
>>> +
>>> def display_iostats(self, sample_time, which):
>>> """Display NFS and RPC stats in an iostat-like way
>>> """
>>> @@ -421,6 +427,11 @@ def print_iostat_help(name):
>>> print ' If one or more <mount point> names are specified,
>>> statistics for only these'
>>> print ' mount points will be displayed. Otherwise, all NFS
>>> mount points on the'
>>> print ' client are listed.'
>>> + print
>>> + print ' You can also specify "--sort" to sort the NFS mount
>>> points by ops/second,'
>>> + print ' and specify a number of mount points to return with -
>>> <num>, e.g. -1.'
>>> + print ' For example, use of "--sort -1" will iterate only
>>> showing you the stats'
>>> + print ' for the mount point with the highest ops/second.'
>>>
>>> def parse_stats_file(filename):
>>> """pop the contents of a mountstats file into a dictionary,
>>> @@ -446,7 +457,10 @@ def parse_stats_file(filename):
>>>
>>> return ms_dict
>>>
>>> -def print_iostat_summary(old, new, devices, time, ac):
>>> +def print_iostat_summary(old, new, devices, time, ac, sortbyops,
>>> entrycount):
>>> + diff_stats = {}
>>> + count = 1
>>> +
>>> if old:
>>> # Trim device list to only include intersection of old and
>>> new data,
>>> # this addresses umounts due to autofs mountpoints
>>> @@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,
>>> time, ac):
>>> devicelist = devices
>>>
>>> for device in devicelist:
>>> + count += 1
>>> stats = DeviceData()
>>> stats.parse_stats(new[device])
>>> if not old:
>>> stats.display_iostats(time, ac)
>>> + if (count>entrycount):
>>> + return
>>> else:
>>> old_stats = DeviceData()
>>> old_stats.parse_stats(old[device])
>>> - diff_stats = stats.compare_iostats(old_stats)
>>> - diff_stats.display_iostats(time, ac)
>>> + diff_stats[device] = stats.compare_iostats(old_stats)
>>> + if not sortbyops:
>>> + diff_stats[device].display_iostats(time, ac)
>>> + if (count>entrycount):
>>> + return
>>> +
>>> + if old and sortbyops:
>>> + # We now have compared data and can print a comparison
>>> + # ordered by mountpoint ops per second
>>> + count = 1
>>> +
>>> + devicelist.sort(key=lambda x: diff_stats[x].ops(time),
>>> reverse=True)
>>> +
>>> + for device in devicelist:
>>> + count += 1
>>> + diff_stats[device].display_iostats(time, ac)
>>> + if (count>entrycount):
>>> + return
>>>
>>> def list_nfs_mounts(givenlist, mountstats):
>>> """return a list of NFS mounts given a list to validate or
>>> @@ -497,6 +530,8 @@ def iostat_command(name):
>>> which = 0
>>> interval_seen = False
>>> count_seen = False
>>> + sortbyops = False
>>> + entrycount = sys.maxint
>>>
>>> for arg in sys.argv:
>>> if arg in ['-h', '--help', 'help', 'usage']:
>>> @@ -507,6 +542,19 @@ def iostat_command(name):
>>> print '%s version %s' % (name, Iostats_version)
>>> return
>>>
>>> + if arg in ['-s', '--sort', 'sort']:
>>> + sortbyops = True
>>> + # sorted display infers a loop, default to 1 second
>>> + if not interval_seen:
>>> + interval = 1
>>> + interval_seen = True
>>> + continue
>>> +
>>> + stop_re = re.compile('-[0-9]+')
>>> + if stop_re.match(arg):
>>> + entrycount = int(arg.lstrip('-'))
>>> + continue
>>> +
>>> if arg in ['-a', '--attr']:
>>> which = 1
>>> continue
>>> @@ -546,12 +594,12 @@ def iostat_command(name):
>>> sample_time = 0.0
>>>
>>> if not interval_seen:
>>> - print_iostat_summary(old_mountstats, mountstats, devices,
>>> sample_time, which)
>>> + print_iostat_summary(old_mountstats, mountstats, devices,
>>> sample_time, which, sortbyops, entrycount)
>>> return
>>>
>>> if count_seen:
>>> while count != 0:
>>> - print_iostat_summary(old_mountstats, mountstats,
>>> devices, sample_time, which)
>>> + print_iostat_summary(old_mountstats, mountstats,
>>> devices, sample_time, which, sortbyops, entrycount)
>>> old_mountstats = mountstats
>>> time.sleep(interval)
>>> sample_time = interval
>>> @@ -562,7 +610,7 @@ def iostat_command(name):
>>> count -= 1
>>> else:
>>> while True:
>>> - print_iostat_summary(old_mountstats, mountstats,
>>> devices, sample_time, which)
>>> + print_iostat_summary(old_mountstats, mountstats,
>>> devices, sample_time, which, sortbyops, entrycount)
>>> old_mountstats = mountstats
>>> time.sleep(interval)
>>> sample_time = interval
>>>
>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-
>>> nfs" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> --
>> Chuck Lever
>> chuck[dot]lever[at]oracle[dot]com
>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com