Change in vdsm[master]: sampling: hoststats: switch to SampleWindow

Francesco Romani has uploaded a new change for review.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................

sampling: hoststats: switch to SampleWindow

HostStatsThread has its own sampling storage code,
remniscent of VmStatsThread before the switch
of SampleWindow.

The requirements and the use cases are already almost
completely covered by SampleWindow, so this patch
ports HostStatsThread to it, to reduce duplication.

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


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/40428/1

diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py
index d59ce2c..cbe53ef 100644
--- a/vdsm/virt/sampling.py
+++ b/vdsm/virt/sampling.py
@@ -555,7 +555,6 @@
     """
     A thread that periodically samples host statistics.
     """
-    AVERAGING_WINDOW = 5
     _CONNLOG = logging.getLogger('connectivity')
 
     def __init__(self, log):
@@ -565,7 +564,7 @@
         self.daemon = True
         self._log = log
         self._stopEvent = threading.Event()
-        self._samples = []
+        self._samples = SampleWindow(size=5)
 
         self._pid = os.getpid()
         self._ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1)
@@ -584,15 +583,13 @@
                 try:
                     sample = HostSample(self._pid)
                     self._samples.append(sample)
-                    if len(self._samples) == 1:
+                    prev, last = self._samples.last_pair()
+                    if prev is None:
                         self._CONNLOG.debug('%s', sample.to_connlog())
                     else:
-                        diff = sample.connlog_diff(self._samples[-2])
+                        diff = sample.connlog_diff(prev)
                         if diff:
                             self._CONNLOG.debug('%s', diff)
-
-                    if len(self._samples) > self.AVERAGING_WINDOW:
-                        self._samples.pop(0)
                 except TimeoutError:
                     self._log.exception("Timeout while sampling stats")
                 self._stopEvent.wait(self._sampleInterval)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
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: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
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

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 1:

Build Started (1/2) ->
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1647/

-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 1:

Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18416/

-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 1:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1647/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18416/ : SUCCESS

-- 
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: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No

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

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: 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: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

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

Ido Barkan has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 6: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/40428/6//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-05-14 12:26:43 +0200
Line 6: 
Line 7: sampling: hoststats: switch to SampleWindow
Line 8: 
Line 9: HostStatsThread has ts own sampling storage code,
it's
Line 10: like VmStatsThread had before the introduction of
Line 11: the SampleWindow class.
Line 12: 
Line 13: The API of SampleWindow covers the HostStatsThread needs,


https://gerrit.ovirt.org/#/c/40428/6/vdsm/virt/sampling.py
File vdsm/virt/sampling.py:

Line 612:             'cpuUserVdsmd': 0.0,
Line 613:             'elapsedTime': int(time.time() - self.startTime)
Line 614:         }
Line 615: 
Line 616:         hs0, hs1, _ = self._samples.stats()
while you are here: maybe rename to "first_sample, last_sample" or something
else, that is more readable than hs0/1 ?
Line 617:         # we need a different interval, see below
Line 618:         if hs0 is None or hs1 is None:
Line 619:             return stats
Line 620: 


-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Francesco Romani has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 6:

(2 comments)

https://gerrit.ovirt.org/#/c/40428/6//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-05-14 12:26:43 +0200
Line 6: 
Line 7: sampling: hoststats: switch to SampleWindow
Line 8: 
Line 9: HostStatsThread has ts own sampling storage code,
>  it's 
Done
Line 10: like VmStatsThread had before the introduction of
Line 11: the SampleWindow class.
Line 12: 
Line 13: The API of SampleWindow covers the HostStatsThread needs,


https://gerrit.ovirt.org/#/c/40428/6/vdsm/virt/sampling.py
File vdsm/virt/sampling.py:

Line 612:             'cpuUserVdsmd': 0.0,
Line 613:             'elapsedTime': int(time.time() - self.startTime)
Line 614:         }
Line 615: 
Line 616:         hs0, hs1, _ = self._samples.stats()
>  while you are here: maybe rename to "first_sample,
> last_sample" or somethin 
good point, will do in a followup patch.
Line 617:         # we need a different interval, see below
Line 618:         if hs0 is None or hs1 is None:
Line 619:             return stats
Line 620: 


-- 
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

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

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 8:

* 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/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 <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com&gt;
Gerrit-Reviewer: Ido Barkan <ibarkan(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: 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 to the fact we
ignore the interval here; it is calculated 2 lines below?
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.
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 been taken,
and hs1 is None?
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 <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: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 8: Code-Review-1

-- 
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 <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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 9:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 9
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: Ido Barkan <ibarkan(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: 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 <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: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

Francesco Romani has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 8:

(1 comment)

https://gerrit.ovirt.org/#/c/40428/8/vdsm/virt/sampling.py
File vdsm/virt/sampling.py:

Line 681: 
Line 682:     def _getInterfacesStats(self):
Line 683:         stats = {}
Line 684:         hs0, hs1, _ = self._samples.stats()
Line 685:         if hs0 is None:
>  Yes, this is wrong. 
I said it is wrong because I got a bit
confused: code uses either last two samples or first and last samples, and it has unusual
sample window = 5.

Actually, this should not explode for two reasons:
* _getInterfaceStats is called when hs0 is not none, so when we do have at least two
samples.
* if there are at least two samples, bot hs0 and hs1 are not None.

This means that this check is redundant, but I would like to remove in a different patch.
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 <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: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes

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

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 10:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 10
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: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No

Ido Barkan has posted comments on this change.

Change subject: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 10:

given this is hard to unittest before the patch, at least a basic unit test is needed here
to prove that SampleWindow doesn't change HostStatsThread behaviour.

-- 
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: 10
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 11:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 11
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 12:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 12
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 12: Code-Review-1 Verified+1

seems to work - verified with vdsClient -s 0 getVdsStats, but I'd like to have at
least one unit test for this.

-- 
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: 12
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 13:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 13
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 14:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 14
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 15:

* 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/40428
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 15
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 15: Code-Review+2

-- 
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: 15
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 15: Verified+1

Verified using the tests added in the parent patch.

-- 
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: 15
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


sampling: hoststats: switch to SampleWindow

HostStatsThread has its own sampling storage code,
like VmStatsThread had before the introduction of
the SampleWindow class.

The API of SampleWindow covers the HostStatsThread needs,
so this patch ports HostStatsThread to SampleWindow,
to reduce duplication.

Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt;
Reviewed-on: https://gerrit.ovirt.org/40428
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com&gt;
---
M tests/samplingTests.py
M vdsm/virt/sampling.py
2 files changed, 10 insertions(+), 12 deletions(-)

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



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I5ec5712ee1fb69ed3d8dbc28ca4e6511998270c9
Gerrit-PatchSet: 16
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: Ido Barkan <ibarkan(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: sampling: hoststats: switch to SampleWindow
......................................................................


Patch Set 16:

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

-- 
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: 16
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: Ido Barkan <ibarkan(a)redhat.com&gt;
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: No