| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-16 | |||
| 16:03:57 | melwitt | ok, so you'd have to have two queries, one for 'instance' consumer type and one for 'migration' instance type in order to take max(old, new) | |
| 16:04:06 | mriedem | dansmith: jinx | |
| 16:04:15 | dansmith | melwitt: or get back them grouped by type in one query | |
| 16:04:32 | melwitt | ok, I see | |
| 16:05:03 | mriedem | mnaser: https://review.openstack.org/#/c/591607/ | |
| 16:05:10 | mriedem | https://bugs.launchpad.net/nova/+bug/1751923 | |
| 16:05:11 | openstack | Launchpad bug 1751923 in OpenStack Compute (nova) "_heal_instance_info_cache periodic task bases on port list from nova db, not from neutron server" [Medium,Fix released] - Assigned to Maciej Jozefczyk (maciej.jozefczyk) | |
| 16:05:33 | mriedem | mnaser: that was something from the public cloud wg (ovh worked the fix i started), and our public cloud ops team needed it as well | |
| 16:05:53 | mriedem | b/c of the same thing you said - network_info gets wiped out and the heal task wouldn't refresh from neutron, but from the cache itself, which is ... dumb | |
| 16:06:21 | mriedem | note there is a pretty beefy data migration patch before that in the series | |
| 16:06:28 | mriedem | which is why we haven't backported it | |
| 16:06:42 | mnaser | ah this is tein | |
| 16:06:44 | mnaser | stein | |
| 16:06:45 | mnaser | poop | |
| 16:07:05 | mriedem | mnaser: i think you also mentioned something like this a few weeks ago which prompted me to write this https://review.openstack.org/#/c/640516/ | |
| 16:07:45 | mriedem | ^ seems obvious to me, but when i dug into history where were previous attempts to do the same which were reverted because of "potential race issues" or something | |
| 16:07:58 | mriedem | those were also many years ago so idk if they'd still exist | |
| 16:08:07 | mnaser | I dunno how this kinda just appeared out of nowhere | |
| 16:08:17 | mnaser | queens cloud upgraded to rocky and poof | |
| 16:08:20 | mriedem | mnaser: that bug has some scenarios where people hit it | |
| 16:09:27 | mriedem | mnaser: https://bugs.launchpad.net/nova/+bug/1751923/comments/4 is the case our ops team hit - changing policy | |
| 16:09:28 | openstack | Launchpad bug 1751923 in OpenStack Compute (nova) "_heal_instance_info_cache periodic task bases on port list from nova db, not from neutron server" [Medium,Fix released] - Assigned to Maciej Jozefczyk (maciej.jozefczyk) | |
| 16:09:39 | mnaser | mriedem: ok so I assume _get_ordered_port_list() will not work in rocky | |
| 16:09:57 | mnaser | because of the lack of index | |
| 16:10:28 | mriedem | mnaser: it depends on if the instances were created after mitaka | |
| 16:10:40 | mriedem | because it relies on the virtual interface record and we didn't start creating those for servers until newton | |
| 16:10:51 | mriedem | hence the online data migration https://review.openstack.org/#/c/614167/ | |
| 16:14:48 | mriedem | a bug was reported last week for that online data migration as well https://bugs.launchpad.net/nova/+bug/1824435 but so far we don't have a reproducer | |
| 16:14:49 | openstack | Launchpad bug 1824435 in OpenStack Compute (nova) stein "fill_virtual_interface_list migration fails on second attempt" [High,Triaged] | |
| 16:15:11 | mnaser | oh well | |
| 16:15:19 | mnaser | this cloud has been running since queens | |
| 16:15:28 | mnaser | so I guess I could get away with running this once? | |
| 16:15:48 | bauzas | dansmith: thanks for your comments on https://review.openstack.org/#/c/650963/ | |
| 16:15:52 | mriedem | if all the instances on that cloud were created since at least queens you should be ok | |
| 16:16:01 | bauzas | dansmith: I don't disagree with you and I understand your concerns | |
| 16:16:19 | mriedem | mnaser: you could find out by comparing the instances table count to the virtual_interfaces table or something, i.e. is there at least one vif per instance? | |
| 16:16:42 | bauzas | dansmith: I guess we could just have a 'preferred' policy | |
| 16:16:54 | bauzas | dansmith: would that be okay for you ? | |
| 16:17:11 | dansmith | bauzas: meaning, not add the knob, and just make the behavior always "preferred" right? | |
| 16:17:26 | stephenfin | mriedem: Sorry, I rushed that. I assume if you've pulled it down I should leave it alone | |
| 16:17:26 | bauzas | yup | |
| 16:17:34 | bauzas | no UX | |
| 16:17:46 | sean-k-mooney | prefered in the context of? | |
| 16:17:51 | sean-k-mooney | vgpu numa? | |
| 16:17:53 | bauzas | I just need to think about the upgrade tho | |
| 16:18:05 | bauzas | but I don't think it would be a problem | |
| 16:18:07 | sean-k-mooney | wasnt that the suggeted default in the spec | |
| 16:18:08 | bauzas | sean-k-mooney: correct | |
| 16:18:09 | stephenfin | It's dumb but running tox cripples Bluejeans and any day that I've loads of meetings (like today) means I can't kick off anything locally that's going to run in the background | |
| 16:18:20 | bauzas | sean-k-mooney: nope, the default was 'nothing changes' | |
| 16:18:39 | dansmith | bauzas: yes I think that'd be ideal | |
| 16:18:41 | sean-k-mooney | oh well prefered was just going to be impleneted by a weigher right | |
| 16:19:02 | bauzas | dansmith: okay, I'll -W the spec and work on a new PS | |
| 16:19:07 | dansmith | cool | |
| 16:19:09 | sean-k-mooney | we will prefer host that can provide numa affinty but not filter out any | |
| 16:19:17 | sean-k-mooney | like the pci weigher | |
| 16:19:20 | mnaser | mriedem: SELECT instances.uuid, COUNT(virtual_interfaces.uuid) FROM instances LEFT JOIN virtual_interfaces ON virtual_interfaces.instance_uuid = instances.uuid WHERE instances.deleted=0 GROUP BY instances.uuid; | |
| 16:19:26 | mnaser | shows 1/2 but no 0's | |
| 16:19:38 | bauzas | sean-k-mooney: yup, just a way to see whether we can have *some affinity* | |
| 16:19:51 | bauzas | if no affinity possible, fine | |
| 16:19:55 | mnaser | so I guess I can cherry-pick that code and run it once.. | |
| 16:20:07 | sean-k-mooney | yep | |
| 16:20:16 | sean-k-mooney | that is why i assumed it would be the default | |
| 16:20:32 | sean-k-mooney | becaue its best effort | |
| 16:20:32 | mriedem | mnaser: cool, let me know how it goes | |
| 16:20:36 | bauzas | sean-k-mooney: the concern wasn't really the default | |
| 16:20:39 | bauzas | value* | |
| 16:20:56 | bauzas | sean-k-mooney: the concern is more whether we want to introduce more knobs, and 'required' needed those | |
| 16:21:24 | openstackgerrit | Mohammed Naser proposed openstack/nova stable/rocky: Force refresh instance info_cache during heal https://review.openstack.org/653040 | |
| 16:21:27 | bauzas | anyway, I'll just write a new revision, and people could chime on it | |
| 16:22:16 | stephenfin | bauzas, sean-k-mooney: RE: the cpu-resources discussion above, it's not nova-compute that makes the call to placement | |
| 16:22:46 | bauzas | stephenfin: which call are we talking about ? | |
| 16:23:04 | stephenfin | the claim | |
| 16:23:44 | bauzas | yup, I don't disagree | |
| 16:23:52 | bauzas | but then I don't get your point | |
| 16:24:05 | mnaser | at least it applies cleanly | |
| 16:24:24 | bauzas | my upgrade concerns are about when and how we should transform inventories | |
| 16:24:36 | bauzas | (and move allocations accordingly) | |
| 16:25:08 | stephenfin | bauzas: We'd either have to request a certain amount of PCPU resources, which couldn't be fulfilled since we're not reporting any inventory (because the config values haven't been set) | |
| 16:25:35 | stephenfin | Or we'd have to keep requesting VCPU resources for everything, which borks the whole idea | |
| 16:25:44 | bauzas | I tend for the latter | |
| 16:26:22 | bauzas | if people start asking PCPU resources, they necessarly have to be fulfilled by hosts ready to accept them, I don't disagree | |
| 16:27:05 | bauzas | but then, Train is necessarly a mitigation release | |
| 16:27:10 | bauzas | because of computes | |
| 16:27:21 | bauzas | we did had the same problem with rolling upgrades | |
| 16:27:42 | bauzas | you can't really make use of a feature unless all computes are up to date in general, in particular when it comes to resources usage | |
| 16:29:30 | stephenfin | bauzas: hmm, that removes our ability to transform 'hw:cpu_policy=dedicated' under the hood though | |
| 16:29:32 | bauzas | that doesn't mean operators can't use PCPU requests with Train | |
| 16:29:47 | stephenfin | and if a CPU is part of the PCPU pool, it can't be part of the VCPU poool | |
| 16:29:53 | bauzas | but they absolutely need to converge all their inventories *before* they use the PCPU request | |
| 16:31:15 | stephenfin | without the transform in place, any existing instances can't be migrated (they wouldn't be using PCPUs but rather the legacy 'hw:cpu_policy=dedicated' extra spec) plus flavours and images would not be requesting the correct stuff | |
| 16:32:01 | stephenfin | The point is I think we need that rewriting in place, and if we need that then we need some initial PCPU inventories in place as soon as the upgrade is in place, otherwise the ability to do move existing instances or create new ones is gone :( | |
| 16:32:22 | stephenfin | This could really do with a call, I think | |
| 16:34:47 | sean-k-mooney | probably | |
| 16:36:48 | bauzas | stephenfin: I have to leave in a few, but I'll summarize my thoughts | |
| 16:37:19 | bauzas | we could just leave existing options as they are and leave inventories be VCPU | |
| 16:37:23 | bauzas | but | |
| 16:38:15 | bauzas | once operator sets config options (and then we can discuss on this specific trigger), then we do a reshape for this host and split VCPU inventory into VCPU and PCPU inventories | |
| 16:38:23 | bauzas | and we accordingly move allocations | |
| 16:38:51 | bauzas | with the slight detail that we verify resources *before* doing the reshape so we can raise an exception | |