Nir Soffer has posted comments on this change. Change subject: virt: stats: make compute_latency more robust ......................................................................
Patch Set 9: (2 comments) https://gerrit.ovirt.org/#/c/50593/9/vdsm/virt/vmstats.py File vdsm/virt/vmstats.py: Line 325: last_sample, last_index, mode): Line 326: operations = ( Line 327: last_sample['block.%d.%s.reqs' % (last_index, mode)] - Line 328: first_sample['block.%d.%s.reqs' % (first_index, mode)] Line 329: ) I hate this style - please avoid it. How about: first = 'block.%d.%s.reqs' % (last_index, mode) last = 'block.%d.%s.reqs' % (first_index, mode) operations = last_sample[last] - first_sample[first] Are you sure that last_index and first_index are not always the same? This would make it even much nicer: key = 'block.%d.%s.reqs' % (index, mode) operations = last_sample[key] - first_sample[key] Line 330: if not operations: Line 331: return 0 Line 332: elapsed_time = ( Line 333: last_sample['block.%d.%s.times' % (last_index, mode)] - Line 344: ('writeLatency', 'wr'), Line 345: ('flushLatency', 'fl')): Line 346: try: Line 347: stats[name] = str(_compute_latency(first_sample, first_index, Line 348: last_sample, last_index, mode)) I commented (maybe on another patch) about separating lookup (with KeyError) and setting the result. for ... try: value = ... except KeyError: continue stats[name] = str(value) Line 349: except KeyError: Line 350: pass Line 351: Line 352: return stats -- To view, visit https://gerrit.ovirt.org/50593 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie4670309d48bd65a3afaba8124407598b76d808d Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@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: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
