| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-09 | |||
| 13:10:41 | mdbooth | lyarwood: We shouldn't be live migrating a config disk anyway | |
| 13:10:46 | mdbooth | That sounds like a different bug | |
| 13:10:58 | mdbooth | We should just host->host copy it | |
| 13:11:35 | lyarwood | mdbooth: yeah the issue wasn't with the config disk but the main instance disk iirc | |
| 13:12:17 | mdbooth | Ok. | |
| 13:12:32 | lyarwood | hmm actually that's vdb | |
| 13:12:43 | lyarwood | but you can see we are mirroring | |
| 13:17:41 | odyssey4me | Hi folks. Is there a conf entry for the number of workers nova-scheduler fires up? | |
| 13:17:43 | mdbooth | lyarwood: Ok, now I understand the interaction. | |
| 13:17:53 | odyssey4me | I can't seem to find one in the references. | |
| 13:19:39 | odyssey4me | ok, it would appear that there is one: https://github.com/openstack/nova/blob/master/nova/cmd/scheduler.py#L49 | |
| 13:28:33 | mriedem | dansmith: i'm +2 on https://review.openstack.org/#/c/582413/ if you want to re-apply your +2 | |
| 13:29:12 | mriedem | bauzas: can you go through these backports? https://review.openstack.org/#/q/topic:bug/1784705+status:open | |
| 13:29:34 | mriedem | mel said she was looking to cut stable releases today | |
| 13:29:40 | mriedem | so i'm going to try and flush some of these out | |
| 13:30:01 | dansmith | okay | |
| 13:40:40 | mriedem | stephenfin_: e. gads. https://review.openstack.org/#/q/topic:bug/1746393+status:open | |
| 13:41:43 | mriedem | feels like a feature as a bug fix | |
| 13:42:49 | mriedem | especially nervous when we have 0 CI of any of this stuff | |
| 13:46:59 | mriedem | sean-k-mooney[m]: i guess the intel 3rd party PCI/NFV CI must be dead huh? | |
| 13:47:58 | mriedem | dansmith: here are the queens backports with a +2 ready to go https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/queens+label:Code-Review=2 - the rest are in stephen's series, which i'd want to hold off on for a bit | |
| 13:49:18 | mriedem | oh and https://review.openstack.org/#/c/590062/ would be nice - that fix sat for over a year | |
| 13:50:14 | melwitt | nova meeting in 10 minutes | |
| 13:53:08 | melwitt | cdent: is this bug considered closed/fixed now that both patches have landed? neither patch used Closes-Bug in the commit message https://bugs.launchpad.net/nova/+bug/1786055 | |
| 13:53:08 | openstack | Launchpad bug 1786055 in OpenStack Compute (nova) "performance degradation in placement with large number of resource providers" [High,In progress] - Assigned to Chris Dent (cdent) | |
| 13:54:05 | cdent | melwitt: hmmm. There is more than can be done, but not likely that more will be done _now_, so I would guess closed is probably a reasonable state. The major factor has been addressed. Fixing the rest will involve considerable refactoring | |
| 13:56:05 | melwitt | cdent: I see, thanks | |
| 14:01:09 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Use os.stat and os.path.getsize for RAW disk inspection https://review.openstack.org/589567 | |
| 14:17:09 | mriedem | i guess we don't need to wait for translations https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:zanata/translations | |
| 14:20:20 | melwitt | yeah, I was thinking it seems like openstack doesn't do translations anymore but I didn't know how to check. I'll add that link to the release checklist wiki | |
| 14:20:34 | melwitt | I know we don't translate log messages | |
| 14:20:49 | melwitt | but other user-facing message, still translate? I wasn't sure | |
| 14:21:15 | efried | afaik we're still supposed to _() for exception messages. | |
| 14:21:37 | lyarwood | mdbooth: remind me again where the compute code was that deleted and recreated an attachment? | |
| 14:21:44 | melwitt | aye, I have seen that | |
| 14:22:57 | mdbooth | lyarwood: _terminate_volume_connections | |
| 14:24:17 | lyarwood | mdbooth: urgh was looking at remove_volume_connection | |
| 14:25:01 | mdbooth | lyarwood: IIRC it was triggering a bug in the cinder fixture, which assumed only 1 attachment | |
| 14:25:13 | mdbooth | But with this we've briefly got 2 attachments | |
| 14:26:30 | mdbooth | lyarwood: I don't love the raw-only fix, tbh, because I think it increases the test and maintenance burden. If that code needs to live on I'd prefer to bring it together somehow. | |
| 14:27:39 | mdbooth | I'll abandon the stat thing, though, because as you point out it's not a solution | |
| 14:28:36 | lyarwood | mdbooth: kk, well it improves performance for the raw images user that reported the issue in the short term until we start using disk.info to store the virtual size | |
| 14:29:00 | lyarwood | mdbooth: and given that means we also need to refactor code out of imagebackend I'd rather land something simple first then focus on that | |
| 14:30:28 | mdbooth | I don't think it's a refactor, btw. Just code motion really iirc. | |
| 14:30:50 | mdbooth | Would just be moving it elsewhere to make it easier to call. | |
| 14:31:23 | mriedem | GET /kashyap returns me a 404 | |
| 14:31:29 | mriedem | is his nick not registered? | |
| 14:31:41 | mdbooth | mriedem: Yep. He's out for a few more days yet, I think | |
| 14:31:46 | mriedem | blarg | |
| 14:31:51 | mriedem | wanted him to read the "nova-compute choosing incorrect qemu binary when scheduling 'alternate' (ppc64, armv7l) architectures?" thread in the ML | |
| 14:32:15 | mriedem | wondering if there is a good reason why we don't set guest.arch in the libvirt domain xml based on the hw_architecture image property | |
| 14:32:22 | mriedem | maybe someone could ask danpb? | |
| 14:33:05 | mdbooth | stephenfin_: might have an opinion | |
| 14:33:57 | mdbooth | mriedem: Is it tagged [nova]? | |
| 14:34:23 | openstackgerrit | Lee Yarwood proposed openstack/nova master: Add regression test for bug#1784353 https://review.openstack.org/587014 | |
| 14:34:24 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP compute: Terminate volume connections during _shutdown_instance https://review.openstack.org/590348 | |
| 14:34:37 | lyarwood | mdbooth: ^ that's the terminate_volume_connections alternative btw, without unit test changes | |
| 14:34:53 | mdbooth | lyarwood: That was fast! Looking. | |
| 14:35:49 | mdbooth | mriedem: Do you have an opinion on ^^^, btw? | |
| 14:36:51 | mdbooth | mriedem: Basically leaves us with a blank attachment when calling _shutdown_instance | |
| 14:36:58 | lyarwood | hmm that removes the call to detach with cinderv2 | |
| 14:37:05 | lyarwood | why would we do that during shutdown? | |
| 14:38:35 | mdbooth | lyarwood: remind me what v2 detach() does | |
| 14:38:52 | mriedem | mdbooth: seems fun at first since it's very much the same code, | |
| 14:38:55 | mriedem | but see inline comments | |
| 14:39:13 | mdbooth | terminate_connection() causes the storage backend to kill the connection | |
| 14:39:25 | mriedem | mdbooth: v2 detach is just the os-detach volume action which changes the volume status to 'available' | |
| 14:39:31 | mriedem | does nothing on the volume backend | |
| 14:39:33 | mdbooth | mriedem: Got it. | |
| 14:40:32 | mriedem | so if you did this, | |
| 14:41:20 | mriedem | you'd have to have volume attachment cleanup code in both compute manager if we don't reschedule (max_attempts=1 or force_hosts/nodes is set) or if we do reschedule and conductor build_instances hits MaxRetriesExceeded | |
| 14:41:43 | mriedem | which is essentially what we have for cleaning up ports | |
| 14:41:49 | sean-k-mooney | mdbooth: stephenfin_ is on vaction for the next week and a half just fyi | |
| 14:42:04 | mriedem | i assume that was meant for me | |
| 14:42:28 | mdbooth | mriedem: See, over here in communist Europe everybody takes vacation in August | |
| 14:42:40 | sean-k-mooney | mriedem: well both you and mdbooth since he suggested stephenfin_ would have an oppion on something | |
| 14:42:49 | sean-k-mooney | i was still scrolling back to see what | |
| 14:43:39 | lyarwood | mriedem: kk thanks, so this doesn't really simplify the fix at all | |
| 14:43:50 | mriedem | lyarwood: not really | |
| 14:44:01 | mriedem | reschedules are a minefield | |
| 14:44:12 | mriedem | mdbooth: is that what bauzas is doing as well? | |
| 14:44:18 | mriedem | france gets august off right? | |
| 14:44:23 | lyarwood | pretty much | |
| 14:44:23 | sean-k-mooney | mriedem: oh ye were talking about setting the arch in libvirtxml based on hw_architecture | |
| 14:44:24 | mriedem | for wine and cheese and love making | |
| 14:44:25 | mdbooth | mriedem: Yep | |
| 14:44:35 | melwitt | efried: haha, I am literally looking at that review already right now | |
| 14:45:09 | efried | melwitt: Cool. I wanted to make sure it got attention from someone who knows how to spell "quota" (which ain't me). | |
| 14:45:45 | dansmith | mriedem: question in here: https://review.openstack.org/#/c/590062/1 | |
| 14:45:54 | dansmith | I know it's a backport, but want to make sure I understand at least | |
| 14:46:09 | melwitt | efried: yeah, makes sense. I'm conflicted about the sentence they're proposing because it's not true that it's not possible to count keypairs. it's just, for legacy reasons (before pike) it always returned zero so I kept the behavior with the quota work in pike | |
| 14:46:14 | mdbooth | mriedem lyarwood: I think that approach is fundamentally good. We do need to think about the attachment cleanup after the last reschedule failure, but if we're not doing that then we've always been leaking there. | |
| 14:46:49 | efried | melwitt: Ah, okay, then perhaps it should just say, "For legacy reasons, this value is always zero. We'll fix it in a future microversion. Maybe. If you're lucky." | |
| 14:47:12 | mdbooth | That is, we can leak there right now, because it's possible to schedule to a compute and have it fail before touching volumes, so the 'reservation' still exists after failure. | |
| 14:47:17 | mdbooth | ^^^ * 3 == leak | |
| 14:47:20 | melwitt | efried: haha, yeah. | |
| 14:47:24 | mdbooth | Unless we already handle it | |
| 14:48:56 | mriedem | mdbooth: what i'd be most comfortable with is if we suspect we leak today, that we add a functional regression test for that which does reschedules with a volume attached, and asserts at the end of the reschedules when we get novalidhost (for max retries exceeded) that we've cleaned up all attachments to the volume | |
| 14:49:08 | mriedem | mdbooth: because this is too hairy to go on based on review alone | |
| 14:49:27 | mriedem | *functional test (not really a regression if it's always leaked) | |