2020-08-31 17:08:15

by Kenneth Dsouza

[permalink] [raw]
Subject: [PATCH] nfsiostat/mountstats: Drop autofs entries before calling compare_iostats()

nfsiostat/mountstats can fail with below KeyError when old stat and new stat
data go out of sync.

$ mountstats iostat 1 3

Traceback (most recent call last):
File "/usr/sbin/mountstats", line 1092, in <module>
res = main()
File "/usr/sbin/mountstats", line 1081, in main
return args.func(args)
File "/usr/sbin/mountstats", line 965, in iostat_command
print_iostat_summary(old_mountstats, mountstats, devices, sample_time)
File "/usr/sbin/mountstats", line 920, in print_iostat_summary
diff_stats = stats.compare_iostats(old_stats)
File "/usr/sbin/mountstats", line 528, in compare_iostats
if old_stats.__nfs_data['age'] > self.__nfs_data['age']:
KeyError: 'age'

Steps to Reproduce:

1> Add autofs mounts in /etc/fstab controlled via systemd.

nfs-server:/test1 /mnt1 nfs noauto,x-systemd.idle-timeout=2,x-systemd.automount 0 0
nfs-server:/test2 /mnt2 nfs noauto,x-systemd.idle-timeout=2,x-systemd.automount 0 0
nfs-server:/test3 /mnt3 nfs noauto,x-systemd.idle-timeout=2,x-systemd.automount 0 0

2> Trigger the mounts via below command:

$ while :; do date; ls -lR /mnt* 2>&1 >/dev/null; sleep 3; done

3> On other terminal run nfsiostat or mountstats command:

$ mountstats iostat 1 3
$ nfsiostat 1 3

Frequent mount and umount can cause autofs entries to be processed in compare_iostats.
We need to filter the devices list and drop autofs entries to fix the issue.
This way we pass only nfs mounts and not autofs entries.

Signed-off-by: Kenneth D'souza <[email protected]>
---
tools/mountstats/mountstats.py | 9 +++++----
tools/nfs-iostat/nfs-iostat.py | 5 ++++-
2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
index 00adc96b..25e92a19 100755
--- a/tools/mountstats/mountstats.py
+++ b/tools/mountstats/mountstats.py
@@ -953,10 +953,11 @@ def print_iostat_summary(old, new, devices, time):
if not old or device not in old:
stats.display_iostats(time)
else:
- old_stats = DeviceData()
- old_stats.parse_stats(old[device])
- diff_stats = stats.compare_iostats(old_stats)
- diff_stats.display_iostats(time)
+ if ("fstype autofs" not in str(old[device])) and ("fstype autofs" not in str(new[device])):
+ old_stats = DeviceData()
+ old_stats.parse_stats(old[device])
+ diff_stats = stats.compare_iostats(old_stats)
+ diff_stats.display_iostats(time)

def iostat_command(args):
"""iostat-like command for NFS mount points
diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
index 4f5e8a66..1df74ba8 100755
--- a/tools/nfs-iostat/nfs-iostat.py
+++ b/tools/nfs-iostat/nfs-iostat.py
@@ -470,10 +470,13 @@ def parse_stats_file(filename):
def print_iostat_summary(old, new, devices, time, options):
stats = {}
diff_stats = {}
+ devicelist = []
if old:
# Trim device list to only include intersection of old and new data,
# this addresses umounts due to autofs mountpoints
- devicelist = [x for x in old if x in devices]
+ for device in devices:
+ if "fstype autofs" not in str(old[device]):
+ devicelist.append(device)
else:
devicelist = devices

--
2.21.3


2020-09-08 19:57:32

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] nfsiostat/mountstats: Drop autofs entries before calling compare_iostats()



On 8/31/20 1:06 PM, Kenneth D'souza wrote:
> nfsiostat/mountstats can fail with below KeyError when old stat and new stat
> data go out of sync.
>
> $ mountstats iostat 1 3
>
> Traceback (most recent call last):
> File "/usr/sbin/mountstats", line 1092, in <module>
> res = main()
> File "/usr/sbin/mountstats", line 1081, in main
> return args.func(args)
> File "/usr/sbin/mountstats", line 965, in iostat_command
> print_iostat_summary(old_mountstats, mountstats, devices, sample_time)
> File "/usr/sbin/mountstats", line 920, in print_iostat_summary
> diff_stats = stats.compare_iostats(old_stats)
> File "/usr/sbin/mountstats", line 528, in compare_iostats
> if old_stats.__nfs_data['age'] > self.__nfs_data['age']:
> KeyError: 'age'
>
> Steps to Reproduce:
>
> 1> Add autofs mounts in /etc/fstab controlled via systemd.
>
> nfs-server:/test1 /mnt1 nfs noauto,x-systemd.idle-timeout=2,x-systemd.automount 0 0
> nfs-server:/test2 /mnt2 nfs noauto,x-systemd.idle-timeout=2,x-systemd.automount 0 0
> nfs-server:/test3 /mnt3 nfs noauto,x-systemd.idle-timeout=2,x-systemd.automount 0 0
>
> 2> Trigger the mounts via below command:
>
> $ while :; do date; ls -lR /mnt* 2>&1 >/dev/null; sleep 3; done
>
> 3> On other terminal run nfsiostat or mountstats command:
>
> $ mountstats iostat 1 3
> $ nfsiostat 1 3
>
> Frequent mount and umount can cause autofs entries to be processed in compare_iostats.
> We need to filter the devices list and drop autofs entries to fix the issue.
> This way we pass only nfs mounts and not autofs entries.
>
> Signed-off-by: Kenneth D'souza <[email protected]>
Committed...

steved.
> ---
> tools/mountstats/mountstats.py | 9 +++++----
> tools/nfs-iostat/nfs-iostat.py | 5 ++++-
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/tools/mountstats/mountstats.py b/tools/mountstats/mountstats.py
> index 00adc96b..25e92a19 100755
> --- a/tools/mountstats/mountstats.py
> +++ b/tools/mountstats/mountstats.py
> @@ -953,10 +953,11 @@ def print_iostat_summary(old, new, devices, time):
> if not old or device not in old:
> stats.display_iostats(time)
> else:
> - old_stats = DeviceData()
> - old_stats.parse_stats(old[device])
> - diff_stats = stats.compare_iostats(old_stats)
> - diff_stats.display_iostats(time)
> + if ("fstype autofs" not in str(old[device])) and ("fstype autofs" not in str(new[device])):
> + old_stats = DeviceData()
> + old_stats.parse_stats(old[device])
> + diff_stats = stats.compare_iostats(old_stats)
> + diff_stats.display_iostats(time)
>
> def iostat_command(args):
> """iostat-like command for NFS mount points
> diff --git a/tools/nfs-iostat/nfs-iostat.py b/tools/nfs-iostat/nfs-iostat.py
> index 4f5e8a66..1df74ba8 100755
> --- a/tools/nfs-iostat/nfs-iostat.py
> +++ b/tools/nfs-iostat/nfs-iostat.py
> @@ -470,10 +470,13 @@ def parse_stats_file(filename):
> def print_iostat_summary(old, new, devices, time, options):
> stats = {}
> diff_stats = {}
> + devicelist = []
> if old:
> # Trim device list to only include intersection of old and new data,
> # this addresses umounts due to autofs mountpoints
> - devicelist = [x for x in old if x in devices]
> + for device in devices:
> + if "fstype autofs" not in str(old[device]):
> + devicelist.append(device)
> else:
> devicelist = devices
>
>