Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-05-16
15:40:29 sdague dansmith: I don't know
15:40:42 sdague but that's where the google's are leading me to
15:40:56 dansmith sdague: okay will try that
15:41:15 bauzas dansmith: my own opinion is that given we say to first upgrading the conductor, then the conductor is anyway newer that the scheduler, so it would know the new key
15:41:31 sdague dansmith: daemon-reload probably should be tried first
15:41:37 dansmith sdague: yeah
15:41:44 openstackgerrit dane-fichter proposed openstack/nova master: Add trusted certificates to InstanceExtras https://review.openstack.org/457711
15:41:49 dansmith bauzas: of course, if you're changing the rpc api... you should version that change :)
15:41:56 bauzas dansmith: but I could just add a new RPC API version for not returning the new key in case the asked version is not cool
15:42:16 bauzas dansmith: I'm not modifying the interface
15:42:23 bauzas dansmith: so it's not needing a version
15:42:36 bauzas dansmith: I'm just adding some new key for what we return
15:42:55 bauzas but lemme just upload the change
15:43:00 bauzas and you'll see
15:43:39 dansmith changing the return is changing the api, period.
15:44:25 bauzas okay
15:50:56 cfriesen dansmith: with regards to the multiple network-changed events for the same instance, this is in the context of a live-migration on a busy system. Adding to the pain is that the "with lockutils.lock" took a long time to be acquired (which seems to be due to the lack of a "fair" lock in oslo).
15:54:46 dansmith cfriesen: not sure I get it, but if that is just avoiding processing the same instance in a single go, then I guess it makes sense
15:54:55 dansmith (but should use a set() of course)
16:02:38 cfriesen dansmith: yes, it's just to try to process each instance once. I guess neutron must be batching multiple events in a single call.
16:02:53 dansmith I'm pretty sure they don't
16:03:30 cfriesen dansmith: then why is the code written to handle multiple events per call?
16:03:35 dansmith tbh, I think I'd want an analysis of that first.. like "neutron started batching things in commit <foo> therefore we need this"
16:03:36 dansmith otherwise I'd be highly suspect that something else is going on
16:04:00 dansmith cfriesen: because the whole thing was written to be high bandwidth, but AFAIK, they never used it that way
16:04:24 dansmith and multiple events of the same instance in a single batch hardly makes sense
16:04:47 dansmith if they're doing that, then there needs to be a fix on both sides, but if you just paper over it here, we'll never find that
16:08:44 melwitt dansmith, sdague: I did some tests where I ran a subset of tempest locally with concurrency of 4 with and without caching oslo.db transaction context managers and with, it topped out around 90 Threads_connected in mysql and without it was around 140. so it did seem to make a difference. this is the patch I tried https://review.openstack.org/#/c/465042
16:09:16 openstackgerrit Gage Hugo proposed openstack/nova master: Admin API Policy contingent on is_admin_project https://review.openstack.org/384148
16:10:26 sdague melwitt: ok, cool, so that's definitely helpful
16:10:55 dansmith sdague: melwitt: I'm not sure I'm surprised by that, up to the limit of the connection pool
16:11:07 sdague melwitt: it feels like we should do that in addition to the raise of the open fds, right?
16:11:08 dansmith the question would be if that continues to grow forever
16:11:33 melwitt the subset I ran was like 'ostestr run --regex tempest.api.compute.servers --concurrency 4' which is about 206 tests
16:11:56 dansmith sdague: melwitt: we also need to keep in mind that we need to be able to flush this cache on SIGHUP if/when we implement that for the list-of-cells cache, else we'll never notice a connection change
16:14:13 cfriesen dansmith: I think on the neutron side they're using BatchNotifier, calling send_events() (https://git.openstack.org/cgit/openstack/neutron/tree/neutron/notifiers/nova.py#n242) which just blindly fires the events over to nova.
16:15:14 dansmith cfriesen: sure, but what does that have to do with anything?
16:15:28 openstackgerrit Balazs Gibizer proposed openstack/nova master: use context mgr in instance.delete https://review.openstack.org/443764
16:15:45 cfriesen dansmith: neutron has multiple network changes over live migration, queues up multiple events for the same instance, and fires them off in a single API call to nova
16:15:58 cfriesen dansmith: nova then updates the cache multiple times for the same instance
16:16:37 dansmith cfriesen: it has multiple plug events, but it should not send multiple refresh-cache events
16:16:54 cfriesen dansmith: it's just a "network-changed" event
16:17:01 dansmith yeah, that
16:19:03 melwitt dansmith: okay, just looked through the code and I see what you mean. so that one does a load-all-cells once, so it would need to be flushed itself if a new cell is added, right?
16:19:21 cfriesen dansmith: looks like it might send a network-changed event each time a port is updated or a floating ip is updated.
16:19:45 dansmith cfriesen: ...right
16:20:05 cfriesen dansmith: so if you've got 16 ports on an instance...
16:20:45 dansmith melwitt: we've built multiple lists of cells that will have to be flushed, and this is another, is my only point
16:21:06 melwitt ack
16:21:13 dansmith cfriesen: I understand, but that still doesn't mean that neutron should send a single request with 16 identical events for the same instance
16:23:16 cfriesen dansmith: oh, I agree. was just trying to show how we could end up in this scenario since earlier you said you were pretty sure neutron didn't do that.
16:24:18 dansmith cfriesen: I meant for multiple instances
16:25:42 cfriesen dansmith: with the neutron BatchNotifier code I think neutron just sends all queued notifications every couple seconds
16:26:00 cfriesen dansmith: so I think it could include events for multiple instances
16:27:01 dansmith cfriesen: that would be a change from how it originally behaved, and would delay instance booting unnecessarily
16:27:16 dansmith I don't see any timing in that code
16:27:35 cfriesen look at neutron/notifiers/batch_notifier.py
16:28:53 dansmith yeah, that's unfortunate
16:30:09 cfriesen we'll try pushing a neutron change to remove duplicate network-changed events for the same instance from the batch
16:31:00 dansmith in your nova dupe check, you should log a warning or something so we can see if this ever regresses once you fix the sending side
16:31:16 cfriesen was just going to ask if we wanted a nova dupe check as well
16:41:50 sean-k-mooney sfinucan: https://bugs.launchpad.net/nova/+bug/1691178 this is the bug for the qemu cpu affinity ill upload the patch WIP patch in a few minutes but if you have in put on the scheduler question feel free to advise.
16:41:50 openstack Launchpad bug 1691178 in OpenStack Compute (nova) "virt_type=qemu does not support cpu affinity." [Undecided,New] - Assigned to sean mooney (sean-k-mooney)
16:43:00 sean-k-mooney sfinucan: there isnt much in the bug but i will add you to the review of the code change when i split it out of my other changes.
16:47:01 cfriesen sean-k-mooney: would we maybe want to make this configurable? otherwise you're going to need nested kvm to test any of the "dedicated" cpu policy stuff
16:47:16 cfriesen (or real hardware)
16:48:48 sean-k-mooney cfriesen: i was going to make it condtional based on the config value of virt_type
16:49:08 sean-k-mooney cfriesen: cpu pinning is currently only tested in intels nfv ci
16:50:16 sean-k-mooney cfriesen: i basically looks like this https://pastebin.com/afp0M4Rm
16:50:26 sean-k-mooney im currently rebasing to master
16:51:14 sean-k-mooney cfriesen: sfinucan basically just adding if(CONF.libvirt.virt_type != "qemu"): in 2 places in the libvirt dirver and then i need to check unit tests and update if needed
16:53:00 dansmith melwitt: your comment about stale objects is about conf change? or are you worried that seldom-used ones will actually become stale in some un-usable way?
16:54:53 sean-k-mooney cfriesen: sfinucan that change should be in /nova/virt/libvirt/driver.py not /nova/virt/libvirt/config.py ... this is why i never edit diffs/patches
16:55:00 efried I just discovered this (apologies if everyone else knew it already): Add `-a` to journalctl and you get COLORS!
16:56:00 melwitt dansmith: I was thinking more of the latter, though I can't imagine it being much of a problem in practice. it would be things like, removing cells from the fleet or if a cell went temporarily offline
16:56:26 dansmith hmm
16:56:28 melwitt conf change is important but I wasn't thinking of that yet
16:56:40 cfriesen sean-k-mooney: I know at least some of our devs are testing with virtualbox which doesn't do nested KVM. You'd basically require nested KVM or real hardware to exercise the cpu pinning logic. I think a "permissive_qemu_affinity" config option might make sense, though I know we're trying to get rid of config options in general.
16:56:43 dansmith we don't have to do something if we restart the db today, right?
16:56:55 dansmith it reconnects for us I think, so I'm not sure we need to actually worry about things going stale
16:57:03 dansmith and by conf change, I mean changing a cell record or something
16:57:10 melwitt I don't think so, no. good point
16:57:27 melwitt yeah, understood. like changing a connection string
16:57:33 sean-k-mooney cfriesen: you dont need nested virt you just need the kvm module loaded in the vm
16:57:35 dansmith right
16:57:48 cfriesen sean-k-mooney: the kvm module won't load without nested KVM
16:58:07 sean-k-mooney if they are using virt_type=qemu they cannont use any form of cpu pinning becasue TSG does not support it
16:58:23 cfriesen sean-k-mooney: it accepts the commands, it just doesn't work properly. :)
16:58:50 sean-k-mooney libvirt throws an error though whne you try to spawn the vm from nova
16:59:19 cfriesen sean-k-mooney: not ours...wonder what version that was added in
16:59:37 sean-k-mooney cfriesen: i am testing with qemu 2.8
17:00:04 sean-k-mooney i also have libvirt 3.3 form master currently but this happens on older version too
17:00:22 cfriesen sean-k-mooney: I'm on qemu 2.6 and libvirt 2.0
17:01:25 sean-k-mooney cfriesen: im a litlle unfortable adding a new conf option just for this as well if we do they are almost impossible to get rid of
17:01:33 cfriesen sean-k-mooney: we can work around it if necessary
17:02:08 sean-k-mooney cfriesen: do you know is there any way to query this from qemu/libvirt so we could decet at runtime?
17:03:55 cfriesen sean-k-mooney: no clue. :) I'm a bit surprised that your libvirt complains since for the most part (especially for single-vcpu guests) it seems to work okay.
17:06:43 sean-k-mooney cfriesen: i think it is actully qemu that is complaining rather then libvirt it just passes along the message
17:07:39 sean-k-mooney cfriesen: libvirt happilly accpts the xml as it is vailid but TCG then complaines when qemu is starting the instance
17:35:38 openstackgerrit Gage Hugo proposed openstack/nova master: Admin API Policy contingent on is_admin_project https://review.openstack.org/384148

Earlier   Later