| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-28 | |||
| 16:27:31 | leakypipes | mriedem: grr, shitbuckets. | |
| 16:27:39 | leakypipes | mriedem: we forgot about resize to same host. :( | |
| 16:27:41 | mriedem | literally? | |
| 16:28:06 | leakypipes | mriedem: so when doing a resize same host, we'll end up deleting the entire allocation except for the shared providers in the patch I just put up. | |
| 16:28:29 | cdent | yeehaw | |
| 16:28:41 | mriedem | just check instance.host == CONF.host? | |
| 16:29:12 | leakypipes | mriedem: yeah. though... in the thing we added for scheduler claiming a double-up allocation, did we account for resize to same host? :( | |
| 16:29:49 | mriedem | yes | |
| 16:29:53 | mriedem | it comares the rp uuid right? | |
| 16:29:56 | mriedem | does a set difference | |
| 16:30:15 | mriedem | pretty sure i thought about that when reviewing and came to the conclusion it's handled by the set difference on rp uuid | |
| 16:30:28 | cdent | yeah, but isn’t the point that you still need a doubling and since the rp uuid is the same, you need to double within the allocation, not adjacent to | |
| 16:30:38 | mriedem | no | |
| 16:30:39 | leakypipes | mriedem: yeah, but the issue is that we need to create an allocation for the *same provider* but the resource amounts are added together (old and new amount) | |
| 16:30:44 | mriedem | the double is for the source and target | |
| 16:30:49 | superdan | yes | |
| 16:30:51 | mriedem | so you don't lose the source when putting allocations for the target | |
| 16:30:53 | superdan | but if resize to same host, you still have two copies | |
| 16:30:59 | superdan | so you need twice the allocation | |
| 16:31:06 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/488034 | |
| 16:31:26 | leakypipes | superdan: well, not twice the allocation, but the new amount of resources added to the old amount of resources, but on the same resource provider. :( | |
| 16:31:26 | mriedem | i guess you do need to account for the resize up flavor resource class stuff | |
| 16:31:46 | leakypipes | I hate my life. | |
| 16:31:47 | superdan | leakypipes: yes, I mean the sum of the allocations of course :) | |
| 16:31:48 | mriedem | so is it just that we aren't doing the correct 'new' flavor allocation when resize to same host? | |
| 16:31:56 | mriedem | is it sum? | |
| 16:31:56 | superdan | leakypipes: I've been using "doubled up" to mean "the sum" | |
| 16:32:04 | mriedem | for resize to same host i mean | |
| 16:32:10 | leakypipes | superdan: understood. | |
| 16:32:23 | superdan | it's definitely sum for disk, but I don't think we should distinguish, we should just sum everything | |
| 16:32:28 | mriedem | ok | |
| 16:32:30 | leakypipes | mriedem: yeah, it's sum | |
| 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 | |