| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-01 | |||
| 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, | |
| 14:29:28 | mriedem | i don't think i even started asking for reviews until i had a multi-cell gate job working | |
| 14:29:58 | mriedem | right now this all looks like unit tests | |
| 14:30:05 | dansmith | mriedem: please ignore the conflicts and lack of testing and focus on the content | |
| 14:30:24 | mriedem | the next closest thing to the cyborg stuff is probably gibi's qos ports series | |
| 14:30:36 | mriedem | and he did a lot of functional test for those b/c of the complicated nature of dealing with nested provider allocations | |
| 14:31:01 | mriedem | so maybe you could build on some of that while also working on a beginning of a cyborg fixture for functional testing - at least the basic happy path, | |
| 14:31:27 | mriedem | create a server with arqs and then delete the server and verify the arqs are deleted/unbound whatever happens to those | |
| 14:31:42 | mriedem | the fixture would obviously have to track the state of the arqs associated with a server | |
| 14:31:53 | mriedem | which we do in the NeutronFixture for ports and volumes in the CinderFixture | |
| 14:32:18 | Sundar | mriedem: Looking at the discussion we had at the Cyborg/Nova cross-project at Denver PTG: https://etherpad.openstack.org/p/ptg-train-xproj-nova-cyborg | |
| 14:33:17 | Sundar | The criterion for integration was said to be upstream tempest CI with a fake driver. We got the fake driver done, and tempest can create/delete VMs wtth that fake driver now | |
| 14:34:20 | dansmith | Sundar: yeah, that's necessary too, | |
| 14:34:47 | dansmith | Sundar: but just about anything with tentacles running from API to the deepest depths of the compute service has serious functional testing | |
| 14:35:47 | Sundar | mriedem, dansmith: Just to be clear, you are talking about extending today's Nova functional testing, right? Not gabbi? | |
| 14:35:56 | mriedem | nova doesn't use gabbi | |
| 14:35:57 | mriedem | placement does | |
| 14:36:50 | mriedem | Sundar: some solid examples are what gibi did with qos port functional testing, for which this is the parent class https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L5565 | |
| 14:37:13 | mriedem | so here is an example https://github.com/openstack/nova/blob/master/nova/tests/functional/test_servers.py#L5996 | |
| 14:37:22 | mriedem | "Tests creating a server with a pre-existing port that has a resource request for a QoS minimum bandwidth policy." | |
| 14:37:54 | mriedem | we have other tests that interact with cinder https://github.com/openstack/nova/blob/master/nova/tests/functional/test_boot_from_volume.py | |
| 14:38:03 | mriedem | well, usinga CinderFixture | |
| 14:38:26 | mriedem | https://github.com/openstack/nova/blob/master/nova/tests/functional/test_multiattach.py | |
| 14:40:59 | Sundar | mriedem: Thanks, will look at it and follow up. | |
| 14:44:07 | mriedem | i traced the instance create/delete through the conductor and compute logs for the test_server_basic_ops tempest test and it looked fine, i saw the creating/waiting/deleting arqs stuff, no weird errors in the logs, so that's all good to see | |