Hi everyone,
I'd like to gather some thoughts about this change http://gerrit.ovirt.org/#/c/23138/5
Sorry for the long mail, but the scenario is not trivial to explain and I tried to achieve a decent compromise between terseness and completeness.
Context -------
Everything starts from the bugzilla https://bugzilla.redhat.com/866484 .
Summary of the bug: when a blockInfo call to libvirt goes in timeout, the status of the affected VM start to bounce in the engine between 'Non Responsive' and 'Up'. This is because the responsiveness of a VM is tracked using a single flag, updated each after libvirt call, and the flag could be overwritten behind the back of the engine (between two getStats). In a nutshell, the responsiveness of a VM could observed in a different way depending on the time sequence of the updates.
It is worth to mention that, I *cannot* yet reproduce in the terms described in the bug description. Part of the reason why I failed to reproduce it is a particular event sequence is required to achieve the bouncing effect described in the bug. The event sequence is in turn determined by the timing of the block storage timeout and by the time the engine queries the affected VDSM.
However, my understanding is that the scenario which emerges from the bug description and the analysis of the logs attached to the bug is still relevant, although somewhat unlikely. This is the reason why I submitted the change http://gerrit.ovirt.org/#/c/23138/5
The issue with the new patch ----------------------------
The new code determines the responsiveness of the monitor -and thus of the VM- in a `pessimistic` way, meaning that now as long as at least one call to libvirt fails, the VM is deemed unresponsive.
On the other hand, with the current code, if an attribute is unresponsive but if the sequence of events is "good" enough, the not responsive time interval could be hidden from the engine's sight. Or, of course, the engine could see the bouncing we're discussing.
In a nutshell, the new code could be too pessimistic in reporting the state. A VM could be reported not responsive longer and more often (again it all depends on the event sequence) than before. It that is the case, the problem is: how to fix the reporting in a more "optimistic" way?
Possible solution -----------------
While reviewing the change with Michal, we found that partitioning the set of the calls to be monitored in critical/not critical could be a way:
* critical libvirt calls: if at least one fails for timeout, and while it fails for timeout, the VM is considered Unresponsive. * not critical: a timeout is logged but ignored to decide the responsiveness of the VM
Would that be an improvement with respect to the current state and, possible, a viable long-term solution?
Any advice will be greatly appreciated.
Thanks and regards,
On Wed, Jan 22, 2014 at 05:49:11AM -0500, Francesco Romani wrote:
Hi everyone,
I'd like to gather some thoughts about this change http://gerrit.ovirt.org/#/c/23138/5
Sorry for the long mail, but the scenario is not trivial to explain and I tried to achieve a decent compromise between terseness and completeness.
Context
Everything starts from the bugzilla https://bugzilla.redhat.com/866484 .
Summary of the bug: when a blockInfo call to libvirt goes in timeout, the status of the affected VM start to bounce in the engine between 'Non Responsive' and 'Up'. This is because the responsiveness of a VM is tracked using a single flag, updated each after libvirt call, and the flag could be overwritten behind the back of the engine (between two getStats). In a nutshell, the responsiveness of a VM could observed in a different way depending on the time sequence of the updates.
It is worth to mention that, I *cannot* yet reproduce in the terms described in the bug description. Part of the reason why I failed to reproduce it is a particular event sequence is required to achieve the bouncing effect described in the bug. The event sequence is in turn determined by the timing of the block storage timeout and by the time the engine queries the affected VDSM.
Pardon my skeptism, but a bug that was opened with "How reproducible: 100%" and now cannot be reproduced at all smells like a misunderstood bug.
However, my understanding is that the scenario which emerges from the bug description and the analysis of the logs attached to the bug is still relevant, although somewhat unlikely. This is the reason why I submitted the change http://gerrit.ovirt.org/#/c/23138/5
The issue with the new patch
The new code determines the responsiveness of the monitor -and thus of the VM- in a `pessimistic` way, meaning that now as long as at least one call to libvirt fails, the VM is deemed unresponsive.
On the other hand, with the current code, if an attribute is unresponsive but if the sequence of events is "good" enough, the not responsive time interval could be hidden from the engine's sight. Or, of course, the engine could see the bouncing we're discussing.
In a nutshell, the new code could be too pessimistic in reporting the state. A VM could be reported not responsive longer and more often (again it all depends on the event sequence) than before.
Indeed, this issue is crippling. For example, if you fire up migration, and due to a transient hickup, libvirt times out, we would get into a dead end. The attribute for "migrate" would be set to -1, the VM would be reported as non-responsive, and no future migrate command would ever be sent to free it.
It that is the case, the problem is: how to fix the reporting in a more "optimistic" way?
Possible solution
While reviewing the change with Michal, we found that partitioning the set of the calls to be monitored in critical/not critical could be a way:
- critical libvirt calls: if at least one fails for timeout, and while it fails for timeout, the VM is considered Unresponsive.
- not critical: a timeout is logged but ignored to decide the responsiveness of the VM
Would that be an improvement with respect to the current state and, possible, a viable long-term solution?
I like the idea of partitioning, but "criticality" is not a clear indicator imo. monitorResponse was introduced as a heuristical answer to the question "would qemu accept an upcoming command and act upon it in a timely manner?"
A libvirt.virDomain method that succeeds despite its qemu process being SIGSTOPped, should not be considered when setting monitorResponse. I we undestand what has reset monitorResponse in Dafna's scenerio, we could avoid wrapping it with _timeoutExperienced.
Dan.
vdsm-devel@lists.fedorahosted.org