| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-12 | |||
| 16:03:45 | dansmith | Roamer`: yep, or that | |
| 16:03:47 | Roamer` | (with the correct variable names, of course, pfheh) | |
| 16:05:00 | dansmith | mine makes anything with an updated_at win and short circuit before comparing the updated_ats which means you won't ever compare non-nones | |
| 16:05:09 | dansmith | er, Non-None to None | |
| 16:06:11 | lyarwood | mdbooth: os-brick had a number of bugs around multiple devices getting grouped under the same mpath device blocking the final flush/removal on disconnect | |
| 16:06:21 | Roamer` | dansmith, TBH, I hadn't seen yours before writing mine :) it took me a while to test it and copy it here, yours might be better and less unwieldy :) | |
| 16:06:33 | lyarwood | mdbooth: QEMU's own flush was enough to ensure everything made it to the backend before we forced anything in that case | |
| 16:07:11 | mdbooth | lyarwood: That flush was happening on the source node, after switchover? | |
| 16:07:26 | lyarwood | mdbooth: the QEMU flush? | |
| 16:07:40 | lyarwood | mdbooth: no that's before we converge iirc | |
| 16:07:44 | lyarwood | or maybe during | |
| 16:07:47 | mdbooth | lyarwood: os-brick's flush on disconnect | |
| 16:08:11 | mdbooth | So the bug was that os-brick thought it had data to flush, but it didn't really? | |
| 16:08:31 | lyarwood | mdbooth: that's after but it's just something you have to do before disconnecting mpath devices iirc, there's no data actually cached anywhere at that point | |
| 16:08:56 | mdbooth | Right. It would just be really bad if there really was data to flush at that point. | |
| 16:09:48 | lyarwood | yup | |
| 16:10:01 | dansmith | jroll: didn't you add some ironic grouping construct so that one ironic can serve independent pools of nodes to dedicated nova computes? | |
| 16:10:28 | dansmith | ah, conductor groups | |
| 16:15:11 | belmoreira | dansmith jroll how that would work? | |
| 16:15:23 | dansmith | belmoreira: did you read the spec I just sent you? :) | |
| 16:15:49 | belmoreira | :) maybe... which one? :) | |
| 16:15:58 | dansmith | belmoreira: https://docs.openstack.org/ironic/latest/admin/conductor-groups.html | |
| 16:16:09 | dansmith | oops, | |
| 16:16:16 | dansmith | belmoreira: https://specs.openstack.org/openstack/nova-specs/specs/stein/implemented/ironic-conductor-groups.html | |
| 16:16:52 | belmoreira | this is train | |
| 16:17:07 | dansmith | what is? | |
| 16:17:48 | belmoreira | sorry. This is available in Stein | |
| 16:17:52 | dansmith | right | |
| 16:21:18 | belmoreira | I forgot about it. sorry. This is great however means that we will need to scale nova-compute in the same way as the ironic-conductor | |
| 16:21:56 | dansmith | belmoreira: well, you probably should be doing that anyway given you're hitting nova-compute scale issues when you make ironic big :) | |
| 16:22:51 | dansmith | I understand the desire to make the RT loop in nova-compute more efficient, and I'm sure we can without breaking things, but it's a change to complex crufty code that only (positively) impacts the one oddball driver that uses multiple nodes, | |
| 16:23:28 | dansmith | so I'm less enthusiastic about that, especially when grouping nova-computes gets you a lot more gain too, | |
| 16:23:29 | dansmith | like better fault tolerance, mapping your existing failure domains through to ironic, etc | |
| 16:24:04 | belmoreira | dansmith I understand that. That's why the initial plan was the sharding. And I wasn't aware (forgot) about the conductor groups | |
| 16:24:12 | belmoreira | this can make it | |
| 16:24:33 | dansmith | cool | |
| 16:31:01 | belmoreira | Thanks dansmith | |
| 16:31:33 | belmoreira | I will try this and let you know how it goes | |
| 16:32:23 | jroll | \o/ | |
| 16:33:57 | kashyap | mdbooth: On that "no_performance_impact" thing for post-copy / auto-converge spec, I have to bike-shed: the name is too generic for my taste. I suggested being more clear WTF it is doing: "no_post_copy_and_auto_converge". | |
| 16:34:23 | dansmith | belmoreira: sweet that'd be super | |
| 16:38:07 | openstackgerrit | Lee Yarwood proposed openstack/nova-specs master: WIP Virtual instance rescue with stable disk devices https://review.opendev.org/693849 | |
| 16:48:43 | dansmith | mriedem: on lyarwood's spec to do stable disk rescue, | |
| 16:48:56 | dansmith | do you have strong feelings about bundling the can-now-rescue-BFV feature in it, | |
| 16:49:03 | dansmith | vs. a relatively tiny follow-on effort? | |
| 16:56:01 | openstackgerrit | sean mooney proposed openstack/nova master: block rebuild when numa topology changed https://review.opendev.org/687957 | |
| 16:56:02 | openstackgerrit | sean mooney proposed openstack/nova master: Disable NUMATopologyFilter on rebuild https://review.opendev.org/689861 | |
| 17:02:01 | mriedem | dansmith: i haven't read it but what's the difference? you mean making the api capability separate which is what, a microversion and a min compute service version compat check? | |
| 17:02:22 | dansmith | mriedem: not even a min version check I think, a compute capability check | |
| 17:02:47 | mriedem | so the api checking the resource provider traits? | |
| 17:03:13 | dansmith | er, maybe I'm confused, I thought the api could check the compute capabilities, but maybe that's stupid | |
| 17:03:25 | dansmith | so yeah maybe a version check and a cap flag in the compute manager | |
| 17:03:52 | dansmith | mriedem: point being, lyarwood had it in his now-abandoned first rev to remove that api check, and his current spec says no API impact, so wondering which way you want to go | |
| 17:03:52 | mriedem | i've brought up that idea before about the api checking compute capabilities via traits, e.g. https://review.opendev.org/#/c/666604/ | |
| 17:04:17 | mriedem | well there is obviously an api impact | |
| 17:04:39 | dansmith | mriedem: ...only if he removes that check in his series again | |
| 17:04:59 | dansmith | mriedem: and, you think that needs a microversion? just removing a check in some cases? | |
| 17:05:46 | mriedem | it's a behavior change that a user opts into, i.e. i can rescue volume-backed servers on cloud A but not cloud B, | |
| 17:05:49 | dansmith | if so, I understand why, but I don't have a good gut feeling for when we do and don't need to signal removing checks | |
| 17:05:50 | mriedem | which is usually a microversion | |
| 17:06:02 | dansmith | they can opt in but still refuse with the same error | |
| 17:06:08 | dansmith | it won't work for all hypervisors, | |
| 17:06:18 | dansmith | and unless the api documents that you can't do that with volume-backed instances, | |
| 17:06:19 | mriedem | same idea for multiattach volume support, same for volume-backed rebuild (if someone finished that up) | |
| 17:06:22 | dansmith | it seems like an obscure thing to me | |
| 17:06:37 | mriedem | i think we've said in the past (and in our docs) that the docs are not a contract, the api behavior is | |
| 17:06:55 | mriedem | and yes i know we fudge these lines all the time, e.g. gibi's changes to support moving servers with qos ports | |
| 17:07:13 | dansmith | absolutely, but I'm not sure this is discoverable as the reason, which is why it doesn't seem clear-cut to me | |
| 17:07:16 | mriedem | and yes i realize not all hypervisors are going to support this so just because a cloud has the microversion that allows the request it doesn't mean it's going to work | |
| 17:08:02 | dansmith | if you view the current error as "You can't rescue that instance for reasons I will not divulge" then all users right now just try it and accept the fate if they can't, without knowing why" | |
| 17:08:09 | mriedem | as i said in lee's original change about microversions, i said i don't know what others would think so it's likely a ML discussion | |
| 17:08:12 | dansmith | in which case allowing it on more things seems fine | |
| 17:08:28 | mriedem | it's not a 409 though right? it's a 400 | |
| 17:08:53 | mriedem | yes it's a 400 today | |
| 17:08:55 | dansmith | the distinction doesn't mean anything to me | |
| 17:09:07 | dansmith | 409 is transient or something? | |
| 17:09:09 | mriedem | if it were a 409 that'd be a bit fudgier to saying "you can' do this right now" | |
| 17:09:20 | dansmith | okay, well, that's legit | |
| 17:09:31 | dansmith | probably *should* have been a 409 then, but alas | |
| 17:09:54 | mriedem | i don't think there was a lot of thought put into response code nuance back in the day | |
| 17:10:03 | dansmith | no, clearly not | |
| 17:10:53 | dansmith | does 400 mean it's a fatal error specifically or just not imply retry-ability one way or the other? I would assume the latter as a catch-all | |
| 17:11:10 | openstackgerrit | sean mooney proposed openstack/nova-specs master: Add spec for VM-scoped SR-IOV NUMA affinity https://review.opendev.org/683174 | |
| 17:11:35 | dansmith | anyway, if it's a microversion, then it probably makes sense to have that be a separate effort, maybe even before this stable rescue series to open up the possibility, even with no implementors, followed by his patch that actually allows it | |
| 17:11:59 | sean-k-mooney | efried: alex_xu ^ fixed the typos but that is otherwise the same | |
| 17:12:44 | mriedem | 400 means don't try this again | |
| 17:14:02 | dansmith | "The client SHOULD NOT repeat the request without modifications." | |
| 17:14:03 | sean-k-mooney | or at least dont try this again until the resouce is in a different state | |
| 17:14:24 | dansmith | sean-k-mooney: that's not what the w3c says for 400, but does for others, so not sure you're right about that | |
| 17:14:41 | mriedem | sean-k-mooney: you just described 409 | |
| 17:14:44 | sean-k-mooney | 400 is bad request | |
| 17:14:48 | dansmith | mriedem: that's an unfortunate implication for our default error code, but fair enough | |
| 17:15:11 | sean-k-mooney | but it may be a bad request for the give state fo the resouce but not in general | |
| 17:15:22 | dansmith | sean-k-mooney: no | |
| 17:15:32 | sean-k-mooney | 409 conflcit is ment for concurrent modifcation | |
| 17:15:34 | dansmith | sean-k-mooney: it says the reason is "malformed syntax" and "do NOT try this again" | |
| 17:15:43 | dansmith | https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html | |
| 17:15:59 | dansmith | the reason is not "the thing is in the wrong state for the operation you tried" | |
| 17:16:08 | sean-k-mooney | ya fair | |
| 17:16:12 | sean-k-mooney | i normally use https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/400 | |