Francesco Romani has uploaded a new change for review. Change subject: periodic: explicitely track domain availability ......................................................................
periodic: explicitely track domain availability The periodic sampling code knows that libvirt can become clogged (most common case: unresponsive storage) and tries to not make things worse, avoiding to schedule work on unresponding domain. To do so, the code needs a way to know or at least guesstimate the libvirt responsiveness. The initial implementation relied on virDomainGetControlInfo, which reports if there is any monitor job currently running, or not. This approach has the plus to have one single source of information, which is also the most reliable - libvirt. But it also creates a a dangerous short circuit, because the controller relies on the controlled itself to assess its health. Furthermore, we experienced firsthand a bad failure of the aforementioned control code under high scale, and this was the final blow to the initial approach. We need to try another way to solve this problem, without depend on libvirt so much. Now VDSM counts explicitely the uses of a given domain, using a ExpiringCache per operation, in order to minimize the inter-operation interference. We want to avoid false negative as top-priority, but once this is granted, we also want to minimize the false positive! The rational for this change is - isDomainReadyForCommands, which we used previously, needs o access libvirt, thus can actually make things worse on extreme load conditions. - under normal load, reduce the interference and false positives between operations Change-Id: Idd90750ab36cb90bad401fab0ff7ff98429e78a5 Bug-Url: https://bugzilla.redhat.com/1250839 Signed-off-by: Francesco Romani <from...@redhat.com> --- M tests/periodicTests.py M vdsm/virt/periodic.py M vdsm/virt/sampling.py 3 files changed, 23 insertions(+), 31 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/47246/1 diff --git a/tests/periodicTests.py b/tests/periodicTests.py index ce4d592..1d730b7 100644 --- a/tests/periodicTests.py +++ b/tests/periodicTests.py @@ -241,8 +241,9 @@ VMS = defaultdict(int) - def __init__(self, vm): + def __init__(self, vm, skip_doms): self._vm = vm + self._skip_doms = skip_doms @property def required(self): @@ -250,26 +251,19 @@ raise ValueError('required failed') return True - @property - def runnable(self): - if getattr(self._vm, 'fail_runnable', False): - raise ValueError('runnable failed') - return True - def __call__(self): - _Visitor.VMS[self._vm.id] += 1 + vm_id = self._vm.id + self._skip_doms[vm_id] = True + _Visitor.VMS[vm_id] += 1 + del self._skip_doms[vm_id] class _Nop(object): - def __init__(self, _): + def __init__(self, *args): pass @property def required(self): - return True - - @property - def runnable(self): return True def __call__(self): diff --git a/vdsm/virt/periodic.py b/vdsm/virt/periodic.py index 13d7632..9c05f30 100644 --- a/vdsm/virt/periodic.py +++ b/vdsm/virt/periodic.py @@ -31,6 +31,7 @@ from . import sampling from . import virdomain +from .utils import ExpiringCache # just a made up number. Maybe should be equal to number of cores? @@ -215,6 +216,7 @@ self._executor = executor self._create = create self._timeout = timeout + self._skip_doms = ExpiringCache(ttl=sampling.TIMEOUT) def __call__(self): vms = self._get_vms() @@ -222,7 +224,11 @@ for vm_id, vm_obj in vms.iteritems(): try: - op = self._create(vm_obj) + to_skip = self._skip_doms.get(vm_id, False) + if to_skip: + continue + + op = self._create(vm_obj, self._skip_doms) if not op.required: continue @@ -233,9 +239,6 @@ # still OK if occasional misdetection occours, but we # definitely want to avoid known-bad situation and to # needlessly overload libvirt. - if not op.runnable: - skipped.append(vm_id) - continue except Exception: # we want to make sure to have VM UUID logged @@ -257,14 +260,13 @@ class _RunnableOnVm(object): - def __init__(self, vm): + def __init__(self, vm, skip_doms): self._vm = vm - - @property - def runnable(self): - return self._vm.isDomainReadyForCommands() + self._skip_doms = skip_doms def __call__(self): + vm_id = self._vm.id # keep reference for safety + self._skip_doms[vm_id] = True try: self._execute() except virdomain.NotConnectedError: @@ -272,7 +274,9 @@ # race on shutdown: next cycle won't pick up this VM. # both cases: let's reduce the log spam. self._vm.log.warning('could not run on %s: domain not connected', - self._vm.id) + vm_id) + finally: + del self._skip_doms[vm_id] def _execute(self): raise NotImplementedError @@ -298,12 +302,6 @@ @property def required(self): return self._vm.hasGuestNumaNode - - @property - def runnable(self): - # NUMA operations don't require QEMU monitor access - # (inspected libvirt sources v1.2.17) - return True def _execute(self): self._vm.updateNumaInfo() diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py index 1ad9d46..6f8ecce 100644 --- a/vdsm/virt/sampling.py +++ b/vdsm/virt/sampling.py @@ -501,12 +501,12 @@ # a multiple of known timeout and period: # - vm sampling period is 15s (we control that) # - libvirt (default) qemu monitor timeout is 30s (we DON'T contol this) -_TIMEOUT = 40.0 +TIMEOUT = 40.0 class VMBulkSampler(object): def __init__(self, conn, get_vms, stats_cache, - stats_flags=0, timeout=_TIMEOUT): + stats_flags=0, timeout=TIMEOUT): self._conn = conn self._get_vms = get_vms self._stats_cache = stats_cache -- To view, visit https://gerrit.ovirt.org/47246 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idd90750ab36cb90bad401fab0ff7ff98429e78a5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
