Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/48409
to review the following change.
Change subject: periodic: make VmDispatcher ignore TooManyTasks
......................................................................
periodic: make VmDispatcher ignore TooManyTasks
VmDispatcher is meant to make N independent operations
from one (VM bulk sampling), being N the number of active VMs.
To fulfil this claim, if one of the N independent calls fails with
TooManyTasks, we go on attempting running the rest of the calls, even
though it is highly likely that once one TooManyTasks error is
encountered, many subsequent dispatch() attempts would fail.
Backport-To: 3.6
Change-Id: Iad2d186327f8607aaeb41ca132a9a8a0806869cf
Signed-off-by: Francesco Romani <from...@redhat.com>
Reviewed-on: https://gerrit.ovirt.org/41657
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <dan...@redhat.com>
---
M tests/periodicTests.py
M vdsm/virt/periodic.py
2 files changed, 44 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/48409/1
diff --git a/tests/periodicTests.py b/tests/periodicTests.py
index b73ed72..ce4d592 100644
--- a/tests/periodicTests.py
+++ b/tests/periodicTests.py
@@ -209,6 +209,22 @@
for vm_id in failed_ids:
self.assertEqual(_Visitor.VMS.get(vm_id), None)
+ def test_dispatch_fails(self):
+ """
+ make sure that VmDispatcher attempts to dispatch work
+ for every registered VMs, and doesn't exit prematurely
+ when one dispatch() fails.
+ """
+ exc = _FakeExecutor(fail=True)
+
+ op = periodic.VmDispatcher(
+ self.cif.getVMs, exc, _Nop, 0)
+
+ skipped = op()
+
+ self.assertEqual(set(skipped),
+ set(self.cif.getVMs().keys()))
+
def _make_fake_vms(self):
for i in range(VM_NUM):
vm_id = _fake_vm_id(i)
@@ -244,10 +260,32 @@
_Visitor.VMS[self._vm.id] += 1
+class _Nop(object):
+ def __init__(self, _):
+ pass
+
+ @property
+ def required(self):
+ return True
+
+ @property
+ def runnable(self):
+ return True
+
+ def __call__(self):
+ pass
+
+
class _FakeExecutor(object):
+ def __init__(self, fail=False):
+ self._fail = fail
+
def dispatch(self, func, timeout):
- func()
+ if self._fail:
+ raise executor.TooManyTasks()
+ else:
+ func()
# fake.VM is a quite complex beast. We need only the bare minimum here,
diff --git a/vdsm/virt/periodic.py b/vdsm/virt/periodic.py
index 632674f..fb96402 100644
--- a/vdsm/virt/periodic.py
+++ b/vdsm/virt/periodic.py
@@ -246,11 +246,15 @@
self._log.exception("while dispatching %s to VM '%s'",
self._create, vm_id)
else:
- self._executor.dispatch(op, self._timeout)
+ try:
+ self._executor.dispatch(op, self._timeout)
+ except executor.TooManyTasks:
+ skipped.append(vm_id)
if skipped:
self._log.warning('could not run %s on %s',
self._create, skipped)
+ return skipped # for testing purposes
def __repr__(self):
return 'VmDispatcher(%s)' % self._create
--
To view, visit https://gerrit.ovirt.org/48409
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iad2d186327f8607aaeb41ca132a9a8a0806869cf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
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/mailman/listinfo/vdsm-patches
