Change in vdsm[master]: vm: migration: exponential downtime increment
mpolednik at redhat.com
mpolednik at redhat.com
Tue Sep 16 09:54:37 UTC 2014
Martin Polednik has posted comments on this change.
Change subject: vm: migration: exponential downtime increment
......................................................................
Patch Set 8: Code-Review-1
(5 comments)
-1 for visibility, other than the minor things it seems to be OK (+ need answer for the steps >= 2 question)
http://gerrit.ovirt.org/#/c/25820/8/tests/vmMigrationTests.py
File tests/vmMigrationTests.py:
Line 46: class TestVmMigrationDowntimeSequence(TestCaseBase):
Line 47:
Line 48: @permutations(_PARAMS)
Line 49: def test_downtime_is_sequence(self, dtime, steps):
Line 50: self.assertTrue(len(self._default(dtime, steps)) >= 2)
can be rewritten as self.assertGreaterEqual(len(self._default(dtime, steps)), 2)
another thing, is steps really >= 2 or do we allow single step?
Line 51:
Line 52: @permutations(_PARAMS)
Line 53: def test_downtime_increasing(self, dtime, steps):
Line 54: for a, b in pairwise(self._default(dtime, steps)):
Line 51:
Line 52: @permutations(_PARAMS)
Line 53: def test_downtime_increasing(self, dtime, steps):
Line 54: for a, b in pairwise(self._default(dtime, steps)):
Line 55: self.assertTrue(a <= b)
can be rewritten as self.assertGreater(b, a)
Line 56:
Line 57: @permutations(_PARAMS)
Line 58: def test_exponential_dowtime_never_zero(self, dtime, steps):
Line 59: for dt in self._default(dtime, steps):
Line 56:
Line 57: @permutations(_PARAMS)
Line 58: def test_exponential_dowtime_never_zero(self, dtime, steps):
Line 59: for dt in self._default(dtime, steps):
Line 60: self.assertTrue(dt > 0)
can be rewritten as self.assertGreater(dt, 0)
Line 61:
Line 62: @permutations(_PARAMS)
Line 63: def test_exponential_downtime_is_lower(self, dtime, steps):
Line 64: # it's OK if exponential starts a little higher than linear...
Line 69:
Line 70: # ...but what matters is that after that, it stays lower.
Line 71: for i, (a, b) in enumerate(zip(exp[1:], lin[1:])):
Line 72: msg = 'step=%i/%i exp=%f lin=%f' % (i+1, steps, a, b)
Line 73: self.assertTrue(a <= b, msg)
again, self assertGreater(b, a, msg)
Line 74:
Line 75: @permutations(_PARAMS)
Line 76: def test_exponential_same_end_value(self, dtime, steps):
Line 77: exp = self._default(dtime, steps)
Line 107: def pairwise(iterable):
Line 108: "s -> (s0,s1), (s1,s2), (s2, s3), ..."
Line 109: a, b = tee(iterable)
Line 110: next(b, None)
Line 111: return izip(a, b)
thought about this a bit, wouldn't importing islice and running izip(a, islice(b, 1, None)) be a bit nicer?
Line 112:
Line 113:
Line 114: def _linear_downtime(downtime, steps):
Line 115: "this is the old formula as reference"
--
To view, visit http://gerrit.ovirt.org/25820
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6401772f52ea28144452e67198bddff18f6703eb
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Martin Betak <mbetak at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list