Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-11-01
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
14:44:40 mriedem once the series is rebased, if tests are passing and it's ready for review, put it into a runway slot
14:45:55 mriedem Sundar: ^
14:47:20 Sundar mriedem: Thanks, will do.
14:48:02 mriedem Sundar: this is going to require a microversion bump https://review.opendev.org/#/c/631245/36/nova/api/openstack/compute/schemas/server_external_events.py@36
14:48:09 mriedem you can't change schema on a versioned api without a new microversion
14:49:37 dansmith this does not look "ready to go since train" to me
14:49:49 dansmith there are things used in early patches that aren't added until later patches
14:50:12 dansmith and presumably very large test (even unit) gaps to allow that to happen
14:53:14 Sundar dansmith: "used in early patches that aren't added until later patches" -- What are you referring to?
14:53:30 dansmith Sundar: the event change. I left comments
14:54:09 mriedem Sundar: https://review.opendev.org/#/c/631244/43/nova/compute/manager.py@2609
14:54:43 mriedem Sundar: note that nova still generally tries to subscribe to a philosophy of anything we merge today can be in production today, and people can CD nova
14:55:21 mriedem even if that's not the reality of how the vast majority of openstack consumers consume openstack, it's a development philosophy to try and avoid merging broken code
14:57:53 mriedem having said that i can't easily find anything about that in https://docs.openstack.org/nova/latest/contributor/ but i know it comes up every so often, and not all projects in openstack subscribe to the same philosophy, e.g. if it's all working by RC1 whatever, let it ride
14:59:36 dansmith probably because it's core openstack philosophy than not everyone has adhered to
14:59:40 mriedem closest is, https://docs.openstack.org/nova/latest/contributor/process.html#smooth-upgrades "upgrade from any commit, to any future commit, within the same major release"
14:59:42 dansmith is, or was
14:59:55 mriedem dansmith: i'm just not sure it's clearly documented anywhere
14:59:58 mriedem and probably should be
15:00:20 mriedem like something simple in https://docs.openstack.org/nova/latest/contributor/policies.html
15:01:00 dansmith sounds like a good thing to do tome
15:01:18 Sundar I have generally thought of the patch series as a unified whole. Merging individual parts will not give you any useful functionality. The base patch has a procedural -2, so that the whole series will merge together.
15:01:40 dansmith it's fine to not have incremental functionality,
15:01:46 Sundar That said, I don't believe in merging 'broken patches' either
15:01:52 dansmith it's not fine to merge things that use something before the thing is available
15:02:12 dansmith and we can't arrange for the whole series to merge as a whole,
15:02:34 dansmith which means the -2 holds things until all the pieces are ready, but they still have to be mergeable in isolation, because they will
15:02:54 mriedem especially since it takes about 3 days of rechecks to merge anything in nova right now
15:03:08 dansmith three days? please
15:03:19 mriedem ok ok 5 days
15:03:20 dansmith my notifications thing has been going all week.. maybe since last week
15:03:49 mriedem for big series like this it's always best to work from the bottom up where everything is noop until "turned on" in the API at the end if possible
15:03:52 dansmith yeah, since monday
15:04:01 mriedem so there is less risk in merging the bottom noop stuff
15:07:45 Sundar mriedem: The code flow starts from the API, such as getting device profile info based on the extra specs. If the API stuff came later, wouldn't the patch sequence be out of order? We can only do UT for the earlier patches then.
15:08:02 cdent mriedem: I assume you saw my response on the gate+placement email? Sorry I've not been able to dig more deeply. I've been engaged elsewhere...To such an extent that I may come off the mailing list pretty soon to maintain sanity.
15:09:01 efried Sundar: in this case the "master switch" could conceivably be the part that processes the device profile out of the flavor.
15:09:28 mriedem cdent: yup, and replied, and thanks
15:09:45 mriedem cdent: i think the likeliest culprit is noise neighbors on oversubscribed nodes as you said
15:17:28 cdent mriedem: the gist of what I can find on that innodb error (now that I've managed to actually read your responses) is "you disk i/o is way constrained"
15:19:32 mriedem cdent: heh, yeah looking at this dstat graph, i/o is just 0 from 16:52 to 16:55
15:19:42 cdent oh dear
15:20:32 cdent there should be some kind of "how am I supposed to work in these conditions" meme we can interject here
15:21:08 efried https://makeameme.org/meme/i-cant-work-2sl5sn
15:21:11 mriedem https://toddroblog.files.wordpress.com/2017/02/drunk-hobo-flips-double-bird-1.jpg?w=809 ?
15:21:22 mriedem oh blurred out?!
15:23:09 mriedem i wish i knew about https://lamada.eu/dstat-graph/# years ago
15:26:44 cdent yeah, why don't things like that get passed around more easily?
15:27:16 mriedem probably because i was too shy to say "i'm dumb, how can i see this dstat output in a graph?"

Earlier   Later