| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-09 | |||
| 15:08:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: replace chance with filter scheduler in func tests https://review.openstack.org/491529 | |
| 15:08:48 | dansmith | I was just commenting on your comment | |
| 15:10:06 | mriedem | want to send this in also https://review.openstack.org/#/c/491581/ ? | |
| 15:10:11 | mriedem | it was previously approved | |
| 15:10:27 | mriedem | pad your stats before i wreck your stats on these RT patches :) | |
| 15:10:42 | mnaser | would someone be kind enough to just eye this for a second with me before i dive deeper? is it possible that i get an error of sibling_set being referenced before assignment if: the siblings_set is empty, therefore the loop next occurs and it is referenced in line 805 (outside the loop) -- https://github.com/openstack/nova/blob/stable/newton/nova/virt/hardware.py#L777-L807 | |
| 15:11:11 | mnaser | sorry poorly worded that, the loop is pretty much skipped over so sibling_set is never set to anything and it's referenced in the if statement below it | |
| 15:11:32 | dansmith | mriedem: :( | |
| 15:12:05 | dansmith | mnaser: looking | |
| 15:12:23 | mriedem | mnaser: https://github.com/openstack/nova/blob/stable/newton/nova/virt/hardware.py#L805 would be the problem right? | |
| 15:12:32 | mriedem | if sibling_sets.items() was empty | |
| 15:12:41 | mriedem | then there is no sibling_set variable set in the for loop above | |
| 15:12:57 | mnaser | thats what i was guessing -- kinda wanted a second pair of eyes before i dive in deeper in the wrong place | |
| 15:13:04 | mnaser | i will check and see what the value of sibling_sets is | |
| 15:13:07 | dansmith | yeah it's referencing the loop variable | |
| 15:13:39 | mnaser | ok cool, i'll do some more checking and see what the sibling set value is when it works and when it doesnt | |
| 15:13:56 | mnaser | (oddly enough, it fails only on the *last* instance to go in the server -- ex: if it fits 15 VMs, 14 will go in, the 15th will fail with that) | |
| 15:14:17 | dansmith | mnaser: you could just put 798 and below inside an "if sibling_sets" conditional | |
| 15:14:28 | dansmith | mnaser: then it won't run if the loop didn't do a thing, which would avoid the problem | |
| 15:14:55 | mriedem | if only stephenfin were around to harass | |
| 15:15:05 | dansmith | it'd be much better to just set something before the loop to None, and then set it inside the loop and only run the bottom code if we found a thing | |
| 15:15:18 | dansmith | because it's just using the last value it iterated over | |
| 15:15:40 | mnaser | yeah that seems cleaner, but i also wonder if the issue is siblings_set being empty | |
| 15:15:44 | dansmith | sibling_set has to be a tuple for the use on L805 | |
| 15:15:50 | dansmith | so you can use None as the sentinel | |
| 15:15:50 | mnaser | becuase maybe it shouldn't be and thats the issue | |
| 15:16:19 | dansmith | mnaser: well, it's fragile code so it deserves fixing regardless, IMHO | |
| 15:16:21 | mnaser | so i just want to make sure the original issue isnt siblings_set being empty? | |
| 15:16:43 | mnaser | true | |
| 15:16:44 | mnaser | i was a bit taken aback seeing a variable referenced before assignment in nova's code :p | |
| 15:16:45 | dansmith | if it being empty is possible (which it clearly is) and that's fatal, then this needs to check for it and log a warning | |
| 15:17:38 | dansmith | we could ask sahid | |
| 15:17:50 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add release note for shared storage known issue https://review.openstack.org/491582 | |
| 15:18:31 | gibi | mriedem, jaypipes, dansmith: As Jay's resize confirm fix is almost done could you take a look on the other resize bugfix https://review.openstack.org/#/c/491491 | |
| 15:19:54 | mnaser | yum, theory validated - SIBLING_SETS: defaultdict(<type 'list'>, {}) _pack_instance_onto_cores /usr/lib/python2.7/site-packages/nova/virt/hardware.py:781 | |
| 15:20:04 | mnaser | now time to understand the siblings_set and why its empty | |
| 15:20:35 | danpb | dansmith: ? | |
| 15:20:36 | dansmith | mnaser: danpb is your huckleberry | |
| 15:21:04 | mnaser | hi danpb -- i think i've discovered an issue in nova/virt/hardware.py | |
| 15:21:19 | mnaser | for some reason, siblings_set is become an empty list | |
| 15:21:25 | mnaser | https://github.com/openstack/nova/blob/stable/newton/nova/virt/hardware.py#L777-L807 | |
| 15:21:43 | openstackgerrit | Sean Dague proposed openstack/nova master: Structure cli page https://review.openstack.org/492111 | |
| 15:21:43 | openstackgerrit | Sean Dague proposed openstack/nova master: Add documentation for documentation contributions https://review.openstack.org/492124 | |
| 15:21:44 | openstackgerrit | Sean Dague proposed openstack/nova master: Clean up *most* ec2 / euca2ools references https://review.openstack.org/492166 | |
| 15:21:44 | openstackgerrit | Sean Dague proposed openstack/nova master: doc: Import configuration reference https://review.openstack.org/491853 | |
| 15:21:44 | mnaser | and when its empty (for whatever reason), line 805 reference a variable that was not assigned | |
| 15:21:56 | dansmith | danpb: we know how to fix the acute issue, but mnaser is concerned that sibling_sets being empty might be indicative of some more fundamental problem | |
| 15:22:01 | dansmith | and wanted a sanity check | |
| 15:22:35 | sdague | mriedem: the config reference import that stephenfin was working on should be workable now, the last iteration had a wrong include stanza that worked locally because of cruft I had, but failed in the gate | |
| 15:22:44 | dansmith | danpb: we're hoping it was just an oversight and that set can be empty for legit reasons | |
| 15:23:05 | mnaser | is there any scenario where sibling_set should be empty? this is a server with 32 cores, vcpu_pin_set is set to 2-31 | |
| 15:23:22 | mnaser | wait | |
| 15:23:23 | mnaser | oh boy | |
| 15:23:37 | mnaser | did i just forget how to math | |
| 15:23:50 | mnaser | and the fact it had 29 cores probably messed up the math | |
| 15:24:06 | danpb | sibling_sets gets populated from available_siblings iiuc | |
| 15:24:15 | danpb | so presumably available_siblings is empty too ? | |
| 15:24:42 | mnaser | danpb: let me put some log.debug's, but i confirmed that siblings_set was empty | |
| 15:24:53 | mnaser | fyi this issue only occurs on the *last* VM to fit on the compute node | |
| 15:26:12 | danpb | well i think you'd need to work backwards through the call stack to figure out why it becomes empty | |
| 15:26:17 | mnaser | danpb: AVAILABLE_SIBLINGS: [CoercedSet([]), CoercedSet([]), CoercedSet([]), CoercedSet([]), CoercedSet([]), CoercedSet([]), CoercedSet([])] | |
| 15:27:19 | mnaser | ok i guess ill have to see why host_cell.free_siblings is being set to that | |
| 15:27:30 | dansmith | danpb: doesn't this come from libvirt? | |
| 15:28:12 | mnaser | the only thing i can imagine which can cause a corner case is the fact that we reserve 2 cores for the OS, so vcpu_pin_set=2-31 .. maybe that's not taken in consideration (guessing) | |
| 15:28:17 | openstackgerrit | Matt Riedemann proposed openstack/nova master: [placement] Add api-ref for usages https://review.openstack.org/480563 | |
| 15:29:51 | danpb | dansmith: libvirt will provide info on the siblings present on the host, but iiuc free_siblings is populated by nova | |
| 15:30:17 | danpb | so if mnaser is saying it works correctly for all VMs until the last one, it sounds like nova is filtering the info from libvirt and ending up with the empty set | |
| 15:30:24 | dansmith | danpb: oh okay, I hadn't traced it very far up the stack because I assumed we must just be getting an empty set of things from libvirt because we had no numa info or something | |
| 15:30:32 | dansmith | danpb: yeah, makes sense | |
| 15:30:35 | sahid | danpb: mnaser danpb i just for information i was working on this but did not find the root cause | |
| 15:30:37 | sahid | https://review.openstack.org/#/c/458848/ | |
| 15:30:42 | mnaser | im checking the values of host_cell and instance_cell | |
| 15:31:15 | mnaser | HOST_CELL: NUMACell(cpu_usage=14,cpuset=set([2,4,6,8,10,12,14,16,18,20,22,24,26,28,30]),id=0,memory=196562,memory_usage=57344,mempages=[NUMAPagesTopology,NUMAPagesTopology],pinned_cpus=set([2,4,6,8,10,12,14,18,20,22,24,26,28,30]),siblings=[set([8,24]),set([2,18]),set([10,26]),set([12,28]),set([6,22]),set([14,30]),set([4,20])]) | |
| 15:31:36 | danpb | how many VMs are you running ? | |
| 15:32:09 | danpb | you've got 7 pairs of siblings there, so if each VM wanted one pair, you'd be able to run 7 VMs | |
| 15:32:42 | mnaser | booted with 120x 1GB hugepages, 30 cores available, trying to boot 15 VMs with 2 cores each + 8gb of memory each | |
| 15:32:43 | mnaser | but im not using isolate, im using prefer which i believe should try to schedule them on the same hyperthread | |
| 15:33:17 | mnaser | my flavor has properties: hw:cpu_policy=dedicated, hw:cpu_thread_policy=prefer, hw:mem_page_size=1048576, hw:numa_nodes=2 | |
| 15:33:58 | mnaser | (also hw:cpu_thread_policy=prefer forgot to put that in) | |
| 15:34:23 | mnaser | i was thinking hw:numa_nodes=2 and hw:cpu_thread_policy=prefer might be the source of the issue because they are a bit the opposite of each other | |
| 15:35:48 | mnaser | getting memory split from 2 numa nodes but also wanting to be on the same hyperthread isnt really possible considering you'll want to be on two physical cpus to access the numa node | |
| 15:36:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: [placement] Add api-ref for RP usages https://review.openstack.org/450105 | |
| 15:37:50 | danpb | mnaser: how many vcpus per guest ? | |
| 15:38:00 | danpb | (i mean what is vcpu count in the flavour ?) | |
| 15:38:04 | mnaser | danpb: 2 | |
| 15:38:18 | danpb | that flavour config looks flawed then | |
| 15:38:33 | danpb | you've got 2 vcpus, and you've said to give the VM 2 virtual numa nodes | |
| 15:38:55 | danpb | nova will want to place each virtual numa node on a separate host numa node | |
| 15:39:03 | danpb | so each vcpu will need to be on a separate socket | |
| 15:39:09 | openstackgerrit | Matthew Edmonds proposed openstack/nova master: use conf for keystone session creation https://review.openstack.org/485121 | |
| 15:39:16 | danpb | so asking nova to put the vcpus in the same hyperthread sibling is nonsensical | |
| 15:39:29 | danpb | as you can't have siblings if you have separate sockets | |
| 15:39:43 | mnaser | that is what i kinda theorized a bit | |
| 15:39:54 | mnaser | so will have to fallback to 1 numanode default | |
| 15:40:08 | danpb | nova shouldn't be trying to use hypthread siblings at all in ths case | |
| 15:40:43 | mnaser | that was my assumption that what it would do, thats why i put 'prefer' in there because i figured that was the more reasonable choice | |
| 15:40:46 | cfriesen | with hw:cpu_thread_policy=prefer it should just give separate pCPUs. if you had hw:cpu_thread_policy=require I think it'd fail | |
| 15:40:59 | danpb | putting in prefer should be an no-op in this case | |
| 15:41:14 | danpb | as the need to pick separate sockets should have taken priority in nova to satisfy the numa constraint | |
| 15:41:28 | danpb | and if you have thread_policy=require, then nova ought to raise a fatal error | |