Change in vdsm[master]: virt: sampling: omit stacktrace if no cpu stats

fromani at redhat.com fromani at redhat.com
Fri Jun 19 16:08:20 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: virt: sampling: omit stacktrace if no cpu stats
......................................................................


Patch Set 2:

(3 comments)

https://gerrit.ovirt.org/#/c/42476/2//COMMIT_MSG
Commit Message:

Line 5: CommitDate: 2015-06-19 11:04:49 +0200
Line 6: 
Line 7: virt: sampling: omit stacktrace if no cpu stats
Line 8: 
Line 9: Cpu stats may be temporarily unavailable.
> could you elaborate on when they are unavailable, but we should not really 
in general, the bulk stats will skip groups that cannot be gathered (e.g.  cpu stats) and will raise error only in presence of critical errors, e.g. memory allocation failure, or timeout.

So it is safer and better not assume about the availability of groups.

For example the cgroups bug we faced not long ago (https://bugzilla.redhat.com/show_bug.cgi?id=1139223) may trigger this misbehaviour.

(will add the above to commit message in the next upload)
Line 10: Unrecoverable errors are rare. However,
Line 11: in any case a warning is sufficient, and
Line 12: a stack trace nowadays only adds noise
Line 13: and confusion.


https://gerrit.ovirt.org/#/c/42476/2/vdsm/virt/vmstats.py
File vdsm/virt/vmstats.py:

Line 80:             'invalid interval %i when computing CPU stats for %s',
Line 81:             interval, vm.id)
Line 82:         return
Line 83: 
Line 84:     try:
> it would be cleaner and safer to check precisely for the keys that may not 
Libvirt uses a cgroup to do this accounting. The case I'm looking to guard against is when all of the cpu.* below stats are missing.

If, let's say, _only_ cpu.user is missing, something really wrong happenend inside libvirt.

In another patch, I will check for the availability of the three keys we use in this function, and remove the try/catch block.
Line 85:         stats['cpuUsage'] = str(last_sample['cpu.system'] +
Line 86:                                 last_sample['cpu.user'])
Line 87: 
Line 88:         stats['cpuSys'] = _usage_percentage(


Line 94:             - _diff(last_sample, first_sample, 'cpu.user')
Line 95:             - _diff(last_sample, first_sample, 'cpu.system'),
Line 96:             interval)
Line 97: 
Line 98:     except KeyError as e:
> nit: e is not used
will remove
Line 99:         logging.warning("CPU stats not available for %s", vm.id)
Line 100: 
Line 101: 
Line 102: def balloon(vm, stats, sample):


-- 
To view, visit https://gerrit.ovirt.org/42476
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I894a0ace8e3dce3cf3399ed71942e41f719133fb
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list