Francesco Romani has posted comments on this change. Change subject: periodic: explicitely track domain availability ......................................................................
Patch Set 5: (4 comments) https://gerrit.ovirt.org/#/c/47246/5//COMMIT_MSG Commit Message: Line 27: We need to try another way to solve this problem, without Line 28: depend on libvirt so much. Line 29: Line 30: Now VDSM counts explicitely the uses of a given domain, using Line 31: a ExpiringCache per operation, in order to minimize the > an/one ExpiringCache ..., in ... Done Line 32: inter-operation interference. We want to avoid false negative Line 33: as top-priority, but once this is granted, we also want to minimize Line 34: the false positive! Line 35: Line 30: Now VDSM counts explicitely the uses of a given domain, using Line 31: a ExpiringCache per operation, in order to minimize the Line 32: inter-operation interference. We want to avoid false negative Line 33: as top-priority, but once this is granted, we also want to minimize Line 34: the false positive! > I'd suggest defining false negative (VM available, considered unavailable?) Good point, will do. Line 35: Line 36: The rational for this change is Line 37: - isDomainReadyForCommands, which we used previously, needs Line 38: o access libvirt, thus can actually make things worse on extreme Line 34: the false positive! Line 35: Line 36: The rational for this change is Line 37: - isDomainReadyForCommands, which we used previously, needs Line 38: o access libvirt, thus can actually make things worse on extreme > to access ... Done Line 39: load conditions. Line 40: - under normal load, reduce the interference and false positives Line 41: between operations Line 42: https://gerrit.ovirt.org/#/c/47246/5/vdsm/virt/periodic.py File vdsm/virt/periodic.py: Line 229: continue Line 230: Line 231: op = self._create(vm_obj, self._skip_doms) Line 232: Line 233: if not op.required: > Shouldn't vm_id be added to `skipped' here? No, because "required" means: "should this operation be done at all on this vm?" if an operation is "required", then we must check if an operation is runnable, so if the current state of the VM allows so. "required" it is supposed to change much less frequently than "runnable", and it is not affected by libvirt congestion. "skipped", lastly, means "this operation is required, but it is not runnable on this vm now, so let's track it". Hence, it is correct NOT to add a not required operation. Line 234: continue Line 235: # When dealing with blocked domains, we also want to avoid Line 236: # to pile up jobs that libvirt can't handle and eventually Line 237: # clog it. -- To view, visit https://gerrit.ovirt.org/47246 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd90750ab36cb90bad401fab0ff7ff98429e78a5 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
