Francesco Romani has posted comments on this change. Change subject: scale: limit cpu usage using cpu-affinity ......................................................................
Patch Set 18: (7 comments) https://gerrit.ovirt.org/#/c/45738/18/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 649 Line 650 Line 651 Line 652 Line 653 > Add comment about importance of order; taskset must be added after sudo so Right, we were biten once, we don't want to be biten again. Line 625: def __del__(self): Line 626: self._poller.close() Line 627: Line 628: Line 629: _ANY_CPU = ["0-%d" % (os.sysconf('SC_NPROCESSORS_CONF') - 1)] > We were using only online cpus in cpopen, and we can use it here with "SC_N ok, makes sense to me. Line 630: Line 631: Line 632: _USING_CPU_AFFINITY = config.get('vars', 'cpu_affinity') != "" Line 633: https://gerrit.ovirt.org/#/c/45738/18/tests/testlib.py File tests/testlib.py: Line 466: Line 467: return wrapper Line 468: Line 469: Line 470: # we expect that on devel/test boxes there are no offline cpu > Use sysconf to get the online cpus instead of depending on multiprocessing Done Line 471: def all_active_cpus(): https://gerrit.ovirt.org/#/c/45738/18/tests/utilsTests.py File tests/utilsTests.py: Line 526: resetCpuAffinity=True) Line 527: Line 528: self.assertEquals(taskset.get(proc.pid), Line 529: all_active_cpus()) Line 530: proc.kill() > Use try finally so the process is kill when the test fail. Done Line 531: Line 532: Line 533: class ExecCmdStressTest(TestCaseBase): Line 534: Line 528: self.assertEquals(taskset.get(proc.pid), Line 529: all_active_cpus()) Line 530: proc.kill() Line 531: Line 532: > Missing test that we do not change cpu affinity when _USING_CPU_AFFINITY is Right. Added. Line 533: class ExecCmdStressTest(TestCaseBase): Line 534: Line 535: CONCURRENCY = 50 Line 536: FUNC_DELAY = 0.01 https://gerrit.ovirt.org/#/c/45738/18/vdsm/vdsm File vdsm/vdsm: Line 164: sysname, nodename, release, version, machine = os.uname() Line 165: log.info('(PID: %s) I am the actual vdsm %s %s (%s)', Line 166: pid, dsaversion.raw_version_revision, nodename, release) Line 167: Line 168: __set_cpu_affinity(log) > Passing the log is ugly, we should use getLogger in functions, or set the l OK, but fixed because I need another upload anyway. Line 169: Line 170: serve_clients(log) Line 171: except: Line 172: log.error("Exception raised", exc_info=True) Line 263: cpu_affinity = config.get('vars', 'cpu_affinity') Line 264: if cpu_affinity: Line 265: cpu_list = [cpu.strip() for cpu in cpu_affinity.split(',')] Line 266: taskset.set(os.getpid(), cpu_list, all_tasks=True) Line 267: log.info('VDSM configured with cpu_affinity: %s', cpu_list) > Better log before you make the change. This makes errors more clear - you s good point, fixed. Line 268: Line 269: Line 270: def main(): Line 271: __assertVdsmUser() -- To view, visit https://gerrit.ovirt.org/45738 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984 Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: Yaniv Kaul <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
