Francesco Romani has uploaded a new change for review.
Change subject: WIP: statscache: support multiple removals ......................................................................
WIP: statscache: support multiple removals
There is a still obscure bug that causes multiple removals from cache, and everyone but the first will fail (as expected).
This patch handles this case, until either we pinpoint the bug, or the actual need for supporting multiple removals.
Change-Id: I1748cba3646341d2be799aaf30b81432b009359c Signed-off-by: Francesco Romani fromani@redhat.com --- M vdsm/virt/sampling.py 1 file changed, 4 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/40439/1
diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py index bb2f462..455f49e 100644 --- a/vdsm/virt/sampling.py +++ b/vdsm/virt/sampling.py @@ -425,7 +425,10 @@ Remove any data from the cache related to the given VM. """ with self._lock: - del self._vm_last_timestamp[vmid] + ts = self._vm_last_timestamp.pop(vmid, None) + + if ts is None: + self._log.warn('timestamp for vm %s already removed', vmid)
def get(self, vmid): """
automation@ovirt.org has posted comments on this change.
Change subject: WIP: statscache: support multiple removals ......................................................................
Patch Set 1:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: WIP: statscache: support multiple removals ......................................................................
Patch Set 1:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18439/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: WIP: statscache: support multiple removals ......................................................................
Patch Set 1:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1670/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: WIP: statscache: support multiple removals ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/1670/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/18439/ : SUCCESS
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: support multiple removals ......................................................................
Patch Set 2:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 3:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 4:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 4:
jenkins failure unrelated:
15:12:35 CalledProcessError: Command '['/usr/sbin/brctl', 'delbr', 'vdsmtest-Kk3l7']' returned non-zero exit status 1
Francesco Romani has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 4: Verified+1
verified running patched VDSM, mostly migrations back and forth, issuing various combinations of VM shutdown/destroy, and with the tests.
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 5:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 6: Code-Review-1
(1 comment)
https://gerrit.ovirt.org/#/c/40439/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py:
Line 426: """ Line 427: with self._lock: Line 428: ts = self._vm_last_timestamp.pop(vmid, None) Line 429: Line 430: if ts is None: I'd prefer to follow my religion of catching it in the highest possible point - on releaseVM(). Line 431: self._log.warn('timestamp for vm %s already removed', vmid) Line 432: return ts # for testing purposes. Should be ignored elsewhere. Line 433: Line 434: def get(self, vmid):
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 7: Code-Review+2
Francesco Romani has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 7: Verified+1
re-verified migrating a VM back and forth an handful of times, and trying various shutdowns.
Dan Kenigsberg has submitted this change and it was merged.
Change subject: sampling: statscache: more robust remove() ......................................................................
sampling: statscache: more robust remove()
It is possible that StatsCache.remove() is called for a VM not yet added to the cache. This would move VDSM into an inconsistent state, possibly preventing to re-run the same VM. This is obviously very bad and must be avoided.
A prominent example of this is if a vmMigrationCreate succeeds, but the asynchronous libvirt domain startup fails. The source VDSM would correctly issue destroy(), but the stats cache removal on the destination would fail.
It must be noted that in some still-unclear circumstances, VDSM receives multiple destroy requests. VDSM reacts correctly to those requests, except for the fact that this causes multiple removals from stats_cache, and everyone but the first would fail (as expected).
This patch - makes StatsCache.remove() capable of supportting removal of net-yet-added VMs - adds log warnings to make sure these events will not hide more bugs, like the multiple removal due to the failure to create underlying libvirt domain.
Change-Id: I1748cba3646341d2be799aaf30b81432b009359c Signed-off-by: Francesco Romani fromani@redhat.com Reviewed-on: https://gerrit.ovirt.org/40439 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/virt/vm.py 1 file changed, 7 insertions(+), 1 deletion(-)
Approvals: Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified
automation@ovirt.org has posted comments on this change.
Change subject: sampling: statscache: more robust remove() ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org