| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-25 | |||
| 15:51:24 | bauzas | I mean, L95 rather | |
| 15:51:25 | dansmith | right, | |
| 15:51:37 | bauzas | so take my example | |
| 15:51:46 | bauzas | a driver reporting 1 VCPU | |
| 15:51:57 | bauzas | and an instance asking 2 vCPUs | |
| 15:52:26 | bauzas | if we use the allocation ratio of 999.0 then the compute will satisfy the request per the CoreFilter rules | |
| 15:53:15 | bauzas | https://github.com/openstack/nova/blob/master/nova/scheduler/filters/core_filter.py#L47 | |
| 15:53:23 | dansmith | you're saying that max_unit isn't scaled by allocation ratio, right? | |
| 15:53:27 | dansmith | (effectively) | |
| 15:54:30 | edleafe | bauzas: lines 54-56 are what I'm looking at | |
| 15:54:47 | cdent | dansmith: yes, and the questions are whether that is correct or not, and even if it not correct, if deprecation of some kind is required to deal | |
| 15:55:11 | bauzas | dansmith: wait, I think I'm puzzled | |
| 15:55:20 | dansmith | cdent: so the change from what we have now is that any one instance can't overcommit the host, effectively | |
| 15:55:21 | edleafe | I thought the issue was for operators who disabled CoreFilter | |
| 15:55:29 | cdent | dansmith: yes | |
| 15:55:44 | dansmith | cdent: like if the host has 1 vcpu to give, then you can have $allocation_ratio instances there, but not a multi-vcpu instance | |
| 15:55:47 | edleafe | They can currently put an 8VCPU request on a 2 VCPU host | |
| 15:56:02 | edleafe | But they won't be able to now, because of max_unit | |
| 15:56:08 | mdbooth | artom vladikr: volume_id in connection_info came up in review the other week, no? | |
| 15:56:16 | dansmith | edleafe: which is kinda what max_unit is for | |
| 15:56:23 | edleafe | dansmith: agreed | |
| 15:56:42 | mdbooth | artom vladikr: I'm currently doing a trawl of cinder volume drivers, and so far I've found at least 1 which doesn't set it. | |
| 15:56:44 | edleafe | dansmith: but bauzas and johnthetubaguy point out that that's a change w/o deprecation | |
| 15:56:47 | artom | mdbooth, yeah, you said it was a bad idea | |
| 15:56:48 | dansmith | edleafe: right now we just set it to the max, assuming a sane default, but we could change that later if someone has a good reason to allow such a thing | |
| 15:56:57 | mdbooth | artom vladikr: It's all very ad-hoc | |
| 15:57:06 | bauzas | what the hell | |
| 15:57:12 | artom | mdbooth, and then I went and re-checked with traget_dev/device_name, and it turns out that it works | |
| 15:57:15 | dansmith | edleafe: yeah, I can kinda see that, but.. it's also not very smart | |
| 15:57:31 | artom | mdbooth, my previous testing must have been as ad-hoc as colume id in cinder :) | |
| 15:57:33 | mdbooth | artom: Awesome. Wasn't sure what the resolution was, just thought I'd throw in a data point :) | |
| 15:57:47 | edleafe | dansmith: ok, so we have "bonkers", "crazy people", and "not very smart" to describe that | |
| 15:57:57 | edleafe | dansmith: you win the prize for most polite | |
| 15:57:59 | dansmith | edleafe: artom cdent: If someone had a problem with this, we could allow setting insanity_mode=on, which would report max_unit=N*allocation_ratio | |
| 15:58:18 | dansmith | edleafe: I'd like to change my answer to "fucking stupid" | |
| 15:58:49 | cdent | laaaaaadies and gentleman in a last minute change, dansmith wins | |
| 15:59:01 | bauzas | we don't use CoreFilter in the default list of filters | |
| 15:59:04 | bauzas | soooooo great | |
| 15:59:21 | dansmith | bauzas: that's not news, is it? | |
| 15:59:30 | bauzas | it is to me | |
| 15:59:38 | bauzas | I thought the gate was special | |
| 15:59:55 | bauzas | but that we allowed corefilter by default | |
| 16:00:01 | bauzas | I'm utterly surprised | |
| 16:00:35 | bauzas | so I'm just discovering that we *never* verified in Newton the CPU resources | |
| 16:00:41 | bauzas | unless you explicitely asked for | |
| 16:01:09 | bauzas | because if so, the CoreFilter would deny like the placement engine | |
| 16:01:20 | bauzas | that's even worst ! | |
| 16:01:28 | bauzas | holy shit | |
| 16:02:04 | bauzas | pardon my language | |
| 16:02:15 | bauzas | dansmith: ^ | |
| 16:02:32 | edleafe | bauzas: at least curse in French | |
| 16:02:45 | bauzas | so, just to be clear, lemme restate for the audience | |
| 16:02:47 | dansmith | even worse in what way? that it's a change? | |
| 16:03:07 | bauzas | in Newton, we totally accept that we can return a destination that doesn't satisfy the vCPU requirements | |
| 16:03:31 | bauzas | that's worst because that's changing the scheduler behaviour | |
| 16:03:48 | edleafe | Returning a destination is an implementation detail, not a behavior | |
| 16:04:09 | bauzas | placing an instance on a target is a behavioural choice | |
| 16:04:15 | dansmith | bauzas: but the whole point of placement is making better decisions right? | |
| 16:04:19 | edleafe | If anything, the change should reduce the number of retries | |
| 16:04:21 | dansmith | not less opinionated ones, but better ones | |
| 16:04:48 | edleafe | dansmith: jinx (sorta) | |
| 16:04:53 | dansmith | aye | |
| 16:06:32 | bauzas | edleafe: dansmith: the fun with that is that the problem still remains for RAMFilter | |
| 16:06:51 | dansmith | but really, | |
| 16:06:55 | bauzas | even if we say "fair enough, we don't need to advert operators to modify their ratios for cpu" | |
| 16:07:01 | bauzas | the problem remains for ram | |
| 16:07:02 | dansmith | overcommitting the whole node in a single VM is just nuts | |
| 16:07:10 | dansmith | eve more nuts than cpu | |
| 16:07:27 | dansmith | if we want to offer an insanity_mode config option, then let's do it | |
| 16:07:35 | dansmith | and make it clear what it is | |
| 16:07:49 | dansmith | I'll write the patch | |
| 16:08:23 | bauzas | I'm still wondering why we never used CoreFilter in that case | |
| 16:08:41 | dansmith | all I really care about is getting this patch in so we're not kicking it to another release | |
| 16:08:42 | edleafe | dansmith: "If you want to stick your fork into the electrical socket..." | |
| 16:08:51 | dansmith | edleafe: that can go in the help text | |
| 16:09:12 | bauzas | so you gonna rev'd my change? | |
| 16:09:30 | dansmith | it doesn't affect your change, right? it'd be a reporting change | |
| 16:09:48 | dansmith | but yes, if I need to write that patch to move us forward I'll do it | |
| 16:10:46 | edleafe | I think that's the wrong approach | |
| 16:11:18 | edleafe | It's not breaking anything | |
| 16:11:34 | dansmith | edleafe: you think what is wrong? allowing insanity_mode? | |
| 16:11:46 | edleafe | dansmith: yes | |
| 16:12:10 | edleafe | If someone is actively relying on this, they *should* fail | |
| 16:12:14 | dansmith | edleafe: IMHO, I'd wait for people to complain and then do it to get them out of hot water | |
| 16:12:15 | edleafe | and they should feel bad | |
| 16:12:18 | dansmith | hah | |
| 16:12:40 | dansmith | but if bauzas is going to rage quit now, then I could be persuaded to add it now | |
| 16:12:47 | dansmith | but I agree, it seems like making better choices to me | |
| 16:12:53 | bauzas | I'm not rage quitting, just exhausted | |
| 16:13:01 | dansmith | johnthetubaguy: were you one of the concerned porties/ | |
| 16:13:02 | dansmith | jeez | |
| 16:13:03 | dansmith | *parties? | |
| 16:13:12 | bauzas | because in particular, my f**** change is hitting a functional test issue | |
| 16:13:20 | bauzas | based on that particular assumption | |
| 16:13:30 | dansmith | bauzas: that's trivial to fix right? | |
| 16:13:42 | dansmith | that functional test is reporting something that depends on the broken behavior to work, right? | |
| 16:13:52 | bauzas | honestly? I dunno | |
| 16:14:10 | bauzas | we could fix up what the compute reports | |
| 16:14:18 | bauzas | that sounds the better approach | |
| 16:14:38 | dansmith | bauzas: you mean what compute reports in that functional test, yes? | |