Change in vdsm[master]: vm: migration: exponential downtime increment
danken at redhat.com
danken at redhat.com
Mon Sep 15 10:48:43 UTC 2014
Dan Kenigsberg has posted comments on this change.
Change subject: vm: migration: exponential downtime increment
......................................................................
Patch Set 7: Code-Review-1
(6 comments)
http://gerrit.ovirt.org/#/c/25820/7/tests/vmMigrationTests.py
File tests/vmMigrationTests.py:
Line 38: yield downtime * (i + 1) / steps
Line 39:
Line 40:
Line 41: class TestVmMigrationDowntimeSequence(TestCaseBase):
Line 42: # defaults as per config
read them from config, instead of having this comment and duplicating the value.
Line 43: _DOWNTIME = 500
Line 44:
Line 45: _STEPS = 10
Line 46:
Line 61: self.assertTrue(len(self._default) >= 2)
Line 62:
Line 63: def test_downtime_increasing(self):
Line 64: for a, b in pairwise(self._default):
Line 65: self.assertTrue(a <= b)
assertLessEqual() etc gives more info upon failure - please use more specific asserts when possible.
Line 66:
Line 67: def test_exponential_dowtime_never_zero(self):
Line 68: for dt in self._default:
Line 69: self.assertTrue(dt > 0)
Line 78: self.assertTrue(a <= b)
Line 79:
Line 80: def test_exponential_same_end_value(self):
Line 81: self.assertAlmostEqual(self._default[-1], self._linear[-1],
Line 82: delta=self._DELTA)
You'd also want to assert that the final step is the maximum allow downtime.
http://gerrit.ovirt.org/#/c/25820/7/vdsm/virt/migration.py
File vdsm/virt/migration.py:
Line 354: raise
Line 355:
Line 356:
Line 357: def exponential_downtime(downtime, steps):
Line 358: offset = downtime / steps
Let's do a float division on Python2, too.
Line 359: base = (downtime - offset) ** (1 / float(steps))
Line 360:
Line 361: for i in range(1, steps+1):
Line 362: yield offset + int(base ** i)
Line 357: def exponential_downtime(downtime, steps):
Line 358: offset = downtime / steps
Line 359: base = (downtime - offset) ** (1 / float(steps))
Line 360:
Line 361: for i in range(1, steps+1):
use spaces around the + sign.
Line 362: yield offset + int(base ** i)
Line 363:
Line 364:
Line 365: class DowntimeThread(threading.Thread):
Line 358: offset = downtime / steps
Line 359: base = (downtime - offset) ** (1 / float(steps))
Line 360:
Line 361: for i in range(1, steps+1):
Line 362: yield offset + int(base ** i)
> Minor: I'd go with yield int(offset + base ** i) in order to guarantee the
+1, particularly since on Python 3, (downtime / steps) may be in ℝ∖ℕ, anyway.
Line 363:
Line 364:
Line 365: class DowntimeThread(threading.Thread):
Line 366: DOWNTIME_STEPS = config.getint('vars', 'migration_downtime_steps')
--
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: 7
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