| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-02 | |||
| 15:56:21 | cdent | but if we are discounting shared providers, for now, we don’t need to worry about that | |
| 15:56:46 | mriedem | doesn't seem like it would be hard to keep accounting for shared providers | |
| 15:56:53 | mriedem | if new_rp_uuids is empty, we know something is afoot | |
| 15:57:22 | mriedem | i think i need to check out gibi | |
| 15:57:30 | mriedem | gibi's new test for resize to same host first | |
| 15:57:39 | cdent | a) I’m thinkin in terms of trying to iterate, b) I think the code in jay’s stack is incorrect for shared providers anyway, based on some of the things dansmith said earlier in the week | |
| 15:57:40 | mriedem | and then could start playing with a change that builds on that | |
| 15:58:20 | dansmith | cdent: I assume so as well | |
| 15:58:38 | gibi | mriedem: that test needs a bit of update based on the above discussion. I used assert(max(old, new), usage) type of asserts but you agreed about old+new as I see | |
| 15:59:26 | cdent | gibi: you’re near the end of your day, yeah? | |
| 15:59:47 | gibi | cdent: yeah, and on a train with spotty conenction | |
| 16:00:03 | cdent | then I won’t say “maybe you should do the spike” :0 | |
| 16:00:19 | gibi | at least not today | |
| 16:00:27 | gibi | :) | |
| 16:00:50 | mriedem | so we know the scheduler report client doesn't know about shared storage providers, and will trample your shared storage provider allocations saying any disk consumed is local to the compute node provider | |
| 16:00:51 | dansmith | mriedem: we could probably help by landing his two bottom patches at least | |
| 16:01:01 | dansmith | mriedem: I +2d the bottom one this morning and can look at the next one now | |
| 16:01:15 | dansmith | mriedem: correct | |
| 16:01:33 | mriedem | so before we can say shared storage is supported, we have to fix that, and all of your computes have to be upgraded to the level that has that fix | |
| 16:01:44 | mriedem | meanwhile we're not doing a min service version check in the scheduler to account for that | |
| 16:02:19 | bauzas | are folks discussing of https://bugs.launchpad.net/nova/+bug/1707256 ? | |
| 16:02:19 | openstack | Launchpad bug 1707256 in OpenStack Compute (nova) "Scheduler report client does not account for shared resource providers" [High,Confirmed] - Assigned to Jay Pipes (jaypipes) | |
| 16:02:28 | mriedem | we're discussing all things | |
| 16:02:41 | bauzas | all things | |
| 16:02:55 | bauzas | :) | |
| 16:03:06 | mriedem | if we say, f it, shared storage isn't supported in pike, then we just fix the resize to same host thing by doubling allocations in the scheduler, right? | |
| 16:03:56 | dansmith | mriedem: yeah and ideally gracefully subtracting in the compute when done | |
| 16:04:13 | dansmith | mriedem: we should be as graceful as possible though so we don't screw up queens nodes that may do it right | |
| 16:04:16 | mriedem | on confirm resize? | |
| 16:04:19 | dansmith | yeah | |
| 16:04:43 | mriedem | ma | |
| 16:04:43 | mriedem | hurts | |
| 16:04:44 | mriedem | brain | |
| 16:06:58 | dansmith | this is oregon, | |
| 16:07:05 | dansmith | we have better things at our disposal | |
| 16:07:21 | cdent | my stash is cashed | |
| 16:08:24 | dansmith | mriedem: so I can start looking at making it do the right thing on the confirm if you want | |
| 16:09:10 | dansmith | I wish his top patch didn't marry the two things he's fixing together | |
| 16:09:18 | dansmith | the ocata compat and the resize_confirm fix | |
| 16:09:27 | dansmith | I'll put mine on top at least | |
| 16:10:46 | mriedem | i'll check out the bottom change that fixes PUT to overwrite all allocations - already did the other day and it made sense, seems simple, | |
| 16:11:00 | bauzas | I just +Wd it | |
| 16:11:05 | mriedem | also need to check out gibi's resize to same host tests, and then i was going to tinker with some of the code in https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L200 | |
| 16:12:24 | cfriesen | mriedem: would we be looking at backporting any shared storage accounting fix back to pike? or only fixing in Q? (or does that sort of depend on what the fix looks like?) | |
| 16:13:06 | mriedem | it depends, but also not sure since the scheduler isn't making any distinction based on the version of the compute serivce | |
| 16:13:07 | mriedem | *Service | |
| 16:13:22 | mriedem | so it kind of sucks to say, 'well make sure you have this fix and everything is upgraded first' | |
| 16:13:45 | cdent | dansmith: are you doing just the undoubling, or also the doubling? | |
| 16:13:58 | dansmith | cdent: we're already doubling right? | |
| 16:14:15 | mriedem | we only double if >1 provider | |
| 16:14:18 | cdent | we don’t have info to double | |
| 16:14:20 | cdent | yeah, what mriedem says | |
| 16:14:24 | dansmith | ah okay | |
| 16:14:30 | mriedem | we can figure out the resize to same host case | |
| 16:14:33 | dansmith | then yeah I'll look at that too | |
| 16:14:37 | mriedem | in the scheduler, that's what i was going to poke at | |
| 16:17:22 | dansmith | cdent: can you look at my comment on the top one just now? | |
| 16:18:16 | cdent | oh yeah that. every single time I read that chunk of code I get confused | |
| 16:18:20 | cdent | they are different structures | |
| 16:18:46 | cdent | i’m not sure how we ended up there | |
| 16:19:07 | dansmith | cdent: they're supposed to be different you mean? | |
| 16:19:11 | dansmith | GET vs PUT? | |
| 16:19:15 | cdent | yeah | |
| 16:19:27 | dansmith | how is that restful? | |
| 16:19:42 | cdent | it isn’t very | |
| 16:19:50 | dansmith | okay, glad we agree on that :) | |
| 16:19:55 | cdent | but there was a disagreement between you/me and jay at some point | |
| 16:20:10 | cdent | you and i wanted the GET to return a dict because it made processing the response easy | |
| 16:20:19 | cdent | (this was at the end of last summer or so) | |
| 16:20:24 | dansmith | not about that, that I know of, but maybe it ended up with a disparity as a side effect? | |
| 16:20:46 | cdent | side effect of? | |
| 16:21:08 | dansmith | meaning, I would never argue for GET/PUT to be different structures, so I'm wondering if we just never made PUT match the changed GET or something and nobody realized? | |
| 16:21:55 | cdent | oh, possibly? | |
| 16:22:05 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 16:22:20 | dansmith | mriedem: you're working on the scheduler doubling or what? I kinda need to do it to test any changes I make for the un-doubling, so I might as well do it unless you've already started | |
| 16:22:30 | cdent | but I also think there was some dislike (I don’t recall why) of the rp uuid being a key in the POST | |
| 16:22:36 | dansmith | cdent: that's pretty disappointing | |
| 16:22:41 | dansmith | regardless of how we ended up here | |
| 16:22:45 | cdent | indeed | |
| 16:22:58 | cdent | there are quite a lot of disappointments | |
| 16:23:12 | mriedem | dansmith: was just starting with a unit test for the scheduler | |
| 16:23:26 | dansmith | mriedem: okay I guess I'll hold off them | |
| 16:23:27 | dansmith | *then | |
| 16:23:59 | mriedem | i'll throw up the wip shortly | |
| 16:24:00 | dansmith | I'm really kinda confused about this doubling anyway | |
| 16:25:17 | dansmith | I guess it's the attempt to account for shared storage that makes this complicated, | |
| 16:25:22 | dansmith | and which avoids doubling for same-host | |
| 16:25:30 | cdent | dansmith: yes | |
| 16:26:03 | mriedem | yup | |
| 16:26:38 | mriedem | "Remove any allocations against resource providers that are | |
| 16:26:38 | mriedem | # already allocated against on the source host (like shared storage | |
| 16:26:39 | mriedem | # providers)" | |
| 16:26:49 | mriedem | so i guess the intention was to specifically not double up shared storage | |
| 16:26:51 | mriedem | on a mov | |
| 16:26:52 | mriedem | *move | |
| 16:26:55 | dansmith | which is wrong anyway | |
| 16:27:00 | dansmith | for certain types of shared storage | |
| 16:27:11 | mriedem | seemed like the right idea at the time?! | |
| 16:27:13 | dansmith | it's not wrong for a volume, but is wrong for a compute node using ceph | |
| 16:27:21 | mriedem | which was 72 hours ago? | |