| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-13 | |||
| 14:45:53 | dansmith | right, which means we chose the wrong one yeah? | |
| 14:45:59 | melwitt | if the operator has ever deleted a service and replaced it with another with the same host name | |
| 14:46:24 | melwitt | the field on Instance is 'services' so we get all of them and something in the API must be picking one of them | |
| 14:46:43 | melwitt | so we get a _from_db_object call per service returned from the join | |
| 14:47:00 | mriedem | the lazy load on instance.services is generic right? | |
| 14:47:50 | dansmith | melwitt: okay so we process all of them and try to generate uuids for all the previously-deleted ones? | |
| 14:47:53 | melwitt | what do you mean by generic? | |
| 14:48:15 | mriedem | in that we don't do a targeted query to get services by instance uuid and filter out deleted ones | |
| 14:48:17 | melwitt | dansmith: yeah process all of them and try to generate uuids for all that do not have a uuid (including the previously deleted ones) | |
| 14:48:23 | mriedem | we call instance_get_by_uuid and join on the services table | |
| 14:48:39 | dansmith | I see | |
| 14:49:20 | melwitt | mriedem: oh, for the lazy load I'm not sure. the scenario of the bug is the API is adding the 'services' expected attr before getting instances for the list | |
| 14:49:27 | mriedem | 'Service.binary == "nova-compute",' | |
| 14:49:27 | mriedem | primaryjoin='and_(Service.host == Instance.host,' | |
| 14:49:27 | mriedem | backref='services', | |
| 14:49:27 | mriedem | "Instance", | |
| 14:49:27 | mriedem | instance = orm.relationship( | |
| 14:49:27 | mriedem | and this is the backref | |
| 14:49:28 | mriedem | foreign_keys=host, | |
| 14:49:28 | mriedem | 'Instance.deleted == 0)', | |
| 14:49:29 | mriedem | ) | |
| 14:49:40 | mriedem | umm, so why would we load deleted services? | |
| 14:49:53 | mriedem | oh so not a lazy-load ok | |
| 14:50:04 | melwitt | because that's not filtering on Service.deleted? | |
| 14:51:30 | mriedem | query_prefix = query_prefix.options(joinedload(column)) | |
| 14:51:30 | mriedem | righ | |
| 14:52:26 | melwitt | why would the lazy load filter out deleted services? | |
| 14:53:14 | mriedem | maybe it doesn't, i thought it would use the backref | |
| 14:54:58 | melwitt | the backref says Instance.deleted == 0, not Service.deleted, so I thought it wouldn't filter out deleted service? maybe I'm missing something else | |
| 14:55:16 | mriedem | oh right, | |
| 14:55:20 | mriedem | glossed right over that | |
| 14:55:25 | mriedem | anyway, ignore me now | |
| 15:18:03 | openstackgerrit | Chen proposed openstack/nova master: fix cellv2 delete_host https://review.openstack.org/582171 | |
| 15:24:39 | mriedem | tssurya: i'm reminded that we can get a HostMappingNotFound when deleting a compute service https://review.openstack.org/#/c/554920/12/nova/api/openstack/compute/services.py@243 - we were just talking about that this week (or late last week), but i can't remember if a bug was reported for it? | |
| 15:25:57 | mriedem | especially since we lookup the service to delete by id, not by host (so we don't check for a host mapping when doing the service lookup) | |
| 15:29:09 | mriedem | i just opened a bug anyway https://bugs.launchpad.net/nova/+bug/1781625 | |
| 15:29:10 | openstack | Launchpad bug 1781625 in OpenStack Compute (nova) "Deleting compute service can fail with 500 error if HostMappingNotFound" [High,Triaged] - Assigned to Matt Riedemann (mriedem) | |
| 15:39:11 | tssurya | mriedem: sorry was out, yeah I had opened one actually, https://bugs.launchpad.net/nova/+bug/1780727 | |
| 15:39:12 | openstack | Launchpad bug 1780727 in OpenStack Compute (nova) "Handle HostMappingNotFound when deleting a service" [Undecided,Confirmed] - Assigned to Surya Seetharaman (tssurya) | |
| 15:39:32 | mriedem | ack | |
| 15:39:35 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Heal allocations with incomplete consumer information https://review.openstack.org/574488 | |
| 15:39:36 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Refactor _heal_instances_in_cell https://review.openstack.org/577896 | |
| 15:39:37 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use consumer generation in _heal_allocations_for_instance https://review.openstack.org/577905 | |
| 15:41:34 | mriedem | tssurya: do you plan on putting up a patch for that soon? if not, i can do that quickly today | |
| 15:41:49 | mriedem | we just backported the code that added the regression so i'd like to fix it soon | |
| 15:42:06 | tssurya | mriedem: yea please go ahead, my day is almost over | |
| 15:42:10 | mriedem | ok | |
| 15:42:17 | mriedem | in general, | |
| 15:42:20 | tssurya | sorry for not getting around to that | |
| 15:42:30 | mriedem | you don't need to assign bugs to yourself if you're not going to immediately work on them | |
| 15:42:36 | tssurya | ack | |
| 15:53:13 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Skip more rebuild tests for cells v1 job https://review.openstack.org/581717 | |
| 15:55:21 | dansmith | mriedem: how far up the stack on the numa set do you want to get before we pull the plug and let the stack of stuff on the bottom drain out? | |
| 15:58:45 | dansmith | I guess the scheduler one still has a -1 on it | |
| 16:00:16 | mriedem | the "scheduler" one is really a whole mess o stuff | |
| 16:00:29 | mriedem | including api changes and such that i'm saying should be split apart and the api change should land at the end of the series | |
| 16:00:45 | mriedem | right now he's got the api changes turned on but the libvirt driver changes are coming after that, which doesn't make sense | |
| 16:01:08 | dansmith | yeah, I saw he had rev'd that one and assumed it'd be ready now but looks like not | |
| 16:01:10 | mriedem | i'd be ok with draining the bottom half if we had agreement on the nullable question in the request spec | |
| 16:01:26 | dansmith | I was just thinking it might be good to clear that road before I go, but yeah | |
| 16:01:34 | mriedem | i think the 3 options i pointed out yesterday were: | |
| 16:02:09 | mriedem | 1. make RequestSpec.network_metadata nullable so that create_resource_requests can return None and we just blindly set it (which is what he'd be doing as coded today), | |
| 16:02:45 | mriedem | 2. keep reqspec.network_metadata non-nullable and then: (a) he has to return an 'empty' NetworkMetadata if no networking or (b) leave RequestSpec.network_metadata unset | |
| 16:02:54 | mriedem | i don't really like 2-a | |
| 16:03:22 | dansmith | yeah I mean 2b is what I was thinking | |
| 16:03:23 | mriedem | since you can't tell if the network metadata is 'empty' because the user requested no networking, or because the network just doesn't have physnet/tunneled | |
| 16:03:27 | dansmith | right | |
| 16:03:34 | mriedem | i'm cool with 2-b | |
| 16:03:40 | dansmith | or not 2-b? | |
| 16:03:52 | mriedem | i'll pretend that didn't happen | |
| 16:04:01 | dansmith | haha | |
| 16:04:17 | mriedem | if 2-b, we can drain the bottom half now and he'll just have to handle the 'if network_metadata is not None: reqspec.network_metadata = network_metadata" in the API | |
| 16:04:21 | mriedem | but that's easy enough, | |
| 16:04:30 | mriedem | and his hardware.py change are already unset-aware | |
| 16:04:31 | dansmith | kinda need him to agree to that though right? | |
| 16:04:42 | mriedem | stephenfin: ^ agree? | |
| 16:05:01 | mriedem | dansmith: do we? this is the week where you and i just take over everyone's changes | |
| 16:05:07 | dansmith | heh | |
| 16:05:22 | dansmith | stephenfin: it's four minutes past beer o clock, what are you still doing here? | |
| 16:05:40 | stephenfin | dansmith: I had beer o'clock last night and started late :( | |
| 16:05:50 | stephenfin | But we won our tag (rugby) final | |
| 16:06:02 | dansmith | heh | |
| 16:06:29 | stephenfin | a'ight, what do you need from me, mriedem? | |
| 16:06:39 | stephenfin | the null'ness thing is still up in the air, right? | |
| 16:06:51 | mriedem | stephenfin: summarized above | |
| 16:06:56 | mriedem | with dansmith and my preferred option | |
| 16:07:04 | mriedem | which also allows us to start draining the bottom half of the series | |
| 16:07:18 | mriedem | (11:01:38 AM) mriedem: i think the 3 options i pointed out yesterday were: | |
| 16:07:19 | mriedem | (11:02:49 AM) mriedem: 2. keep reqspec.network_metadata non-nullable and then: (a) he has to return an 'empty' NetworkMetadata if no networking or (b) leave RequestSpec.network_metadata unset | |
| 16:07:19 | mriedem | (11:02:13 AM) mriedem: 1. make RequestSpec.network_metadata nullable so that create_resource_requests can return None and we just blindly set it (which is what he'd be doing as coded today), | |
| 16:07:23 | mriedem | we both want 2-b | |
| 16:08:06 | stephenfin | I'm fine with 1 or 2-b. 2-a does feel weird | |
| 16:08:21 | mriedem | ok 2-b it is and we can start approving patches | |
| 16:09:10 | stephenfin | cool. I'll throw an eye over the last few patches again and address all those comments of yours I missed yesterday | |
| 16:10:40 | mriedem | dansmith: there +W up to https://review.openstack.org/#/c/564442/ | |
| 16:10:45 | mriedem | bbiab | |
| 16:11:19 | dansmith | woot | |
| 16:34:10 | stephenfin | dansmith: Thoughts on https://review.openstack.org/#/c/564444/14/nova/objects/fields.py@76 ? | |
| 16:34:50 | stephenfin | I'd read that comment as "these are the things we don't use directly in our models" but that's not what it means. Just remove that comment? | |
| 16:36:31 | dansmith | stephenfin: those are aliases, because much of that file was initially all in nova, | |