Skip to contentSkip to conversation

nfs-utils: nfs-iostat.py autofs cleanup and option to sort by ops/s

PATCH In review Lans Carstensen 3 participants 6 messages Aug 26, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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
index 9f3b3eb..5df9bfb 100644
@@ -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

#1 lore raw

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 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 +0 -0
index 9f3b3eb..5df9bfb 100644
@@ -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 """

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 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,

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  

new data,

        # this addresses umounts due to autofs mountpoints
tools/nfs-iostat/nfs-iostat.py +0 -0
@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,  

time, ac):
devicelist = devices

for 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):
+                return

def list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate or

tools/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 = interval

tools/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

#2 lore raw
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 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 +0 -0
index 9f3b3eb..5df9bfb 100644
@@ -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 """

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 mount 

points 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 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 and new 

data,

        # this addresses umounts due to autofs mountpoints
tools/nfs-iostat/nfs-iostat.py +0 -0
@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices, 

time, ac):
devicelist = devices

for 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):
+                return

def list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate or

tools/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 = interval

tools/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

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.html
Signature · 2 lines
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
#3 lore raw
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/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.

Quoted 169 lines
Quoted 169 lines · Lans Carstensen
Quoted 168 lines

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 +0 -0
index 9f3b3eb..5df9bfb 100644
@@ -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 """

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 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,

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  

new data,

       # this addresses umounts due to autofs mountpoints
tools/nfs-iostat/nfs-iostat.py +0 -0
@@ -455,15 +469,34 @@ def print_iostat_summary(old, new, devices,  

time, ac):
devicelist = devices

for 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):
+                return

def list_nfs_mounts(givenlist, mountstats):
"""return a list of NFS mounts given a list to validate or

tools/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 = interval

tools/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.html
Signature · 2 lines
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

#4 lore raw
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

#5 lore raw
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