| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-12 | |||
| 09:12:33 | efried | stephenfin: or what we could do is just do both GET /a_c queries, cat the vcpu ones after the pcpu ones, and run the filter once. | |
| 09:12:55 | bauzas | we are technically sharding the capacity, right? | |
| 09:13:09 | stephenfin | efried: I thought of that but doesn't the filter scheduler shuffle the allocation candidates? | |
| 09:13:20 | efried | placement does the shuffle | |
| 09:13:24 | stephenfin | ohhhh | |
| 09:13:29 | efried | I don't know if the filter scheduler also does | |
| 09:14:30 | efried | stephenfin: this scenario only applies if PCPUs were requested, right? | |
| 09:14:43 | efried | Meaning if you have the granny switch flipped *and* you're on a flavor that's requesting dedicated? | |
| 09:15:55 | efried | Doing a double GET /a_c would be expensive, so it would be nice if we were only doing it in a corner case. | |
| 09:18:57 | gibi | efried: if there is more than one candidate for a single host then the scheduler will only look for the first candidate (per host) | |
| 09:19:30 | openstackgerrit | Eric Fried proposed openstack/nova master: Support reverting migration / resize with bandwidth https://review.opendev.org/676140 | |
| 09:20:21 | gibi | efried: that will complicate my work a bit ^^ | |
| 09:21:03 | efried | gibi: just a rebase, shouldn't affect anything | |
| 09:21:48 | efried | gibi: only one candidate per host -- yeah, that kills it (<== stephenfin) | |
| 09:23:06 | gibi | efried: I have ongoing work in that series locally, so I have to restach top of the rebase | |
| 09:23:11 | gibi | efried: I will manage | |
| 09:23:14 | efried | oh shit, I'm sorry gibi | |
| 09:23:25 | luyao | sean-k-mooney: Hi Sean, are you around. | |
| 09:23:29 | gibi | efried: no worries, I think I can do some interactive rebase magic | |
| 09:23:40 | efried | gibi: I didn't rebase anything except the first patch | |
| 09:23:57 | efried | so you may just be able to proceed as you were | |
| 09:24:02 | gibi | efried: ack. I will rebase my work on top of that | |
| 09:24:24 | efried | I was just trying to do what little I could to get the gate moving | |
| 09:24:34 | gibi | efried: sure, I understand your motives | |
| 09:24:39 | efried | that patch had already failed py37 on the f'in innodb thing | |
| 09:24:41 | stephenfin | efried, gibi: That shouldn't matter, should it? | |
| 09:25:04 | stephenfin | I mean, if a host is reporting PCPU then by definition we shouldn't try using it for VCPU | |
| 09:25:07 | efried | stephenfin: It makes it a very imperfect solution, but I guess it still narrows the window | |
| 09:25:11 | efried | oh | |
| 09:25:13 | gibi | efried: no worries. I guess I'm overreacted | |
| 09:25:30 | efried | that's a good point stephenfin... I think | |
| 09:25:48 | stephenfin | the crucial thing is does the filter scheduler shuffle hosts? | |
| 09:25:57 | stephenfin | *shuffle allocation candidates | |
| 09:25:58 | gibi | stephenfin: I see. so either there will be more than once candidate but then we are OK with the first. Or there only be candidate with VCPU and then we fallback to that | |
| 09:26:17 | gibi | stephenfin: there is some processing involved let me find a link | |
| 09:26:34 | efried | gibi: actually, I'm not sure if you had hoped to merge more in that series than is already +Wed, but it might be best to wait until after FF to do *anything* else. Esp since gate resources are going to be really scarce for the forseeable future. | |
| 09:27:29 | gibi | efried: Matt and I has hopes for the series. He said he could be able to +2 the rest today. But I accept the bad news if the gate is sad | |
| 09:27:55 | stephenfin | gibi: I think so. Either we'll return a candidate for host from the PCPU query (host has 'cpu_dedicated_set' configured)... | |
| 09:28:02 | stephenfin | or we'll return a candidate for a host from the VCPU query (host has 'vcpu_pin_set' or 'cpu_shared_set' configured or nothing)... | |
| 09:28:22 | gibi | stephenfin: https://github.com/openstack/nova/blob/5fa49cd0b8b6015aa61b4312b2ce1ae780c42c64/nova/scheduler/manager.py#L176 | |
| 09:28:34 | stephenfin | or we'll return the same candidate for both (host has 'cpu_dedicated_set' *and* 'cpu_shared_set') but ignore the one from the VCPU request | |
| 09:28:44 | gibi | stephenfin: sound OK to me | |
| 09:28:44 | stephenfin | because the PCPU query was done first | |
| 09:28:49 | stephenfin | schweet | |
| 09:28:53 | gibi | stephenfin: but look at the link. We bould dicts :/ | |
| 09:29:38 | bauzas | stephenfin: gibi: efried: alex_xu: seriously, I'm still continuing to consider we overthink the problem | |
| 09:29:56 | gibi | stephenfin: hm, that dict is not a problem the allocation requests still in a list | |
| 09:30:06 | efried | gibi: but only per host | |
| 09:30:09 | bauzas | I mean, once the operator opts in, he litterally shards his cloud in twice | |
| 09:30:28 | gibi | efried: but the fallback is needed per host, isn't it? | |
| 09:30:36 | bauzas | if we say the opt-in is per compute, that means the sharding can be long | |
| 09:30:53 | efried | gibi: actually, maybe so. | |
| 09:31:18 | bauzas | that's why the more I see our convos, the best i think it would be to just consider a global option that would dictate all computes sending PCPU inventories | |
| 09:31:24 | gibi | stephenfin: we take the first ar per host https://github.com/openstack/nova/blob/5fa49cd0b8b6015aa61b4312b2ce1ae780c42c64/nova/scheduler/filter_scheduler.py#L238 | |
| 09:31:27 | efried | gibi, stephenfin: as long as it's okay if we try has-only-VCPU candidates before we try a has-PCPU candidate | |
| 09:31:52 | stephenfin | efried: It's not an issue. As noted, the NUMATopologyFilter or virt driver will fail that request | |
| 09:31:56 | efried | ...which should still be fine, because the ntf will remove... yeah | |
| 09:32:38 | efried | so again, stephenfin, under what circumstances are we worried about this? | |
| 09:32:39 | stephenfin | bauzas: We sort of have that - the compute nodes won't send PCPU inventory until 'cpu_dedicated_set' is set | |
| 09:32:58 | bauzas | stephenfin: I know, my opinion is just "doc it, dude" | |
| 09:33:01 | efried | If it's only in the "sharded" case as bauzas says, then I think it may be acceptable to take the double GET /a_c hit. | |
| 09:33:37 | stephenfin | efried: The current solution (where we only try the second 'GET /a_c' if the first doesn't return anything) or the modified one we're suggesting here (where we always make two requests and concat them) ? | |
| 09:33:50 | efried | but if it can happen in a fully-pre or fully-post cloud, we should think it through more carefully. | |
| 09:34:06 | cdent | in train two a_c hits is pretty cheap | |
| 09:34:06 | bauzas | adding more complexity into a abstract API that would add special-cases in order to fix a very temporary situation is IMHO like using a big hammer | |
| 09:34:07 | efried | I'm talking about doing double GET /a_c preemptively. | |
| 09:34:28 | efried | cdent: even for CERN? | |
| 09:34:35 | cdent | yeah | |
| 09:34:49 | cdent | <1 sec per | |
| 09:34:56 | stephenfin | oh, then I can't think there are huge issues outside of always making an extra requests to placement | |
| 09:34:56 | efried | and it's not just the API calls; we'd be doubling the amount of memory we're using to store the candidates | |
| 09:35:07 | stephenfin | which cdent is saying isn't a huge issue | |
| 09:35:08 | efried | I suppose we could cut the ?limit in half... but I don't know if that's a good idea. | |
| 09:35:24 | stephenfin | and only happens if we're using pinning | |
| 09:35:37 | stephenfin | efried: alex_xu had suggested a config option to disable the double lookup | |
| 09:35:47 | efried | that's also a good idea | |
| 09:35:51 | stephenfin | or rather, disable the fallback | |
| 09:35:52 | efried | workaround | |
| 09:35:59 | stephenfin | workaround option, killed in the next release | |
| 09:36:00 | stephenfin | yeah | |
| 09:36:21 | cdent | (the perf issue for a double a_c is the nova-side processing of the results, not the placemnt-side generaiton) | |
| 09:36:25 | stephenfin | just for CERN and other people with huge clouds who might run into this issue | |
| 09:36:50 | efried | so: workaround disabled, you might get nvh when you're out of "real" PCPUs; workaround enabled, you're taking an extra perf/mem hit (small though it may be) any time we... what? | |
| 09:37:04 | efried | ...detect that there are any PCPUs in the cloud? | |
| 09:37:16 | efried | still trying to understand when we would do the double thing | |
| 09:37:36 | stephenfin | anytime you try to create or move a pinned instance | |
| 09:37:57 | efried | mm | |
| 09:38:09 | stephenfin | for one cycle | |
| 09:38:12 | efried | mm | |
| 09:39:32 | bauzas | yeah, that's what I'm trying to say : dudes, if you really want to support sharded clouds, it will come with a performance penalty | |
| 09:39:48 | bauzas | in this case, make it super temporary and super clear that this is a workaround | |
| 09:40:05 | efried | bauzas: problem is, it's not just sharded that you get hit on | |
| 09:40:14 | efried | in this case if you fully upgrade, you still get hit | |
| 09:40:15 | efried | but | |
| 09:40:20 | efried | in that case you should disable the workaround | |
| 09:40:22 | efried | because you're done | |
| 09:40:24 | efried | and don't need it. | |
| 09:40:24 | efried | so | |
| 09:40:27 | bauzas | that will leave the operator a choice between a quick rolling upgrade (and no performance hit) or a slow rolling upgrade with some performance degradation he could estimate | |
| 09:40:30 | efried | I think this is acceptable. | |