| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-09 | |||
| 15:01:21 | mnaser | because im trying to go through this code and i cant find how/why it's happening | |
| 15:01:32 | mnaser | okay. i'll go do some more in depth checking then | |
| 15:01:39 | mriedem | UnboundLocalError right? | |
| 15:01:48 | jaypipes | dansmith: heh, yeah... I was tempted to put into the log debug message something like "we're really not sure whether we even get here any more and if we do, what we should do anyway" ;) | |
| 15:01:52 | mriedem | seems that would be obvious | |
| 15:02:09 | mnaser | mriedem the manager is catching the original exception so its making it a tad harder | |
| 15:02:42 | mnaser | based on my search, it should either be in nova/objects/numa.py or nova/virt/hardware.py as those are the two references to it | |
| 15:02:43 | mriedem | oh, LOG.exception? | |
| 15:02:55 | dansmith | jaypipes: yeah, and it might be useful. Just don't say "We were completely burned out at the end of pike and this seems like a bad thing we probably should have handled. Sorry about that." | |
| 15:03:12 | jaypipes | dansmith: don't give me ideas... :P | |
| 15:03:27 | mnaser | :313 | |
| 15:03:27 | mnaser | mriedem i'll have to do, it's def not catching by default, compute logs is just showing: 2017-08-09 14:33:00.758 4242 DEBUG nova.compute.utils [req-91225f58-71c5-44d3-ae5c-734986b4a3f7 16e021e0ed5f47b68c095d6885f18f4b d7594b0298b54bcc9e4e0f252e1da2e4 - - -] [instance: 18b91008-d42f-4eac-b57d-07195e7774ba] local variable 'sibling_set' referenced before assignment notify_about_instance_usage /usr/lib/python2.7/site-packages/nova/compute/utils.py | |
| 15:03:36 | dansmith | mriedem: comment in here about the wording https://review.openstack.org/#/c/491582/6 | |
| 15:03:45 | jaypipes | dansmith: how about this? LOG.debug("We used to think we were indecisive. Now we're not so sure.") | |
| 15:04:07 | dansmith | jaypipes: that seems fine to me. highly truth-based | |
| 15:04:15 | dansmith | jaypipes: but I'd only +1 it and wait for others to +2 | |
| 15:04:15 | jaypipes | very truthy indeed. | |
| 15:07:04 | mriedem | dansmith: replied https://review.openstack.org/#/c/491424/7/releasenotes/notes/pike_prelude-fedf9f27775d135f.yaml - so you want me to add those or leave it? | |
| 15:08:22 | dansmith | mriedem: I didn't realize that wasn't +A.. I would have.. I was just suggesting that maybe circling back and putting it in there might be worthwhile, but it's really not a big deal | |
| 15:08:38 | dansmith | it's on its way now | |
| 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 | mnaser | becuase maybe it shouldn't be and thats the issue | |
| 15:15:50 | dansmith | so you can use None as the sentinel | |
| 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: Add documentation for documentation contributions https://review.openstack.org/492124 | |
| 15:21:43 | openstackgerrit | Sean Dague proposed openstack/nova master: Structure cli page https://review.openstack.org/492111 | |
| 15:21:44 | mnaser | and when its empty (for whatever reason), line 805 reference a variable that was not assigned | |
| 15:21:44 | openstackgerrit | Sean Dague proposed openstack/nova master: doc: Import configuration reference https://review.openstack.org/491853 | |
| 15:21:44 | openstackgerrit | Sean Dague proposed openstack/nova master: Clean up *most* ec2 / euca2ools references https://review.openstack.org/492166 | |
| 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 | |