| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-16 | |||
| 16:32:22 | stephenfin | This could really do with a call, I think | |
| 16:34:47 | sean-k-mooney | probably | |
| 16:36:48 | bauzas | stephenfin: I have to leave in a few, but I'll summarize my thoughts | |
| 16:37:19 | bauzas | we could just leave existing options as they are and leave inventories be VCPU | |
| 16:37:23 | bauzas | but | |
| 16:38:15 | bauzas | once operator sets config options (and then we can discuss on this specific trigger), then we do a reshape for this host and split VCPU inventory into VCPU and PCPU inventories | |
| 16:38:23 | bauzas | and we accordingly move allocations | |
| 16:38:51 | bauzas | with the slight detail that we verify resources *before* doing the reshape so we can raise an exception | |
| 16:38:59 | bauzas | at compute startup | |
| 16:39:55 | bauzas | for requests, we somehow need to make sure that we transform requests by using PCPU based on a specific point in time | |
| 16:40:00 | bauzas | either by providing a flag | |
| 16:40:11 | bauzas | or by verifying the PCPU inventories | |
| 16:40:42 | bauzas | stephenfin: thoughts on that ? | |
| 16:40:58 | bauzas | others: too | |
| 16:43:52 | openstackgerrit | Lee Yarwood proposed openstack/nova-specs master: Re-propose stable device rescue for Train https://review.openstack.org/651151 | |
| 16:45:40 | stephenfin | bauzas: If that happens though, then everything has to be done at once. | |
| 16:46:03 | bauzas | stephenfin: that happens what ? | |
| 16:46:15 | bauzas | asking for PCPU ? | |
| 16:46:36 | stephenfin | if we wait until some point in time to start reporting PCPU | |
| 16:47:37 | stephenfin | so if we say the operator setting this configuration option is that point in time, then we must also ensure the operator also twists the knob that says "transform all my legacy extra specs/image meta to PCPU requests" | |
| 16:48:10 | efried | We said we weren't going to allow reshapes except on upgrade boundaries. When "we" said that, I was a dissenting vote. There are a number of specs we're discussing in the current cycle where that is going to need to be re-evaluated. | |
| 16:48:28 | stephenfin | (that knob has to exist because there is no a point in time where we can have no PCPU resources so therefore we can't always transform) | |
| 16:49:25 | efried | We must either allow reshapes "any time" (conceivably with a compute service restart), or stick to the above guns and require a host to be cleared out before configuration tweaks are done. The latter means we're not reshaping, just shuffling inventory around in a regular update_provider_tree code path, because we don't have allocations to dork with. | |
| 16:50:10 | bauzas | efried: we said earlier that the latter is highly terrible for ops | |
| 16:50:23 | bauzas | earlier being last week | |
| 16:50:39 | bauzas | efried: so that's why I'm considering a reshape at compute startup | |
| 16:50:49 | bauzas | based on config flags modification | |
| 16:50:58 | dansmith | efried: what you mean is make reshapes obligatory and not contingent on a config value or something right? | |
| 16:50:59 | efried | I'm in favor of that. | |
| 16:51:08 | bauzas | and the request knob be ops-driven, I like this | |
| 16:51:29 | dansmith | efried: because I think the two are not mutually exclusive, if the shape of the reshape would be defined by something in config (i.e. how many cpus are dedicated vs. shared) | |
| 16:51:43 | efried | dansmith: I mean allow reshapes to happen when they need to happen, rather than restricting them to upgrade boundaries. That's what bauzas is talking about as well. | |
| 16:52:05 | dansmith | efried: right I know, but I think there's subtlety here | |
| 16:52:05 | efried | Yes, dansmith and bauzas we still need to fail the reshape if it entails moving allocations in an impossible way. | |
| 16:52:19 | bauzas | efried: to be fair, VGPU reshapes are done on compute startup already, not upgrade: ) | |
| 16:52:40 | bauzas | of course, it will in theory run once, after upgrading | |
| 16:52:49 | dansmith | efried: we have to maintain config compatibility, but if if we don't have information in the N-1 config to do the reshape, then we have to be able to punt the reshape (triggered by an upgrade) until after the config is updated | |
| 16:52:53 | efried | like if you suddenly specify your PCPU pinset to be empty, but have instances running with dedicated CPUs, that's a fail. | |
| 16:53:05 | dansmith | and, I agree that if you have to change how many pcpus are dedicated, we have to reshape again | |
| 16:53:16 | efried | I think we're on the same page | |
| 16:53:20 | bauzas | efried: that's why I proposed to check the allocations and inventories *before* providing the reshape | |
| 16:53:51 | bauzas | if the operator changes the config, but placement says "sorry but you can't", then the compute will fail to restart | |
| 16:54:22 | dansmith | efried: I think the thing I don't want, which I expressed as "only at upgrade time" is something like we reshape every time we restart compute because we decide we can arrange things better, or some state in the db has changed, but reshape due to a config/structural change makes sense | |
| 16:54:29 | bauzas | if the operator changes the config, and placement resources are okay, then the driver returns a ReshapeNeeded | |
| 16:54:35 | efried | dansmith: wfm | |
| 16:54:39 | bauzas | and then the new inventories and allocations | |
| 16:54:57 | bauzas | okay, so dansmith, efried and I are on the same page | |
| 16:55:25 | bauzas | there is one last concern from stephenfin about the config knob for the PCPU request | |
| 16:55:44 | efried | So e.g. in the PCPU spec, we're inferring the counts and pinsets of VCPUs vs PCPUs based on existing conf options. | |
| 16:55:49 | bauzas | but I think it's okay to make the request transformation to be "config-driven" | |
| 16:56:15 | efried | So the operator needs to change to the new config in such a way that it *exactly* matches what we inferred, right? | |
| 16:56:27 | bauzas | so, the operator would basically tell when he's okay to count PCPUs (ie. probably after the whole nodes config change) | |
| 16:56:59 | efried | Otherwise we don't just need a reshape (move allocations) - we would also possibly need to re-pin guests to different physical processors and such. | |
| 16:57:06 | bauzas | efried: no, I'm saying that existing config will report VCPUs anyway | |
| 16:57:20 | bauzas | (including options that were asking for pinned cpu)s | |
| 16:57:41 | bauzas | efried: only new config option (explicitely cpu_dedicated_set) will trigger a reshape | |
| 16:57:55 | efried | yes, I get that bauzas, what I'm saying is, we're going to *infer* VCPU/PCPU counts and pinsets based on legacy conf options; but then the operator wants to cut over to using the new conf options. | |
| 16:58:24 | bauzas | no, I don't want us to infer VCPU and PCPU based on those options because they are errorprone | |
| 16:58:28 | bauzas | efried: ^ | |
| 16:58:38 | bauzas | those options being the legacy ones | |
| 16:58:49 | efried | oh, that's the basis for the PCPU spec as written at PS24 anyway. Haven't checked since then... | |
| 16:59:39 | bauzas | that's exactly why I'm saying "don't touch anything until operator explicitely says 'I want cpu_dedicated_set') | |
| 16:59:53 | bauzas | old world = VCPU | |
| 17:00:03 | bauzas | new world = VCPU and PCPU | |
| 17:00:30 | bauzas | for the request, trigger the request option when you consider having enough hosts to sustain PCPU requests | |
| 17:00:59 | bauzas | anyway, I need to bail out | |
| 17:01:24 | bauzas | kids aren't in town, and I promised some evening to my spouse | |
| 17:01:25 | efried | I think I see. Did you comment accordingly on the spec? | |
| 17:01:33 | bauzas | efried: I think so | |
| 17:01:36 | efried | okay. | |
| 17:03:01 | sean-k-mooney | efried: yes the cpu spec currently does infer but i agree with bauzas that we should not | |
| 17:04:02 | sean-k-mooney | efried: i think not supporting inference form the old config values would remove much of the upgrade impact or atleast help us too. | |
| 17:05:04 | efried | I'm good with that. Let's get it reflected in the spec | |
| 17:05:33 | bauzas | I just provided a comment trying to summarize my thoughts | |
| 17:05:45 | bauzas | this said, calling it a day | |
| 17:05:50 | sean-k-mooney | o/ | |
| 17:06:23 | efried | imacdonn: I agree with your assessment in https://bugs.launchpad.net/nova/+bug/1824435 | |
| 17:06:24 | openstack | Launchpad bug 1824435 in OpenStack Compute (nova) stein "fill_virtual_interface_list migration fails on second attempt" [High,Triaged] | |
| 17:06:32 | efried | (I wanted to say that, but not pollute the bug with it) | |
| 17:06:52 | mriedem | dansmith: you want to check my rolling upgrade validation logic that i dumped in gibi's spec https://review.openstack.org/#/c/652608/4/specs/train/approved/server-move-operations-with-ports-having-resource-request.rst@190 ? | |
| 17:10:22 | dansmith | mriedem: I probably have to read the whole spec to make sense of that huh? | |
| 17:11:04 | imacdonn | efried, which one? i.e. do you think we need to address (2) or would fixing (1) obviate that ? | |
| 17:11:37 | efried | imacdonn: fixing (1) would obviate. But that assumes we can do so. | |
| 17:12:05 | imacdonn | efried, right ... so now I'm trying to understand why the row is being created in the first place | |
| 17:12:05 | efried | imacdonn: IMO we should fix (1) and change (2) to raise an explicit exception to "guarantee" it. | |
| 17:12:09 | efried | ++ | |
| 17:12:48 | efried | imacdonn: Is it possible for the rows to differ in any material way? | |
| 17:13:01 | efried | (i.e. a way that makes a difference to the outcome) | |
| 17:13:05 | mriedem | dansmith: not really, it's just the usual "how could this fail during an upgrade" | |
| 17:13:18 | mriedem | dansmith: he needs to pass new parameters to compute rpc api methods, | |
| 17:13:30 | dansmith | well, I read it and seemed like I needed to understand more, so I'm reading the wholething now | |
| 17:13:33 | mriedem | which could be (1) stein computes that don't handle those or (2) rpc pinned so we pop those parameters | |
| 17:13:36 | mriedem | ok | |
| 17:13:47 | imacdonn | efried, I'm assuming that _security_group_get_by_names() is used elsewhere (or at least intended to be reusable), so probably should consider all possible use-cases, if we tackle that one | |
| 17:14:45 | efried | used in two places | |
| 17:15:19 | mriedem | imacdonn: efried: honestly i'm not sure how much relevance that code even has anymore if you're using neutron | |
| 17:15:36 | imacdonn | mriedem, I was wondering about that | |
| 17:16:31 | mriedem | i think it at least means if you're using neutron, every project that ever created an instance in nova has a 'default' security_groups table record that is never used or cleaned up | |
| 17:16:33 | mriedem | vestigial | |
| 17:17:12 | imacdonn | that seems plausible | |