Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/61821
to review the following change.
Change subject: virt: sampling: add is_empty() method to StatsSample
......................................................................
virt: sampling: add is_empty() method to StatsSample
One upcoming patch wants to fill the 'stats_age'
field with meaningful value.
The problem is that None is misinterpreted by the Vm code,
so if the stats are too old or missing, the Vm is not
detected as unresponsive.
So, the EMPTY_SAMPLE is not good anymore, and this patch
removes it.
We still need to know if a given sample is empty, meaning
it doesn't hold meaningful data, so we add one is_empty()
method to fulfill this task.
Another option could be implement __nonzero__, but it
doesn't completely fit in this case.
Change-Id: I7bc3712d36c46ade9a779411028817c197ec34b3
Backport-To: 4.0
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1357798
Signed-off-by: Francesco Romani <from...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/61420
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <dan...@redhat.com>
---
M lib/vdsm/virt/sampling.py
M tests/samplingTests.py
2 files changed, 17 insertions(+), 11 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/21/61821/1
diff --git a/lib/vdsm/virt/sampling.py b/lib/vdsm/virt/sampling.py
index b49666b..f84746b 100644
--- a/lib/vdsm/virt/sampling.py
+++ b/lib/vdsm/virt/sampling.py
@@ -346,12 +346,18 @@
return sample
-StatsSample = namedtuple('StatsSample',
- ['first_value', 'last_value',
- 'interval', 'stats_age'])
+_StatsSample = namedtuple('StatsSample',
+ ['first_value', 'last_value',
+ 'interval', 'stats_age'])
-EMPTY_SAMPLE = StatsSample(None, None, None, None)
+class StatsSample(_StatsSample):
+ def is_empty(self):
+ return (
+ self.first_value is None and
+ self.last_value is None and
+ self.interval is None
+ )
class StatsCache(object):
@@ -414,13 +420,13 @@
first_batch, last_batch, interval = self._samples.stats()
if first_batch is None:
- return EMPTY_SAMPLE
+ return StatsSample(None, None, None, None)
first_sample = first_batch.get(vmid)
last_sample = last_batch.get(vmid)
if first_sample is None or last_sample is None:
- return EMPTY_SAMPLE
+ return StatsSample(None, None, None, None)
stats_age = self._clock() - self._vm_last_timestamp[vmid]
diff --git a/tests/samplingTests.py b/tests/samplingTests.py
index ee8be7f..f0f22e5 100644
--- a/tests/samplingTests.py
+++ b/tests/samplingTests.py
@@ -256,14 +256,14 @@
def test_empty(self):
res = self.cache.get('x') # vmid not relevant
- self.assertEqual(res, sampling.EMPTY_SAMPLE)
+ self.assertTrue(res.is_empty())
def test_not_enough_samples(self):
self._feed_cache((
({'a': 42}, 1),
))
res = self.cache.get('a')
- self.assertEqual(res, sampling.EMPTY_SAMPLE)
+ self.assertTrue(res.is_empty())
def test_get(self):
self._feed_cache((
@@ -283,7 +283,7 @@
({'a': 'bar'}, 2)
))
res = self.cache.get('b')
- self.assertEqual(res, sampling.EMPTY_SAMPLE)
+ self.assertTrue(res.is_empty())
def test_put_overwrite(self):
self._feed_cache((
@@ -322,8 +322,8 @@
))
self.assertEqual(self.cache.get('a'),
('bar', 'baz', 1, FakeClock.STEP))
- self.assertEqual(self.cache.get('b'),
- sampling.EMPTY_SAMPLE)
+ res = self.cache.get('b')
+ self.assertTrue(res.is_empty())
def _feed_cache(self, samples):
for sample in samples:
--
To view, visit https://gerrit.ovirt.org/61821
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7bc3712d36c46ade9a779411028817c197ec34b3
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
