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