Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-06-15
08:20:45 sean-k-mooney ok that should work
08:20:48 sean-k-mooney ill read the rest
08:20:52 gibi sean-k-mooney: I cache the result of the _numa_fit_instance_cell for host_cell instance_cell pairs
08:21:15 sean-k-mooney yep pairs are fine to cache
08:22:25 sean-k-mooney that break will jsut break the inner loop right
08:22:55 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/845896/1/nova/virt/hardware.py#2376
08:23:14 gibi yepp break jumps out from the inner loop
08:23:34 sean-k-mooney then i dont see why this would not work
08:23:34 gibi if you look at the except: branch it uses there too with the same effect
08:23:59 sean-k-mooney and its just storing pairs of ints so the memory overhead will be trivial
08:24:20 gibi yepp it is len(host_cells) * len(instance_cells) * 2 integeres
08:24:52 gibi that is a lot lest than then n!/(n-k)! number of fit calls the algo made
08:25:01 sean-k-mooney so like with comptueing factorial this shoudl reduce to the cell checks to linear
08:25:09 sean-k-mooney form O(n^2)
08:25:20 gibi it is actuall O(n!/(n-k)!)
08:25:34 gibi I mean it was
08:25:38 sean-k-mooney ya just realised that it much large then n^2
08:26:09 sean-k-mooney its n choose m
08:26:29 gibi yeah we generate k long permutations from N items
08:26:51 sean-k-mooney so even with that optimiasation do we still want to change the default on mater
08:27:04 gibi sean-k-mooney: yepp, probably
08:27:17 sean-k-mooney with the sperad default it found a candiate on the first iteration
08:27:35 sean-k-mooney did you try thet repoducer with your chage
08:27:45 sean-k-mooney if not i was going to try that now
08:27:47 gibi the cache change helps with the forst case, the default change only helps in some case
08:28:13 gibi the reproduce took 6 mins for me without the cache, with the cache it took 3 seconds
08:28:33 gibi s/forts/worts/
08:28:34 sean-k-mooney nice
08:29:08 sean-k-mooney so still double spreaing but much much more reasonable
08:31:03 sean-k-mooney gibi: so other then a releasenote im not sure what elese to add to that patch.
08:31:19 sean-k-mooney im not sure we want to unit test using a cache
08:31:38 sean-k-mooney and fucntional test should not be able to tell the differnce
08:31:52 gibi yepp I hope functional will tell me that everything works as is
08:32:20 gibi I can add a unit test that shows that the cache works by mocking and counting the _numa_fit_instance_cell inner calls
08:32:27 sean-k-mooney they should i don tthink you have change the outcome in anyway
08:33:03 sean-k-mooney gibi i woudl only do that if you pull out that code into its own fucntion
08:33:13 sean-k-mooney that code bing the inner loop
08:33:47 sean-k-mooney i think this is too much of an implemation detail to asser for the top level numa_fit_instance_to_host fucntion as it is
08:34:01 opendevreview ribaudr proposed openstack/nova master: Allow unshelve to a specific host (Compute API part) https://review.opendev.org/c/openstack/nova/+/831507
08:34:02 opendevreview ribaudr proposed openstack/nova master: Allow unshelve to a specific host (REST API part) https://review.opendev.org/c/openstack/nova/+/845897
08:34:14 sean-k-mooney espically since its a function scoped cache
09:33:08 gibi yeah I dropped the unit test idea. after played around it a bit it seems very artifical
09:33:23 gibi I have to fix up some unit test case and I will add a reno and follow your inline suggestion
09:33:36 gibi the functional tests passed so I think it is correct
09:39:00 sean-k-mooney i would be interested to triger a white box run against it but we dont really have much numa testing with whitebox upstream
09:39:08 sean-k-mooney i think its a pretty safe optimisation
09:39:44 sean-k-mooney if you like i can try and find some time to try it on real hardware or simulated multi numa hardware at least
09:40:12 sean-k-mooney but if our func test pass i do trust them for most numa stff at this point
09:43:32 gibi I also feel pretty safe with this now
09:44:52 gibi I don't think we should spen much time manual testing it. I will ask the bug author to test it for us
09:45:05 gibi they have big hardware appareantly
09:53:41 sean-k-mooney ish
09:54:08 sean-k-mooney its not really that big given its a singel socket system and amd launched that chip about 3 years ago
09:54:48 sean-k-mooney there are more of them out in the wild then you might otherwise expect vexhost has a similar sku them but they do not expose all the numa nodes
09:55:11 sean-k-mooney its configurable in the bios
09:55:16 sean-k-mooney but sure
09:55:30 sean-k-mooney lets get the op to test and provide feedback
10:07:36 gibi ack
10:40:05 opendevreview Balazs Gibizer proposed openstack/nova master: Optimize numa_fit_instance_to_host https://review.opendev.org/c/openstack/nova/+/845896
10:40:11 gibi now with reno ^^
10:43:31 opendevreview Balazs Gibizer proposed openstack/nova stable/wallaby: Fix eventlet.tpool import https://review.opendev.org/c/openstack/nova/+/845838
11:02:11 gibi bauzas: fyi, reported a new gate failure https://bugs.launchpad.net/nova/+bug/1978817 I will push a fix soo
11:02:16 gibi it is not a blocker
11:02:26 gibi it only fails on slooow nodes
11:13:11 sean-k-mooney gibi other then formating it does not look like much changed in https://review.opendev.org/c/openstack/nova/+/845896/2/nova/tests/unit/virt/test_hardware.py
11:13:28 gibi sean-k-mooney: I needed to add id field for instance cells
11:13:36 sean-k-mooney oh just saw that
11:13:36 gibi (and yes I reformatted it :D)
11:13:56 sean-k-mooney ok ide make sense i guess
11:14:12 sean-k-mooney we need to modle the guest numa node that the object represents
11:14:26 sean-k-mooney and in real code it woudl always be set
11:14:32 sean-k-mooney so you are just fixing the test data
11:16:13 opendevreview Balazs Gibizer proposed openstack/nova master: Make test_wait_for_instance_event_* test time independent https://review.opendev.org/c/openstack/nova/+/845922
11:16:39 gibi sean-k-mooney: yepp it is just adding a more realistic test data
11:17:21 sean-k-mooney ok im +2 on that patch assuming zuul is happy this time
11:17:55 sean-k-mooney stephenfin: you proably know that code the best out of the remaining cores would you mind looking at that if you have time
11:18:18 sean-k-mooney if not artom your +1 and bauzas review would be nice
11:18:55 gibi yepp. I also asked for feedback from the bug reporter
11:47:06 artom sean-k-mooney, which one?
11:52:43 bauzas gibi: ack for the gate failure
11:52:56 gibi bauzas: since then I pushed the fix https://review.opendev.org/c/openstack/nova/+/845922
11:53:06 bauzas just saw it
11:53:26 gibi artom: I think sean-k-mooney refered to the numa scheduling perf optimization fix https://review.opendev.org/c/openstack/nova/+/845896
11:53:50 bauzas gibi: 1.23 secs, heh
11:53:59 bauzas any reason why this value ?
11:54:04 gibi 123 :)
11:54:09 gibi just a sequence of ints
12:00:32 bauzas could be 0.123 :p
12:02:37 gibi lost opportunity
12:37:25 gibi bauzas: finally I understood your comment. see the response in https://review.opendev.org/c/openstack/nova/+/845922/1#message-84ed99281b79342e03258fde3778208610868563
12:38:32 bauzas gibi: sorry yeah, I understood this was a returned value of a mock
12:38:52 gibi so there is no delay in the test
12:39:07 bauzas yeah I was wrong when commenting
12:40:59 gibi no worries
13:06:53 opendevreview Merged openstack/nova stable/yoga: Simulate bug 1969496 https://review.opendev.org/c/openstack/nova/+/840832
14:07:45 opendevreview norman shen proposed openstack/nova master: Clear connection info if vol disconnected https://review.opendev.org/c/openstack/nova/+/845995
14:07:46 sean-k-mooney artom: yes i was refering to https://review.opendev.org/c/openstack/nova/+/845896
14:10:07 opendevreview Merged openstack/nova stable/train: Only allow one scheduler service in tests https://review.opendev.org/c/openstack/nova/+/751362
14:10:14 opendevreview Merged openstack/nova stable/train: func tests: move _run_periodics() into base class https://review.opendev.org/c/openstack/nova/+/751363
14:10:22 opendevreview Merged openstack/nova stable/train: Helper to start computes with different HostInfos https://review.opendev.org/c/openstack/nova/+/751364

Earlier   Later