Francesco Romani has posted comments on this change. Change subject: periodic: explicitely track domain availability ......................................................................
Patch Set 5: > - If period > sampling.TIMEOUT then this change is no-op. > What happens in such a case on the next operation calls? It is implicitely enforced by the choice of default timeouts. I had a patch to make the choice of timeout more robust, but got lost, I need to restore it and add it in queue for 3.6.1. > Just harmless exceptions, or unnecessarily repeated operation > attempts when the period is short, or even problem escalation, > or something else? Perhaps consistent with the declared > "no false negatives" strategy, but it should be appropriately explained. If timeout is too short (less than one period), there is no clogging protection, so all the good intentions go down the sink :\ > - The only case handled by the change is if op > period & period < > sampling.TIMEOUT. > False negatives possible, but at most for sampling.TIMEOUT minus op. > Again, maybe worth a little note? Will add > If I don't understand it right then explanation is especially needed! I agree with this sentiment, not sure how to convey all the context needed to properly understand this change. > Moreover, adding comment why we still insist on trying different periodic > operations > in case another kind of operation blocks us would be useful (as it seems to > be new > behavior introduced by this change). Correct, in the sense that we have interference between operations. The periodic operations are not independent from each other, really. They seems so in VDSM, but we have one shared resource (domain) on which each one insists. So it is always possible that one operation on one domain stalls the others acting on the same domain (this was true even before when we used one thread per VM). The problem here is that operations should not stall unrelated domains. -- 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: No _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
