| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-12 | |||
| 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 | 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:06 | cdent | in train two a_c hits is pretty cheap | |
| 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 | 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:34:56 | stephenfin | oh, then I can't think there are huge issues outside of always making an extra requests to placement | |
| 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 | so | |
| 09:40:24 | efried | and don't need it. | |
| 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. | |
| 09:40:49 | bauzas | efried: not really, you opt-in to a sharded world once you use a conf opt | |
| 09:41:09 | bauzas | efried: I'm talking of the opt usage rolling update (sorry indeed) | |
| 09:41:19 | bauzas | ie. upgrade your cloud to Train | |
| 09:41:22 | bauzas | take your time | |
| 09:41:35 | bauzas | once all computes are done, you have two choices | |
| 09:41:51 | bauzas | A/ you have some puppetry that can change options on the fly | |
| 09:42:11 | bauzas | in this case, you don't really need to use some workaround that gives you performance hit | |
| 09:42:20 | efried | stephenfin: propose double query config on or off by default? | |
| 09:42:21 | bauzas | do the option update at some time | |
| 09:42:49 | bauzas | B/ you don't like CMSes or you prefer updating it smoothly for business reasons | |
| 09:43:01 | bauzas | then, you sign-off for a performance hit | |
| 09:43:13 | bauzas | but you'll be sure everything will continue to work | |
| 09:43:17 | bauzas | this sounds reasonable to me | |
| 09:43:30 | bauzas | that said, I trust cdent | |
| 09:43:30 | stephenfin | efried: the previous approach was a boolean - request PCPU or request VCPU - and it defaulted to requesting VCPU to not break upgrades | |
| 09:43:42 | stephenfin | I think the same thing applies here | |
| 09:44:05 | bauzas | if querying a_c isn't really a performance issue, it's just a matter of providing a temporary codepath | |
| 09:44:09 | efried | stephenfin: meaning we want to err on the side of getting hosts, even if it hurts, so default to double query? | |
| 09:44:18 | cdent | as I said, the performance concern is nova-side processing, which can be tested | |
| 09:44:27 | stephenfin | we need to default to double requests otherwise everyone will upgrade, not realize they need to set 'cpu_dedicated_set' on compute nodes, try to boot a pinned instance and fail miserably | |
| 09:44:38 | stephenfin | efried: yup | |
| 09:44:40 | efried | ack | |
| 09:45:02 | stephenfin | I'll work with TripleO at a minimum to make sure that workaround option is enabled on new deployments | |
| 09:45:14 | stephenfin | Can do the same from OSA and kolla too | |
| 09:45:32 | stephenfin | (though I'm not sure Kolla supports configuring hosts for pinned instances etc.) | |
| 09:46:06 | stephenfin | we've got three hours or so before he's up - should be loads of time to shove everything through | |
| 09:46:24 | efried | yeah, except the part where the gate delay is already past 3h | |
| 09:46:32 | stephenfin | :( | |
| 09:46:33 | efried | 3h13m | |
| 09:46:35 | efried | and counting | |
| 09:46:41 | stephenfin | I saw stuff in the queue for 9 hours last night | |
| 09:46:52 | stephenfin | though I may have been misreading things | |
| 09:47:03 | bauzas | stephenfin: wait a sec | |
| 09:47:24 | bauzas | the workaround necessarly has to be enabled *by default* if you don't wanna break users | |
| 09:47:45 | efried | bauzas: "enabled" meaning "double" yah? | |
| 09:47:52 | efried | I think that's what we're suggesting. | |
| 09:48:14 | efried | stephenfin said he's going to work with the deployment projects to make sure they disable it explicitly for *new* deployments | |
| 09:49:25 | stephenfin | bauzas: what efried said | |
| 09:50:20 | stephenfin | for anyone that knows TripleO, that's what we were planning to do with the previous config option too https://review.opendev.org/#/c/681207/ | |
| 09:50:31 | stephenfin | so it can be done | |
| 09:50:48 | efried | are we ready to unblock the bottom of cpu-resources and/or vpmem at this point? Get them queued up... | |
| 09:51:15 | efried | ...does this issue warrant continuing to hold cpu-resources until ready? | |
| 09:56:48 | stephenfin | I'm biased, but I _think_ we're good | |
| 09:57:04 | luyao | efried: for vpmems, I think I'm ready, but not sure stephenfin and sean-k-mooney, still need sean-k-mooney to confirm the xml again | |
| 09:57:24 | stephenfin | The quota issue and the switch over to the try-try again logic were the blockers. The former's resolved and the latter just needs a tweak | |
| 09:57:58 | stephenfin | I've promised luyao I'll sign off on vPMEM today too, so that's pretty much good to go too | |
| 09:58:40 | efried | Okay. I'll unblock cpu-resources and start +Aing. Plenty of time to yank them out of the gate if something goes pear-shaped. | |
| 09:58:41 | efried | And I'll unblock vpmem, ready for your +A stephenfin | |
| 09:59:09 | stephenfin | Cool. Lemme finish reworking this patch and I'll hit that | |
| 10:01:37 | stephenfin | efried, alex_xu, bauzas: bikeshed time - what should the workaround option be called? | |
| 10:02:09 | stephenfin | it's disabling the second request for VCPUs if the instance is pinned | |
| 10:02:25 | stephenfin | disable_second_request_for_pinned_instances sux | |
| 10:02:31 | efried | [workarounds]try_really_hard_to_get_allocation_candidates_for_pinned_instances_but_suffer_a_performance_hit | |
| 10:02:44 | bauzas | sorry folks, was AFK | |
| 10:02:46 | efried | oh, right, reverse of that | |