Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-12
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
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

Earlier   Later