| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-20 | |||
| 14:14:26 | sdague | mdbooth: yes, the workers are processes | |
| 14:14:28 | jamespage | sdague: I have a simple patch to fix the nova package for Pike in Ubuntu and the UCA; that's not good for direct submission to nova as its not conditional i.e. its a blind add the flag (cause we know which qemu version will be in use for artful and xenial+ Pike UCA) | |
| 14:14:37 | mdbooth | sdague: I'll bet that's it... | |
| 14:14:41 | sdague | jamespage: gotcha | |
| 14:14:48 | mriedem | toabctl: done | |
| 14:15:02 | toabctl | mriedem, thx | |
| 14:15:06 | sdague | jamespage: link for where you injected it might be good regardless | |
| 14:15:17 | sdague | jamespage: then we can figure out the right conditional | |
| 14:15:30 | jamespage | sdague: looking the libvirt driver + images module to figure out the best way to pass that in conditionally - most version checking is done in driver, not in images... | |
| 14:15:34 | jamespage | sdague: | |
| 14:15:35 | jamespage | sure | |
| 14:15:56 | mriedem | jamespage: i was having the same problem when thinking about how to make that conditional | |
| 14:16:04 | mriedem | the driver knows the version, but way down in the bowels of the image code it doesn't | |
| 14:16:20 | jamespage | mriedem: yeah its awkward from that perspective | |
| 14:16:32 | jamespage | lemme attach my patch to the bug report | |
| 14:18:57 | mriedem | tasker: updated https://review.openstack.org/#/c/504260/ | |
| 14:20:09 | jamespage | mriedem: my thinking was to pass that down from driver to the image code as an optional param | |
| 14:20:46 | jamespage | testing that patch shortly | |
| 14:22:32 | mriedem | jamespage: yeah that is probably what i'd do | |
| 14:23:15 | mriedem | exit code is 1 when it fails, so that's not unique enough, | |
| 14:23:29 | mriedem | we could scrape the stderr for the message, and retry with the flag, but that's not fun either | |
| 14:23:49 | openstackgerrit | Dan Smith proposed openstack/nova master: Use improved instance_list module in compute API https://review.openstack.org/505418 | |
| 14:23:50 | openstackgerrit | Dan Smith proposed openstack/nova master: Remove legacy fault-loading routines https://review.openstack.org/505456 | |
| 14:23:52 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix a pagination logic bug in test_bug_1689692 https://review.openstack.org/505661 | |
| 14:24:09 | sahid | stephenfin: about https://review.openstack.org/#/c/501132/, it seems to me the commit message well reflects what is done on the patch | |
| 14:24:17 | sdague | mriedem: is there a reason to not set a CONST with the version on __init__ of the driver? | |
| 14:25:06 | stephenfin | sahid: It reflects what but not _why_. The why is what I care about (I can parse the what from reading the code) | |
| 14:25:38 | mriedem | sdague: and have the image code check the driver? | |
| 14:25:54 | stephenfin | If sean-k-mooney had questions that you took the time to address, the chances are that others will have the same question if they look at that patch in however many months/years time | |
| 14:26:00 | dansmith | mriedem: I think he means a global, which seems less good to me | |
| 14:26:09 | sdague | mriedem: well, I was thinking have the driver set a CONST in the image namespace | |
| 14:26:17 | mdbooth | sdague: Thanks. I've filed a bug against Nova, but suspect it's only actually fixable by using an external logging service. | |
| 14:26:43 | mdbooth | Or perhaps by having separate conductors writing to separate log files? | |
| 14:26:44 | openstackgerrit | Merged openstack/nova master: Skip more racy rebuild failing tests with cells v1 https://review.openstack.org/499001 | |
| 14:26:52 | stephenfin | sahid: See here for example https://review.openstack.org/#/c/479802/ | |
| 14:26:53 | mdbooth | We'd have to name them. | |
| 14:26:53 | sdague | because I thought we wanted image to be distinct | |
| 14:27:10 | sdague | mdbooth: I'm not sure what help a nova bug does here, it's really not fixable in nova | |
| 14:27:11 | cfriesen | is anyone aware of a scheduling issue in Pike when rescheduling instances that were originally booted as part of a multi-instance boot? There's a thread "Pike NOVA Disable and Live Migrate all instances" on the openstack list that seems to indicate a bug. | |
| 14:27:22 | openstackgerrit | Merged openstack/nova master: conf: Rename two VNC options https://review.openstack.org/498387 | |
| 14:27:36 | dansmith | mdbooth: python logging should be locking the fd, which is opened before the fork | |
| 14:27:49 | mdbooth | sdague: Well I found it in Nova, and it's definitely a thing. There's likely to be a better place to move it. | |
| 14:28:12 | mriedem | mdbooth: if you found it in nova, it's also then probably an issue in all other services | |
| 14:28:20 | mriedem | if it's logging related | |
| 14:28:42 | mdbooth | dansmith: That kind of locking could only work by preventing other conductors from opening the log file at all. | |
| 14:28:48 | openstackgerrit | Eric Berglund proposed openstack/nova master: Add PowerVM hypervisor configuration doc https://review.openstack.org/505665 | |
| 14:28:59 | mdbooth | Which would mean that it would block all conductors beyond the first. | |
| 14:29:17 | dansmith | mdbooth: the other processes don't open the log, they inherit it across the fork | |
| 14:30:00 | mdbooth | dansmith: Oh, interesting. However, the locking would still be python thread locking. | |
| 14:30:25 | dansmith | not if logging is locking the file | |
| 14:30:26 | cdent | cfriesen: I’ve been wondering if part of that was due somehow to the doubling stuff | |
| 14:30:53 | mdbooth | Unless the logger is taking and releasing an os lock for every write? | |
| 14:32:17 | sdague | mdbooth: I really think that once you push sufficiently large writes through the python logging buffer, this is just the python behavior | |
| 14:32:25 | sdague | and the only fix is don't do that | |
| 14:33:56 | openstackgerrit | sahid proposed openstack/os-vif master: ovs-hybrid: should permanently keep MAC entries https://review.openstack.org/501132 | |
| 14:35:03 | mdbooth | https://docs.python.org/3/library/multiprocessing.html#module-multiprocessing | |
| 14:35:34 | mdbooth | According to ^^^ in python 3 at least logging doesn't use external locks | |
| 14:36:11 | mdbooth | I guess that would make it a bug in oslo.log | |
| 14:36:40 | openstackgerrit | Merged openstack/nova master: [placement] Unregister the ResourceClass object https://review.openstack.org/502155 | |
| 14:36:50 | stephenfin | sahid: Lovely. +Wd | |
| 14:36:51 | mdbooth | Same for python 2 | |
| 14:37:01 | mdbooth | And I don't see oslo.log importing multiprocessing | |
| 14:37:11 | mdbooth | Well, it does, but it doesn't seem to use it | |
| 14:37:15 | mdbooth | That's pretty weird | |
| 14:38:10 | sdague | mdbooth: I expect that when you don't overrun the python logging natural buffer it just works | |
| 14:38:18 | sdague | and when you do, you get funkiness | |
| 14:38:28 | sdague | and I agree, if you want to fix it, you have to do it down in oslo.log | |
| 14:38:34 | mdbooth | sdague: We log exceptions, though, which are kinda arbitrarily large | |
| 14:38:42 | mdbooth | I don't think we want to stop doing that | |
| 14:39:02 | sdague | mdbooth: we do, but we've apparently been lucky thus far | |
| 14:39:03 | mdbooth | I'll move the bug to oslo.log and mention the multiprocess thing | |
| 14:39:50 | mdbooth | Assuming, that is, we don't want to open separate log files for conductor workers? | |
| 14:40:05 | mdbooth | Because that would be a nova fix, and possibly useful in its own right | |
| 14:40:42 | sdague | mdbooth: no, because that problem is equally theoretically a problem for all the other services as well | |
| 14:40:47 | sdague | except nova-compute | |
| 14:40:47 | dansmith | I definitely don't want separate logs for conductor workers | |
| 14:40:51 | dansmith | because..holy crap | |
| 14:40:54 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-traits master: Updated from global requirements https://review.openstack.org/503646 | |
| 14:40:57 | openstackgerrit | OpenStack Proposal Bot proposed openstack/os-vif master: Updated from global requirements https://review.openstack.org/502708 | |
| 14:41:08 | mdbooth | dansmith: Merged logs, ftw! ;) | |
| 14:41:09 | sdague | and it totally would wreck all the log injest systems people have | |
| 14:41:29 | mdbooth | True. | |
| 14:41:34 | mdbooth | oslo.log it is | |
| 14:43:30 | jamespage | mriedem: working a fix now | |
| 14:43:38 | cfriesen | cdent: that does seem to be the most likely suspect. points to a gap in our testing. | |
| 14:45:25 | openstackgerrit | Merged openstack/nova master: Update docs for _destroy_evacuated_instances https://review.openstack.org/500144 | |
| 14:46:00 | mriedem | jamespage: i'm testing sdague's idea too | |
| 14:46:04 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: check qemu version when calling qemu-img info https://review.openstack.org/505673 | |
| 14:46:34 | openstackgerrit | Elod Illes proposed openstack/nova master: Add instance.interface_attach notification https://review.openstack.org/503089 | |
| 14:46:58 | cdent | cfriesen: gaps in testing is a bit of a trend, but gibi is fixing it ;) | |
| 14:47:51 | openstackgerrit | Chris Dent proposed openstack/nova master: Add functional test for two-cell scheduler behaviors https://review.openstack.org/452006 | |
| 14:47:58 | mriedem | jamespage: testing here https://review.openstack.org/#/c/505674/ | |
| 14:48:01 | sdague | mriedem: you got a devstack patch depends on that? | |
| 14:48:16 | mriedem | yes ^ | |
| 14:48:47 | sdague | ah cool | |
| 14:49:27 | gibi | cdent: do you mean I should add a test case which boots multiple instances with a single boot command then try to migrate them? ;) | |
| 14:49:56 | cdent | I meant you were fixing the trend more generally but if you’re feeling motivated :) | |
| 14:50:24 | dansmith | mriedem: if you're okay with it I'm just going to fast approve all these unregister patches as they're just mechanical search/replace: https://review.openstack.org/#/c/502157/5 | |
| 14:51:24 | openstackgerrit | Merged openstack/nova master: Add @targets_cell for live_migrate_instance method in conductor https://review.openstack.org/503601 | |
| 14:51:56 | mriedem | dansmith: i can go through them quick | |