| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-16 | |||
| 15:58:02 | melwitt | dansmith: yeah, I agree from a user perspective definitely sum(old, new) is weird. but I was feeling unsure because of how we really are holding resources in two places | |
| 15:58:05 | mnaser | I have unsuccessfully been digging the code, and I have a cloud here where a bunch of instances have network_info=[] | |
| 15:58:31 | mriedem | mnaser: for neutron, nova.network.neutronv2.api.API._get_instance_network_info or something like that | |
| 15:58:38 | dansmith | melwitt: right, that's the operator argument, but it still seems silly to me | |
| 15:58:50 | dansmith | melwitt: that's kinda overhead and transient | |
| 15:58:59 | melwitt | dansmith: ok. that was my assumption but like.... we wouldn't be able to just use the /usages API simply anymore. how would we get all the info about what /usages are part of a resize etc. I can't even think about it right now | |
| 15:59:01 | dansmith | melwitt: "cost of business" | |
| 15:59:34 | mriedem | melwitt: i don't think you can unless placement has consumer types | |
| 15:59:40 | melwitt | dansmith: yeah, I agree with that too, overhead and transient | |
| 15:59:46 | dansmith | melwitt: well, the reserved resources are owned by a migration and not an instance, but yeah, consumer types :) | |
| 15:59:55 | mriedem | mnaser: https://github.com/openstack/nova/blob/master/nova/network/neutronv2/api.py#L1824 | |
| 16:00:07 | melwitt | mriedem: yes, definitely true, need consumer types but even still, I'm not 100% sure that would be enough | |
| 16:00:16 | mriedem | mnaser: that's called from here https://github.com/openstack/nova/blob/master/nova/network/base_api.py#L253 | |
| 16:00:30 | melwitt | and what would the calls look like to work that out on the nova side | |
| 16:01:15 | mriedem | GET /usages?project_id=foo&consumer_type=instance | |
| 16:01:19 | mnaser | mriedem: it doesn't seem like there's a task that just calls get_instance_nw_info() from time to time ,gr | |
| 16:01:27 | mriedem | mnaser: oh but there is | |
| 16:01:38 | mriedem | mnaser: question is, which release is this cloud on? | |
| 16:01:42 | mnaser | rocky | |
| 16:01:47 | melwitt | mriedem: yeah, but then you get a sum of everything. how to pick apart and remove the min(old, new)? | |
| 16:01:58 | dansmith | melwitt: no | |
| 16:02:06 | mnaser | *something* has set the network info cache to empty.. dunno what/why yet | |
| 16:02:07 | dansmith | melwitt: that would exclude all the migration-held resources | |
| 16:02:08 | melwitt | no :) | |
| 16:02:19 | mriedem | mnaser: i've got the bug for you and patch, sec | |
| 16:02:24 | melwitt | oh it would? I guess I didn't know that | |
| 16:02:33 | melwitt | oh, because the consumer type would be 'migration'? | |
| 16:02:34 | dansmith | melwitt: if you only show instance-held resources? | |
| 16:02:51 | dansmith | melwitt: right, what else would you use consumer types for in this case? :) | |
| 16:02:53 | mriedem | right if you want to know new flavor usage, you'd filter on consumer_type=instance, | |
| 16:03:02 | mriedem | if you want to know old flavor usage, you'd filter on consumer_type=migration | |
| 16:03:29 | dansmith | the only limitation there would be that you'd get current not max(current, old) but I think that's okay | |
| 16:03:45 | mriedem | unless, | |
| 16:03:55 | dansmith | max() would be nice to charge them for the most they're potentially going to use at any given point to avoid a revert-to-bigger making them go over quota | |
| 16:03:56 | mriedem | you then create some more servers filling up your quota and then you can't revert the resize | |
| 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 | |