Francesco Romani has posted comments on this change. Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 15: Code-Review+2 (3 comments) good enough, raising score. There are a couple of places I'm not entirely happy, but 1. not worth to stall this patch once more for them 2. this hoststats.py code needs more work in unrelated topic anyway, so let's just have this patch. https://gerrit.ovirt.org/#/c/46269/15/tests/hoststatsTests.py File tests/hoststatsTests.py: Line 114: 'cpuUser': '0.00', Line 115: 'nodeIndex': 0 Line 116: } Line 117: Line 118: _core_one_stats = { this is used exactly once, it seems, so it could be inlined, but I don't really mind. Line 119: 'cpuIdle': '100.00', Line 120: 'cpuSys': '0.00', Line 121: 'cpuUser': '0.00', Line 122: 'nodeIndex': 1 Line 138: with MonkeyPatchScope([(caps, 'getNumaTopology', Line 139: self._fakeNumaTopology)]): Line 140: result = hoststats._get_cpu_core_stats(first_sample, last_sample) Line 141: self.assertEqual(len(result), 2) Line 142: self.assertEqual(result['0'], self._core_zero_stats) inlining this makes each test probably a bit clearer, but I prefer to avoid duplication. So let's keep it this way. Line 143: self.assertEqual(result['1'], self._core_one_stats) Line 144: Line 145: def testSkipStatsOnMissingFirstSample(self): Line 146: cpu_sample = {'user': 1.0, 'sys': 2.0} https://gerrit.ovirt.org/#/c/46269/15/vdsm/virt/hoststats.py File vdsm/virt/hoststats.py: Line 113: for cpu_core in cpu_cores: Line 114: try: Line 115: user_cpu_usage = compute_cpu_usage(cpu_core, 'user') Line 116: system_cpu_usage = compute_cpu_usage(cpu_core, 'sys') Line 117: except MissingSample: This is a translation of exception in disguise :) But here I don't mind, and this code needs more work anyway (unrelated from this patch), so let's keep it. Line 118: # Only collect data when all required samples already present Line 119: continue Line 120: core_stat = { Line 121: 'nodeIndex': int(node_index), -- To view, visit https://gerrit.ovirt.org/46269 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9c247f9138e02a9230a0849a04cb2e1705e7fac Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Roman Mohr <rm...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Omer Frenkel <d.mosqu...@gmail.com> Gerrit-Reviewer: Roman Mohr <rm...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@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
