nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
Subsystems (2)
PATCH 4/4 nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s
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.
tools/nfs-iostat/nfs-iostat.py +7 -1
@@ -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
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.
Quoted 168 lines · Lans Carstensen
commit
062577877bd3f1d8da1edeb889a09d380da83364Author: Lans Carstensen <[email protected]> Date: Tue Aug 25 21:53:54 2009 -0700Adds --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.
tools/nfs-iostat/nfs-iostat.py +0
-0@@ -20,7 +20,7 @@ along with this program; if not, write to the FreeSoftware
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA """tools/nfs-iostat/nfs-iostat.py +1
-1-import sys, os, time +import sys, os, time, re
Iostats_version= '0.3'tools/nfs-iostat/nfs-iostat.py +6
-0@@ -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.'tools/nfs-iostat/nfs-iostat.py +2
-0+ print + print ' You can also specify "--sort" to sort the NFS mountpoints 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,tools/nfs-iostat/nfs-iostat.py +1
-1@@ -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):
tools/nfs-iostat/nfs-iostat.py +3
-0+ diff_stats = {} + count = 1 + if old: # Trim device list to only include intersection of old andnew data,
# this addresses umounts due to autofs mountpointstools/nfs-iostat/nfs-iostat.py +0
-0@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,time, ac):
devicelist = devicesfor device in devicelist:
tools/nfs-iostat/nfs-iostat.py +15
-2+ 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)
tools/nfs-iostat/nfs-iostat.py +6
-0+ + for device in devicelist: + count += 1 + diff_stats[device].display_iostats(time, ac) + if (count>entrycount): + returndef
list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate ortools/nfs-iostat/nfs-iostat.py +17
-2@@ -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= mountstatstime.sleep(interval)sample_time= intervaltools/nfs-iostat/nfs-iostat.py +1
-1@@ -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
Quoted 4 lines
--
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
Quoted 11 lines · Greetings! Comments inline. Chuck Lever
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.
Quoted 181 lines · Lans Carstensen
Quoted 173 lines
commit
062577877bd3f1d8da1edeb889a09d380da83364Author: Lans Carstensen <[email protected]> Date: Tue Aug 25 21:53:54 2009 -0700Adds --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.
tools/nfs-iostat/nfs-iostat.py +0
-0@@ -20,7 +20,7 @@ along with this program; if not, write to the FreeSoftware
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA """tools/nfs-iostat/nfs-iostat.py +1
-1-import sys, os, time +import sys, os, time, re
Iostats_version= '0.3'tools/nfs-iostat/nfs-iostat.py +6
-0@@ -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.'tools/nfs-iostat/nfs-iostat.py +2
-0+ print + print ' You can also specify "--sort" to sort the NFS mountpoints by ops/second,'
tools/nfs-iostat/nfs-iostat.py +2
-1+ 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 showingyou 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,tools/nfs-iostat/nfs-iostat.py +1
-1@@ -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):
tools/nfs-iostat/nfs-iostat.py +3
-0+ diff_stats = {} + count = 1 + if old: # Trim device list to only include intersection of old and newdata,
# this addresses umounts due to autofs mountpointstools/nfs-iostat/nfs-iostat.py +0
-0@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,time, ac):
devicelist = devicesfor device in devicelist:
tools/nfs-iostat/nfs-iostat.py +15
-2+ 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)
tools/nfs-iostat/nfs-iostat.py +6
-0+ + for device in devicelist: + count += 1 + diff_stats[device].display_iostats(time, ac) + if (count>entrycount): + returndef
list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate ortools/nfs-iostat/nfs-iostat.py +17
-2@@ -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= intervaltools/nfs-iostat/nfs-iostat.py +1
-1@@ -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= intervalSignature · 3 lines
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.htmlSignature · 2 lines
Chuck Lever chuck[dot]lever[at]oracle[dot]com
Quoted 11 lines · Lans Carstensen
Quoted 6 lines · Greetings! Comments inline. Chuck Lever
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.
Quoted 1 line
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/mountstatsa) 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.
Quoted 169 lines
Quoted 169 lines · Lans Carstensen
Quoted 168 lines
commit
062577877bd3f1d8da1edeb889a09d380da83364Author: Lans Carstensen <[email protected]> Date: Tue Aug 25 21:53:54 2009 -0700Adds --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.
tools/nfs-iostat/nfs-iostat.py +0
-0@@ -20,7 +20,7 @@ along with this program; if not, write to theFree Software
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA """tools/nfs-iostat/nfs-iostat.py +1
-1-import sys, os, time +import sys, os, time, re
Iostats_version= '0.3'tools/nfs-iostat/nfs-iostat.py +6
-0@@ -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.'tools/nfs-iostat/nfs-iostat.py +2
-0+ print + print ' You can also specify "--sort" to sort the NFS mountpoints 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,tools/nfs-iostat/nfs-iostat.py +1
-1@@ -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):
tools/nfs-iostat/nfs-iostat.py +3
-0+ diff_stats = {} + count = 1 + if old: # Trim device list to only include intersection of old andnew data,
# this addresses umounts due to autofs mountpointstools/nfs-iostat/nfs-iostat.py +0
-0@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,time, ac):
devicelist = devicesfor device in devicelist:
tools/nfs-iostat/nfs-iostat.py +15
-2+ 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)
tools/nfs-iostat/nfs-iostat.py +6
-0+ + for device in devicelist: + count += 1 + diff_stats[device].display_iostats(time, ac) + if (count>entrycount): + returndef
list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate ortools/nfs-iostat/nfs-iostat.py +17
-2@@ -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= mountstatstime.sleep(interval)
sample_time= intervaltools/nfs-iostat/nfs-iostat.py +1
-1@@ -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
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
Signature · 3 lines
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.htmlSignature · 2 lines
Chuck Lever chuck[dot]lever[at]oracle[dot]com
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
Quoted 32 lines · Chuck Lever
Quoted 17 lines · Lans Carstensen
Quoted 5 lines · Greetings! Comments inline. Chuck Lever
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.Quoted 8 lines
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.
Quoted 12 lines
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.
Quoted 3 lines
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
Quoted 37 lines · Trond Myklebust
Quoted 32 lines · Chuck Lever
Quoted 17 lines · Lans Carstensen
Quoted 5 lines · Greetings! Comments inline. Chuck Lever
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.Quoted 8 lines
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.
Quoted 12 lines
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.
Quoted 3 lines
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.
Quoted 5 lines
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