Change in vdsm[master]: vmstats: do not catch unlikely ZeroDivisionError

Francesco Romani has uploaded a new change for review.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................

vmstats: do not catch unlikely ZeroDivisionError

Nowadays, the only foreseeable reason to have ZeroDivisionError
in the vmstats functions, is when interval is zero.

This case is in turn very very unlikely, as it should
happen only in the extravagant case of two polling cycles racing
together, saving bad bugs in the sampling code.

So, drop the ZeroDivisionError from the handled
exception, and we add an explicit guard to be sure
we record these end-of-the-world-like cases.

Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
---
M vdsm/virt/vmstats.py
1 file changed, 17 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/16/40916/1

diff --git a/vdsm/virt/vmstats.py b/vdsm/virt/vmstats.py
index 4af8501..11feead 100644
--- a/vdsm/virt/vmstats.py
+++ b/vdsm/virt/vmstats.py
@@ -74,6 +74,11 @@
 
     if first_sample is None or last_sample is None:
         return
+    if interval <= 0:
+        logging.warning(
+            'invalid interval %i when computing CPU stats',
+            interval)
+        return
 
     stats['cpuUsage'] = str(last_sample['system_time']
                             + last_sample['user_time'])
@@ -88,7 +93,7 @@
             - _diff(last_sample, first_sample, 'cpu.system'),
             interval)
 
-    except (KeyError, TypeError, ZeroDivisionError) as e:
+    except (KeyError, TypeError) as e:
         logging.exception("CPU stats not available: %s", e)
 
 
@@ -206,10 +211,16 @@
             last_disk = last_sample.get('block', {}).get(vm_drive.name, {})
             if first_disk and last_disk:
                 # will be None if sampled during recovery
-                drive_stats.update(
-                    _disk_rate(first_disk, last_disk, interval))
-                drive_stats.update(
-                    _disk_latency(first_disk, last_disk))
+                if interval > 0:
+                    drive_stats.update(
+                        _disk_rate(first_disk, last_disk, interval))
+                    drive_stats.update(
+                        _disk_latency(first_disk, last_disk))
+                else:
+                    logging.warning(
+                        'invalid interval %i when calculating '
+                        'stats for vm %s disk %s',
+                        interval, vm.id, vm_drive.name)
 
                 drive_info = last_sample[vm_drive.name]
                 drive_stats['readOps'] = str(drive_info['rd.reqs'])
@@ -217,7 +228,7 @@
                 drive_stats['readBytes'] = str(drive_info['rd.bytes'])
                 drive_stats['writtenBytes'] =
str(drive_info['wr.bytes'])
 
-        except (AttributeError, TypeError, ZeroDivisionError):
+        except (AttributeError, TypeError):
             logging.exception("Disk %s stats not available",
                               vm_drive.name)
 


-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 3:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 4:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 5:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 6:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Francesco Romani has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 6: Verified+1

tested the positive flow

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 7:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and
is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5',
'ovirt-3.4', 'ovirt-3.3'])

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 7: Code-Review+2

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


vmstats: do not catch unlikely ZeroDivisionError

Nowadays, the only foreseeable reason to have ZeroDivisionError
in the vmstats functions, is when interval is zero.

This case is in turn very very unlikely, as it should
happen only in the extravagant case of two polling cycles racing
together, saving bad bugs in the sampling code.

So, drop the ZeroDivisionError from the handled
exception, and we add an explicit guard to be sure
we record these end-of-the-world-like cases.

Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/40916
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M vdsm/virt/vmstats.py
1 file changed, 17 insertions(+), 6 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified



-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org

automation(a)ovirt.org has posted comments on this change.

Change subject: vmstats: do not catch unlikely ZeroDivisionError
......................................................................


Patch Set 8:

* Update tracker::IGNORE, no Bug-Url found
* Set MODIFIED::IGNORE, no Bug-Url found.

-- 
To view, visit https://gerrit.ovirt.org/40916
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6c6719414d185a8b359b5d8a9f4a28ac87b64327
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No