| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-01 | |||
| 13:33:59 | Sundar | sean-k-mooney, efried: I agree it should be non-voting now. With the fake driver, we are putting up a VM without devices. | |
| 13:34:25 | sean-k-mooney | right its just testing the api workflow | |
| 13:34:33 | mriedem | we also have a barbican job in the experimental queue, but i'm not sure how stable that is either | |
| 13:34:34 | sean-k-mooney | even so that is valuable to test | |
| 13:35:12 | sean-k-mooney | with zuulv3 and the fact we can manage this all in repo its not hard to change if we think its stable and worth it | |
| 13:36:27 | Sundar | mriedem, sean-k-mooney, efried: What is the criterion to place a job in experimental, instead of check alone? | |
| 13:36:48 | efried | "We don't want this to run with every patch set" | |
| 13:36:54 | efried | "only on demand" | |
| 13:37:34 | efried | Considering how integrated the cyborg callouts are in the general flow of nova, IMO this one needs to be in check | |
| 13:38:34 | efried | ...once the code is in place. Which is what's signified by the job-add patch being on top of the series. | |
| 13:39:16 | sean-k-mooney | efried: ya i think it should be in check too as non voting | |
| 13:41:11 | Sundar | efried: Re. the criterion "We don't want this to run with every patch set" -- we may want to run the tests when anything changes in the compute manager or virt driver, at least. | |
| 13:41:26 | Sundar | Well, at least libvirt driver | |
| 13:41:38 | sean-k-mooney | Sundar: right erric was commenting about why it would be in experimental | |
| 13:41:59 | sean-k-mooney | if its in experimental one of the resaons for that is we dont want it to run on every patch | |
| 13:42:10 | sean-k-mooney | for cyborg we want it to run on most patches | |
| 13:42:23 | sean-k-mooney | so it better to keep it in check | |
| 13:42:36 | Sundar | sean-k-mooney: Sure, makes sense. How about gate? | |
| 13:42:39 | efried | Sundar: If you wanted to try to come up with a reasonable setting for 'irrelevant-files' you could do that. But for something coupled into the compute manager and virt driver and scheduler and conductor like this, I think that would be very difficult to nail down effectively. | |
| 13:42:54 | efried | Sundar: if it's nonvoting, it doesn't make sense for it to be in the gate queue. | |
| 13:42:56 | dansmith | definitely | |
| 13:43:01 | dansmith | maybe nova/doc :) | |
| 13:43:25 | Sundar | efried: So check only? | |
| 13:43:27 | sean-k-mooney | the default irrelevant-files we use for tempest jobs should be fine | |
| 13:43:46 | efried | Sundar: yes. Check, nonvoting. I commented as such in the patch. | |
| 13:43:54 | sean-k-mooney | Sundar: i think so we dont run most of the backend specific jobs in gate | |
| 13:44:18 | efried | which btw is here https://review.opendev.org/#/c/670999/ for those following along at home | |
| 13:44:57 | efried | Sundar: if we made it voting at some point, we would probably want to add it to gate for the same reasons that motivated us to make it voting (whatever those are). | |
| 13:45:58 | Sundar | efried: Good. Yumeng wasn't clear about that last night. | |
| 13:46:47 | Sundar | efried, sean-k-mooney: On the PTG front,I'll schedule a cross-project Cyborg/Nova. | |
| 13:47:12 | Sundar | for whatever topics are remaining -- specs or patch | |
| 13:47:16 | mriedem | some non-voting jobs in check are more restrictive on when they run, see the nova-lvm job | |
| 13:47:27 | mriedem | talking about a gating cyborg job in nova is jumping the gun by a mile | |
| 13:47:46 | Sundar | mriedem: Sure, was just asking | |
| 13:48:54 | mriedem | dansmith: the state of the gate sub-thread about slowness and timing out of the POST to resize a server reminded me of something you said in my cross-cell series at some point, the api currently calls conductor which calls the scheduler and it's all blocking until we pick a dest compute and cast to it, | |
| 13:49:02 | mriedem | you said something about adding long_rpc_timeout to that, | |
| 13:49:11 | sean-k-mooney | Sundar: i can try to follow some of the etherpad remotely but i wont be at the ptg | |
| 13:49:36 | dansmith | mriedem: oh for resize... | |
| 13:49:42 | mriedem | i don't think that would help the particular case found in the email (tempest would have timed out after 3 minutes i think) but in general it's maybe something we should do anyway since that could be a lengthy blocking resize call on a big deployment like cern | |
| 13:49:53 | dansmith | mriedem: yeah | |
| 13:50:06 | mriedem | i was surprised when i found that was a blocking call in the first place | |
| 13:50:13 | mriedem | since usually that's a no-no from the api | |
| 13:50:42 | dansmith | mriedem: when looking into the cells scheduler fail, I found one of those page_cleaner errors too, but it was only like 8s over the expected 1s, yours is 161s, which is a big(ger) deal | |
| 13:51:22 | mriedem | dansmith: so one question, and maybe the answer is "meh, doesn't matter", but would we want to only use the long_rpc_timeout in the call from the api but *not* the reschedule call from the compute where we won't hit the scheduler again and should be much faster? | |
| 13:51:30 | sean-k-mooney | mriedem: so a resize blocks all the way untill the scduler select a destitaiton ? y a i would have expected that to be asyc | |
| 13:51:32 | mriedem | or just keep it simple and use long_rpc_timeout for both | |
| 13:51:49 | mriedem | sean-k-mooney: more than that, until it selects a dest and conductor casts to it's prep_resize method | |
| 13:52:17 | dansmith | mriedem: well, long_rpc for the first call for sure, because that's the cascade.. on the second one, yeah I dunno.. could do both and if they're really running, then let 'em run | |
| 13:52:21 | mriedem | the reschedule still has to re-swap allocations i guess... | |
| 13:52:34 | mriedem | yeah seems simple to just do both | |
| 13:53:04 | dansmith | so, | |
| 13:53:11 | dansmith | crazy thought just entered my head | |
| 13:53:49 | dansmith | we *could* add configs for each service that lets you provide rpc method names that we do as long-call, | |
| 13:54:07 | dansmith | which would make it easier to experiment with that on a running system when there's an issue like this | |
| 13:54:42 | dansmith | we could hard-code the ones we know are important, but others could be converted by virtue of being mentioned | |
| 13:54:50 | dansmith | I know, it's a little crazy, but.. | |
| 13:55:39 | mriedem | seems a bit fickle, meaning typos would be easy to screw that up, or someone renaming a method, though i doubt that ever happens | |
| 13:55:43 | sean-k-mooney | how would we do that. add a decorator that inspected the config on invocation and convert them to a long rpc if found or something like that | |
| 13:55:54 | mriedem | you'd just check if the method name is in the configured list | |
| 13:56:08 | mriedem | if 'migrate_server' in CONF.conductor.long_rpc_timeout_calls | |
| 13:56:12 | mriedem | is one way | |
| 13:56:21 | dansmith | mriedem: yep all true | |
| 13:56:45 | sean-k-mooney | right i was just wonderign would htis be spread in the code or could we centralise it in one mentod that they all uses | |
| 13:57:09 | mriedem | i think that would be more useful before we had long_rpc_timeout where you otherwise just had to configure the global rpc_response_timeout for everything if you had one slow thing, like select_destinations | |
| 13:57:22 | mriedem | i'm sure vio deals with that | |
| 13:57:33 | mriedem | now having said that, | |
| 13:57:46 | mriedem | i think we know it's an issue for some object indirection api db queries on big lists | |
| 13:58:21 | mriedem | e.g. https://review.opendev.org/#/c/633042/ | |
| 13:58:48 | mriedem | but that would also be harder to configure properly i think | |
| 13:59:02 | dansmith | we should make the indirection methods use it always anyway probably | |
| 13:59:23 | mriedem | long_rpc_timeout? i thought about it when investigating that bug | |
| 13:59:30 | dansmith | yes | |
| 14:00:51 | sean-k-mooney | what would be the cost of makeing long_rpc the default out of interest. i assume there is a reason for being selectiv beyond the fact many rpc methods dont strictly need it | |
| 14:01:27 | dansmith | it increases rabbit load a little bit | |
| 14:02:02 | sean-k-mooney | ok im just wondering how much of a wack a mole problem this is | |
| 14:02:28 | mriedem | for object methods, could we distinguish between an object list query and just a single object query? | |
| 14:02:37 | dansmith | it's exactly a whack-a-mole problem, which is why making it config-driven so people can whack said moles without code changes might be reasonable :) | |
| 14:02:47 | dansmith | mriedem: yes | |
| 14:04:17 | dansmith | mriedem: if you're requesting so many objects that you can't get it done in under the default limit, there are probably other issues, and if it's a load thing, then small queries can be delayed behind big ones, so.. I dunno how meaningful that distinction is | |
| 14:05:34 | mriedem | yeah. at what point do we let someone configure themselves to wait too long rather than solve their bigger load problems by scaling out/up their control plane. | |
| 14:05:48 | mriedem | i guess that answer probably comes down to their SLAs | |
| 14:05:56 | mriedem | or whatever their performance testing thresholds are | |
| 14:06:14 | dansmith | yeah, I'm not really suggesting we let them configure long rpc calls externally to solve problems, | |
| 14:06:30 | dansmith | more to experiment and report (or even just for our own experimentation in devstack) | |
| 14:06:37 | dansmith | not that it's really easier for us I guess | |
| 14:06:41 | dansmith | anyway, it was just a thought | |
| 14:15:48 | Sundar | mriedem, sean-k-mooney, dansmith: What would you like to see, either in the PTG or in terms of spec/patches, to move Cyborg forward? | |
| 14:17:53 | dansmith | Sundar: I'm not going to be at the ptg | |
| 14:20:35 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Use long_rpc_timeout in conductor migrate_server RPC API call https://review.opendev.org/692550 | |
| 14:23:06 | Sundar | dansmith: I see. Would you like to see any changes in https://review.opendev.org/#/q/status:open+project:openstack/nova+bp/nova-cyborg-interaction ? Just checking to see what it takes to merge this patch series. | |
| 14:23:39 | dansmith | Sundar: reviews. doesn't look like it's had much of any, and the little it has had seems pretty un-diverse | |
| 14:23:51 | dansmith | which I know is why you're asking, just saying | |
| 14:24:29 | dansmith | I popped up one to see and it looks like it's in merge conflict and still has stuff like random whitespace damage in it (I'll comment in a sec), but that kind of stuff doesn't make it look like it's ready to go, fwiw | |
| 14:26:11 | Sundar | The conflicts happened recently. The series has passed Zuul checks for the most par and has been ready to go since Train merged. | |
| 14:26:50 | Sundar | Please ignore the conflicts for now and focus on the content | |
| 14:28:19 | mriedem | is there functional testing anywhere in the series? | |
| 14:28:23 | mriedem | i'm not even talking tempest, | |
| 14:28:32 | mriedem | but like creating a cyborg fixture in nova like we have for cinder and neutron, | |
| 14:28:52 | mriedem | so that we can start writing functional in-tree tests that use that fixture and at least exercise the runtime code in nova, despite using a fixture for the cyborg apis | |
| 14:29:14 | mriedem | i didn't ask/expect anyone to review my cross-cell resize series until i had some solid functional testing of at least the happy paths, | |