| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-12 | |||
| 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 | |
| 17:16:29 | sean-k-mooney | but yes it also highlights "The client should not repeat this request without modification." | |
| 17:16:33 | dansmith | none of those reasons imply retry | |
| 17:17:55 | sean-k-mooney | i think the only time you woudl retry is for 5xx errors | |
| 17:18:04 | dansmith | mriedem: so, spec because api change, and put the api change ahead of lyarwood's patch so we can actually test through it and hold merge until both are ready yeah? | |
| 17:18:31 | dansmith | sean-k-mooney: no, some of the 4xx errors are mentioned as retryable with no modification | |
| 17:18:36 | dansmith | sean-k-mooney: like 408 | |
| 17:18:57 | sean-k-mooney | oh ok i normaly think of 503 https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/503 when i think retry | |
| 17:19:09 | mriedem | dansmith: if you're saying single spec, multiple changes (one for compute, one for api), then yes agree | |
| 17:19:11 | sean-k-mooney | oh ya request timeouts | |
| 17:19:40 | dansmith | mriedem: no, I mean another spec for the api change specifically just to avoid mucking up lyarwood's clean understandable spec with the spec to change the api behavior | |
| 17:20:10 | efried | oh sean-k-mooney, you lost your +1s from Vieri and Andriy :( | |
| 17:20:44 | sean-k-mooney | :( your +2 would equal both there +1s right :) | |
| 17:21:02 | dansmith | mriedem: I don't want to do extra work, but it seems like having lyarwood's stable rescue spec be readable in isolation without a bunch of api semantic changes would be beneficial, but maybe the api docs required for just a status change wouldn't be too big | |
| 17:21:13 | efried | sean-k-mooney: re+2, thanks for the update. | |
| 17:21:35 | sean-k-mooney | i still need to fix the typos you noted in the code but thats next on my list | |
| 17:22:06 | sean-k-mooney | then i need to adress dansmith's feedback on another series :) | |
| 17:23:26 | mriedem | dansmith: are you saying the api change would just be changing the status code from 400 to 409 rather than a microversion? | |
| 17:23:37 | dansmith | mriedem: no | |
| 17:24:25 | mriedem | then i don't think a secondary api-only spec is going to be that big of a deal | |
| 17:24:43 | dansmith | cripes | |
| 17:24:45 | mriedem | "depends on blueprint x. in x we make y work on the compute. this spec enables it in the api with a new microversion" | |
| 17:25:08 | mriedem | just do whatever y'all think is best | |
| 17:25:13 | mriedem | i haven't read the current spec | |
| 17:25:24 | dansmith | mriedem: I'm trying to say I think we should separate the api change into its own small spec to keep lyarwood's focused on the disk changes | |
| 17:25:42 | mriedem | and i just said i was ok with that right? | |
| 17:25:53 | dansmith | mriedem: in isolation without his change, we can make the api change to remove the hard block and say "as of microversion X you may or may not be able to rescue BFVs" | |
| 17:26:13 | dansmith | mriedem: it wasn't clear to me | |
| 17:26:34 | dansmith | because that's what I've been suggesting for ten minutes and you seemed to be disagreeing, so I was trying to explain | |
| 17:26:39 | mriedem | so without any compute side changes, we should be able to rescue volume-backed servers today? | |
| 17:27:17 | mriedem | i'm fine with whatever | |
| 17:27:27 | mriedem | let's just stop talking about this | |
| 17:27:29 | dansmith | I'm clearly the problem here, I'll just butt out | |
| 17:34:55 | artom | donnyd, heya, what the difference between your multi-numa and multi-numa-expanded flavors? Or where can I look to find out? | |
| 17:35:14 | artom | (Also, we should talk about enabling hugepages at some point, but that can wait) | |
| 17:35:22 | sean-k-mooney | artom: the expanded one has 16G of ram instead of 8 | |
| 17:35:31 | artom | sean-k-mooney, that's it? | |
| 17:35:37 | sean-k-mooney | yep | |
| 17:35:51 | sean-k-mooney | otherwise they both have the same about of disk and cpu | |
| 17:35:58 | artom | And NUMA nodes | |
| 17:36:02 | sean-k-mooney | but have hw:numa_nodes=2 | |
| 17:36:25 | sean-k-mooney | and hw:cpu_socket=2 and hw:cpu_thread=2 | |
| 17:36:43 | artom | So the non-expanded one is fine | |
| 17:37:07 | sean-k-mooney | we shoudl try to use the non expanded ones if we can to save space but we might need the expanded one on the contoler | |
| 17:37:57 | artom | Ohh | |
| 17:37:58 | sean-k-mooney | ya i didnt want to have to super optimise the job to get it to fit intially which is one of the reason we started with the expanded lables but it should not be needed longterm | |
| 17:38:37 | artom | Let's try with non-expanded I guess first | |
| 17:38:43 | artom | And if we hit oom or something we can -expand :D | |
| 17:39:06 | sean-k-mooney | yep i think i was trying to allocte a low amout of hugepage in my job | |
| 17:39:15 | sean-k-mooney | well in the orginal version | |
| 17:40:00 | sean-k-mooney | i think we are generaly around the 5-6 G mark on used memroy on the contoler so you dont want to exceen more then about 1G of hugepages total | |
| 17:40:37 | artom | Ohh, you actually set hugepages in Ansible | |
| 17:40:51 | sean-k-mooney | yes here https://review.opendev.org/#/c/679656/13/playbooks/nfv/nfv.yaml | |
| 17:40:56 | artom | So I should probably do that instead of https://review.opendev.org/#/c/693690/ | |
| 17:41:01 | sean-k-mooney | but its too much currently for the contoler | |
| 17:41:07 | artom | Although that skip could still be useful in other envs | |
| 17:42:17 | sean-k-mooney | ya i would basically do this | |
| 17:42:19 | sean-k-mooney | hosts: controller | |
| 17:42:21 | sean-k-mooney | tasks: | |
| 17:42:23 | sean-k-mooney | - name: allocate hugepages | |