Francesco Romani has posted comments on this change. Change subject: host stats: Collect stats from online cpu cores only ......................................................................
Patch Set 6: (3 comments) https://gerrit.ovirt.org/#/c/46269/6//COMMIT_MSG Commit Message: Line 26: 'nodeIndex': 0}, Line 27: '1': {'cpuIdle': '96.07', Line 28: 'cpuSys': '0.47', Line 29: 'cpuUser': '3.46', Line 30: 'nodeIndex': 0}} > After thinking about it, I am not for the solution where we have offline cp 1. right, but nowadays are consistent as side effect, because the getVdsStats is either correct or completely absent (due to the error). How does VDSM handle the case on which we start with one offline cpu? we have an hole in the sequence, right? (like, with cpu #2 offline: 0, 1, 3, 4, 5, 6, 7) 2. see above 3. Yes, if everything works (= all cpus online), otherwise I'm suspecting we have (and always had) a silent breakage. If backward compatibility requires to have all fields to be present, we can have placeholder data that Engine can process. 4. ok You're right to raise backward compatibility concerns, but we should also take in account that if a cpu is put offline, it just not works (tm) :) Line 31: [...] Line 32: Line 33: When only one CPU out of two has been online before libvird was Line 34: started, vdsm reports: https://gerrit.ovirt.org/#/c/46269/6/vdsm/virt/sampling.py File vdsm/virt/sampling.py: Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue > I don't think cpu becoming offline or online is exceptional. It is may be u Nir:L Yep. I wrote "exceptional", I meant "unusual". Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'), Line 693: for cpu_core in cpu_cores: Line 694: # Do not try to collect cpu core data when no samples are present Line 695: if (not last_sample.cpuCores.getCoreSample(cpu_core) or not Line 696: first_sample.cpuCores.getCoreSample(cpu_core)): Line 697: continue > > Besides that, why you'd like to avoid exceptions? code style, code clarit Roman: I kinda disagree, but I see your point and since I can't bring an objection different from personal taste, I wont' object with this approach. Line 698: core_stat = { Line 699: 'nodeIndex': int(node_index), Line 700: 'cpuUser': compute_cpu_usage(cpu_core, 'user'), Line 701: 'cpuSys': compute_cpu_usage(cpu_core, 'sys'), -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Roman Mohr <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roman Mohr <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
