Francesco Romani has posted comments on this change. Change subject: sampling: hoststats: switch to SampleWindow ......................................................................
Patch Set 8: (3 comments) https://gerrit.ovirt.org/#/c/40428/8/vdsm/virt/sampling.py File vdsm/virt/sampling.py: Line 613: 'elapsedTime': int(time.time() - self.startTime) Line 614: } Line 615: Line 616: hs0, hs1, _ = self._samples.stats() Line 617: # we need a different interval, see below > it is not clear (to me) to what does this comment relate to. Is it related the comment is confusing, will drop it. Line 618: if hs0 is None or hs1 is None: Line 619: return stats Line 620: Line 621: stats.update(self._getInterfacesStats()) Line 614: } Line 615: Line 616: hs0, hs1, _ = self._samples.stats() Line 617: # we need a different interval, see below Line 618: if hs0 is None or hs1 is None: > checking for hs1 is enough, right? if it is there, hs0 must exist. Yep Line 619: return stats Line 620: Line 621: stats.update(self._getInterfacesStats()) Line 622: Line 681: Line 682: def _getInterfacesStats(self): Line 683: stats = {} Line 684: hs0, hs1, _ = self._samples.stats() Line 685: if hs0 is None: > wouldn't this explode on the very start of vdsm, when only one sample has b Yes, this is wrong. Line 686: return stats Line 687: interval = hs1.timestamp - hs0.timestamp Line 688: Line 689: rx = tx = rxDropped = txDropped = 0 -- To view, visit https://gerrit.ovirt.org/40428 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
