| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-28 | |||
| 16:32:33 | mriedem | so let's open a bug? | |
| 16:32:42 | mriedem | tag it with pike-rc-potential | |
| 16:32:48 | superdan | if you're resizing to a new flavor with more disk, but fewer cpu/mem, you have to protect the older larger amount of resource | |
| 16:32:57 | mriedem | superdan: good point | |
| 16:33:44 | cdent | leakypipes: on the resourcce tracker side, when the resize confirm wants to delete everything on itself, would it be safe/okay to let that happen, and then immediately call into the _update routines so it would then write a new allocation for the correct size? | |
| 16:33:50 | leakypipes | ffs, we're just going to end up porting all the craziness and conditionals from the resource_tracker.py module into the scheduler report client for this stuff :( | |
| 16:34:49 | leakypipes | cdent: might be, yes... need to think through it. | |
| 16:35:06 | mriedem | cdent: _update only updates inventory | |
| 16:35:22 | mriedem | if you're talking about ResourceTracker._update | |
| 16:35:25 | mriedem | maybe we should rename that :) | |
| 16:35:27 | cdent | I don’t mean _update specifically say _update_* perhaps | |
| 16:35:31 | mriedem | heh | |
| 16:35:36 | cdent | wherever the regular allocations happens | |
| 16:35:50 | mriedem | _update_something_for_instance_maybe | |
| 16:36:06 | cdent | anyway, there’s another wrinkle near this that I wanted to ask about: are those regular allocation update shared providers aware yet? | |
| 16:36:25 | mriedem | don't think so | |
| 16:36:44 | figleaf | cdent: no, but they won't delete the shared provider allocation, right? | |
| 16:36:54 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L919 | |
| 16:37:12 | mriedem | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1024 | |
| 16:38:05 | cdent | figleaf: if we write any allocation at all, it will replace. so it depends on what we’re creating, to write | |
| 16:38:12 | figleaf | leakypipes: sorry, but the nuns beat that stuff into me | |
| 16:38:21 | leakypipes | mriedem: right, and the idea was to not have to call put_allocations() once the placement-claims stuff was done. | |
| 16:38:33 | cdent | I’m thinking in terms of these vaunted “heals” we love. Is a heal going to be correct in the face of a shared provider | |
| 16:38:43 | figleaf | cdent: OIC what you're getting at | |
| 16:39:24 | figleaf | cdent: yeah, that would only work if allocations were consumer/rp specific | |
| 16:39:42 | mriedem | leakypipes: yeah, and my original understanding from 4+ months ago was we'd put code into the computes that wouldn't do anything with allocations if they were already created by the scheduler, and we'd not do claims in the scheduler until all computes had that code | |
| 16:39:48 | mriedem | but then that went away | |
| 16:41:06 | superdan | mriedem: that code _is_ in the computes though | |
| 16:41:28 | superdan | well, part of it | |
| 16:41:32 | mriedem | superdan: the diff thing in the report client you mean right? | |
| 16:41:40 | mriedem | my_allocations vs current_allocations | |
| 16:41:41 | mriedem | ? | |
| 16:41:47 | superdan | yes, but the thing is, | |
| 16:41:55 | mriedem | my_allocations doesn't account for shared storage | |
| 16:41:56 | mriedem | right? | |
| 16:41:57 | superdan | we still have to have the healing from the compute node side regardless | |
| 16:44:36 | leakypipes | mriedem: if you create a bug, I'll get to work on a fix. | |
| 16:45:12 | mriedem | which bug are we talking about? the fact we don't sum allocations when resize to same host in scheduler? | |
| 16:45:36 | mriedem | sounds like multiple bugs | |
| 16:45:44 | mriedem | because of the self-heal issue, or is that the one from yesterday? | |
| 16:46:15 | leakypipes | mriedem: no, sorry, I was referring to addressing the TODOs left by cdent around the shared providers (the links you pasted above to the report.py module) | |
| 16:46:35 | cfriesen_ | leakypipes: you could always just stop supporting resize-to-same-host and let all the users scream. /s | |
| 16:46:51 | leakypipes | cfriesen_: cool with me. | |
| 16:47:03 | melwitt | yeah really. especially since they complain that it doesn't mean "force resize to same host" | |
| 16:47:06 | mriedem | leakypipes: ok, but we also have a bug for the resize to same host wrt the scheduler double fudge goodness right? | |
| 16:47:20 | mriedem | let's call it double fudge now | |
| 16:47:28 | leakypipes | mriedem: yeah :) | |
| 16:47:33 | leakypipes | two different bugs | |
| 16:47:45 | mriedem | ok | |
| 16:47:48 | mriedem | will do in a bit | |
| 16:49:24 | cdent | leakypipes: could you please remind me what the _provider_aggregate_map in in the report client was destined for? | |
| 16:49:57 | leakypipes | mriedem: thanks Matt | |
| 16:50:19 | leakypipes | cdent: precisely the case for determining if the compute node was associated to providers of shared resources. | |
| 16:50:43 | cdent | so is it of use in one of these two bugs? | |
| 16:51:50 | openstack | Launchpad bug 1707252 in OpenStack Compute (nova) "Claims in the scheduler does not account for doubling allocations on resize to same host" [Medium,Confirmed] | |
| 16:51:50 | mriedem | leakypipes: bug the first https://bugs.launchpad.net/nova/+bug/1707252 | |
| 16:51:58 | mriedem | superdan: ^ make sure i made sense in there | |
| 16:52:16 | superdan | hang on, I'm reviewing leakypipes' other patch | |
| 16:55:30 | mriedem | even with the fake rpc sleep thing in https://review.openstack.org/#/c/488500/ i'm not seeing the case that the source node is not in the allocations when it goes to delete the allocations for the instance | |
| 16:57:23 | superdan | mriedem: but we know it can happen | |
| 16:57:49 | superdan | so I guess if you want to just leave it, then that's fine, but it'll be super hard to track down if it really happens | |
| 16:58:18 | superdan | mriedem: yes I think your bug text makes sense | |
| 16:58:38 | openstackgerrit | Sean Dague proposed openstack/nova master: Add cinder keystone client opts to config reference https://review.openstack.org/488530 | |
| 16:59:17 | mriedem | superdan: leave what? the part of my patch that returns if the source node isn't in the current allocations? | |
| 16:59:39 | mriedem | the point was really just safe guarding against that and providing logging in case it happens | |
| 16:59:43 | superdan | mriedem: leave it like not apply your patch | |
| 16:59:49 | mriedem | oh | |
| 16:59:51 | mriedem | yeah | |
| 16:59:58 | mriedem | i figured we'd sort that out later | |
| 17:00:04 | superdan | you said this morning something indicating you might want to not commit this patch | |
| 17:00:07 | superdan | I think we should but... | |
| 17:00:27 | mriedem | i think it would be ok to do it - it's not going to impact performance by pulling the allocations again, because this code only runs from the compute periodic | |
| 17:00:52 | mriedem | i intentionally didn't pass the rp uuid in from the scheduler so the scheduler won't double check | |
| 17:01:09 | superdan | okay | |
| 17:03:54 | openstack | Launchpad bug 1707256 in OpenStack Compute (nova) "Scheduler report client is not account for shared resource providers" [High,Confirmed] | |
| 17:03:54 | mriedem | leakypipes: superdan: cdent: here is the other bug https://bugs.launchpad.net/nova/+bug/1707256 | |
| 17:04:04 | mriedem | omg me fail english | |
| 17:04:08 | superdan | is not account? | |
| 17:04:10 | superdan | mah | |
| 17:04:10 | superdan | oh | |
| 17:04:11 | superdan | god | |
| 17:04:17 | superdan | one for the record books kids | |
| 17:04:31 | superdan | let it be known henceforth that mriedem is not perfect | |
| 17:04:35 | mriedem | i was surrounded by a whirlwind of 6 year old in pink | |
| 17:04:41 | mriedem | rattled me | |
| 17:05:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Remove compatibility code for flavors https://review.openstack.org/460377 | |
| 17:08:55 | cdent | you reall do have my disease mriedem : “compute node things it needs” | |
| 17:09:07 | sdague | mriedem: for one glorious moment, 0 bugs in New state - https://bugs.launchpad.net/nova/+bugs?search=Search&field.status=New | |
| 17:09:15 | mriedem | o.O | |
| 17:09:31 | mriedem | sdague: did you just invalidate everything? :) | |
| 17:09:53 | purplerbot | <cdent> so is it of use in one of these two bugs? [2017-07-28 16:50:43.278883] [n 3edk] | |
| 17:09:53 | purplerbot | <leakypipes> cdent: precisely the case for determining if the compute node was associated to providers of shared resources. [2017-07-28 16:50:19.034601] [n 3rxI] | |
| 17:09:53 | cdent | leakypipes: [t 3rxI] [t 3edk] | |
| 17:10:17 | sdague | I read every new bug, moved a bunch of them to Incomplete with specific questions, found all the ones that really were going to need specs and linked them to the specs process and put them in Opinion | |
| 17:10:20 | sdague | duped a few | |
| 17:10:25 | leakypipes | cdent: yes, the latter. | |
| 17:10:28 | sdague | found some that had been fixed | |
| 17:10:31 | mriedem | sdague: thanks | |
| 17:10:42 | sdague | so, it's mostly a legit cleaning of the New state | |
| 17:11:02 | cdent | cool, thanks leakypipes, I figured as much but wanted to be sure | |