Francesco Romani has uploaded a new change for review.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
sampling: add 'ncpus' property to HostSample
to decouple HostStatsThread and hoststats.produce().
Change-Id: I0e7d549b7772e3f38eec2dd9b91bbc6416b549bf Signed-off-by: Francesco Romani fromani@redhat.com --- M vdsm/virt/hoststats.py M vdsm/virt/sampling.py 2 files changed, 7 insertions(+), 6 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/35/42035/1
diff --git a/vdsm/virt/hoststats.py b/vdsm/virt/hoststats.py index 57c6954..7a23743 100644 --- a/vdsm/virt/hoststats.py +++ b/vdsm/virt/hoststats.py @@ -26,7 +26,7 @@ import v2v
-def produce(ncpus, first_sample, last_sample): +def produce(first_sample, last_sample): stats = _empty_stats()
if first_sample is None: @@ -45,10 +45,10 @@
jiffies = ( last_sample.totcpu.user - first_sample.totcpu.user) % (2 ** 32) - stats['cpuUser'] = jiffies / interval / ncpus + stats['cpuUser'] = jiffies / interval / last_sample.ncpus jiffies = ( last_sample.totcpu.sys - first_sample.totcpu.sys) % (2 ** 32) - stats['cpuSys'] = jiffies / interval / ncpus + stats['cpuSys'] = jiffies / interval / last_sample.ncpus stats['cpuIdle'] = max(0.0, 100.0 - stats['cpuUser'] - stats['cpuSys']) stats['memUsed'] = last_sample.memUsed diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py index 5729a54..401bae9 100644 --- a/vdsm/virt/sampling.py +++ b/vdsm/virt/sampling.py @@ -232,7 +232,7 @@ d[p] = {'free': str(free)} return d
- def __init__(self, pid): + def __init__(self, pid, ncpus): """ Initialize a HostSample.
@@ -242,6 +242,7 @@ super(HostSample, self).__init__() self.interfaces = _get_interfaces_and_samples() self.pidcpu = PidCpuSample(pid) + self.ncpus = ncpus self.totcpu = TotalCpuSample() meminfo = utils.readMemInfo() freeOrCached = (meminfo['MemFree'] + @@ -553,7 +554,7 @@ time.sleep(self._sampleInterval) while not self._stopEvent.isSet(): try: - sample = HostSample(self._pid) + sample = HostSample(self._pid, self._ncpus) self._samples.append(sample) second_last = self._samples.last(nth=2) if second_last is None: @@ -571,7 +572,7 @@
def get(self): first_sample, last_sample, _ = self._samples.stats() - return hoststats.produce(self._ncpus, first_sample, last_sample) + return hoststats.produce(first_sample, last_sample)
def _getLinkSpeed(dev):
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
Roman Mohr has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 13:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/13/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 555: self._ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1) I think ncpus can be moved to hoststats.py and we can look it up every time.
automation@ovirt.org has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
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'])
Francesco Romani has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 13:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/13/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 555: self._ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1)
I think ncpus can be moved to hoststats.py and we can look it up every time
nice idea, but also change in behaviour. Which is probably a good thing, because the old one doesn't consider the cpu offlining/onlining.
Francesco Romani has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 14: Verified+1
verified with 42034 running patched VDSM and checking from time to time the output of "vdsClient -s 0 getVdsStats"
Roman Mohr has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 13:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/13/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 555: self._ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1)
nice idea, but also change in behaviour. Which is probably a good thing, be
I guess even more correct, would be to store the number of CPUs always when the sample is collected, with the creation of HostStats. But we would also have to adjust our formula in hoststats then because the samples could then have different cpu counts.
Roman Mohr has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 13:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/13/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 555: self._ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1)
I guess even more correct, would be to store the number of CPUs always when
I meant with the creation of HostSample, not HostStats.
Roman Mohr has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 14: Code-Review+1
(1 comment)
https://gerrit.ovirt.org/#/c/42035/14/vdsm/virt/hoststats.py File vdsm/virt/hoststats.py:
Line 72: ncpus Maybe memorizing ncpus in hoststats.py (with @memorized) would be another non behaviour changing way to decouple the packages.
Francesco Romani has posted comments on this change.
Change subject: sampling: add 'ncpus' property to HostSample ......................................................................
Patch Set 14:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/14/vdsm/virt/hoststats.py File vdsm/virt/hoststats.py:
Line 72: ncpus
Maybe memorizing ncpus in hoststats.py (with @memorized) would be another n
I think (also considering your recent patches) that this change is actually beneficial, so I'm inclined to keep it.
automation@ovirt.org has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
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'])
Roman Mohr has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 15: Code-Review+1
automation@ovirt.org has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 16:
* 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'])
Francesco Romani has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 16: Verified+1
verified running patche VDSM. Host stats are still collected and reported (verified using getVdsStats). The actual value will of course change if admin hot(un)plugs CPU, but I think the new code is more correct. Hence, V+1
Dan Kenigsberg has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 16:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/16/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 246: """ Line 247: super(HostSample, self).__init__() Line 248: self.interfaces = _get_interfaces_and_samples() Line 249: self.pidcpu = PidCpuSample(pid) Line 250: self.ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1) can NPROCESSORS_ONLN ever be less than 1? Line 251: self.totcpu = TotalCpuSample() Line 252: meminfo = utils.readMemInfo() Line 253: freeOrCached = (meminfo['MemFree'] + Line 254: meminfo['Cached'] + meminfo['Buffers'])
Francesco Romani has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 16:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/16/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 246: """ Line 247: super(HostSample, self).__init__() Line 248: self.interfaces = _get_interfaces_and_samples() Line 249: self.pidcpu = PidCpuSample(pid) Line 250: self.ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1)
can NPROCESSORS_ONLN ever be less than 1?
Unlikely (to say the least), but here I just moved existing code. Will remove this check in a separate patch. Line 251: self.totcpu = TotalCpuSample() Line 252: meminfo = utils.readMemInfo() Line 253: freeOrCached = (meminfo['MemFree'] + Line 254: meminfo['Cached'] + meminfo['Buffers'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 17:
* 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'])
Francesco Romani has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 16:
(1 comment)
https://gerrit.ovirt.org/#/c/42035/16/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 246: """ Line 247: super(HostSample, self).__init__() Line 248: self.interfaces = _get_interfaces_and_samples() Line 249: self.pidcpu = PidCpuSample(pid) Line 250: self.ncpus = max(os.sysconf('SC_NPROCESSORS_ONLN'), 1)
Unlikely (to say the least), but here I just moved existing code. Will remo
Done in https://gerrit.ovirt.org/47991 Line 251: self.totcpu = TotalCpuSample() Line 252: meminfo = utils.readMemInfo() Line 253: freeOrCached = (meminfo['MemFree'] + Line 254: meminfo['Cached'] + meminfo['Buffers'])
Martin Polednik has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 17: Code-Review+1
I believe that this is step in the right direction at least in ppc64le world, where one can easily change smt level and on/offline cores via simple utility. That said, I still encounter few issues when trying to get the stats after offlining some cores.
Dan Kenigsberg has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 17: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: sampling: HostSample always gets current cpus ......................................................................
sampling: HostSample always gets current cpus
To have HostSample report more faithful information, and to decouple HostStatsThread and hoststats.produce(), we now compute each time the current number of online cpus.
Previously, this was done anche at initialization time.
Change-Id: I0e7d549b7772e3f38eec2dd9b91bbc6416b549bf Signed-off-by: Francesco Romani fromani@redhat.com Reviewed-on: https://gerrit.ovirt.org/42035 Reviewed-by: Roman Mohr rmohr@redhat.com Continuous-Integration: Jenkins CI Reviewed-by: Martin Polednik mpolednik@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/virt/hoststats.py M vdsm/virt/sampling.py 2 files changed, 5 insertions(+), 5 deletions(-)
Approvals: Roman Mohr: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified Martin Polednik: Looks good to me, but someone else must approve
automation@ovirt.org has posted comments on this change.
Change subject: sampling: HostSample always gets current cpus ......................................................................
Patch Set 18:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org
-
automation＠ovirt.org -
Dan Kenigsberg -
fromani＠redhat.com -
Martin Polednik -
rmohr＠redhat.com