Posted	Nick	Remark
#openstack-nova - 2018-03-26
16:47:16	efried	...which is kind of appropriate, because "identifying a NUMA node" is a virt-specific thing.
16:47:26	efried	I.e. libvirt is gonna do it a different way than hyperv or whatever.
16:48:12	bauzas	the problem is, say you ask for 2GB of memory within a NUMA node, then placement gives you host A with 2 NUMA nodes but only one NUMA node for host B
16:48:24	bauzas	because the other NUMA node of host B is full
16:48:45	bauzas	then, we need to pass to the scheduler filters the root RP
16:49:10	bauzas	in theory, when it goes on NUMA filter for host B, it could consider the second NUMA node for host B as legit
16:49:17	bauzas	there be dragons
16:50:10	efried	how could it?
16:50:27	efried	There's no candidate with allocations in that second NUMA node on host B.
16:50:36	sean-k-mooney	bauzas: there might be dragons but the host state object for host b should also know that the second numa node is fully used and ignore it
16:51:14	sean-k-mooney	bauzas: what is an issue if both numa nodes are valid and the filter chooses the other one form placement
16:51:49	sean-k-mooney	e.g. placement decremetes the inventor that corresponds to node 0 but the numa topology filter decrements node 1
16:51:58	bauzas	okay, then I'm maybe overthinking
16:52:38	sean-k-mooney	bauzas: there is an edge case here but its for host A with 2 NUMA not host B with 1
16:53:52	sean-k-mooney	for host b the resouce tracker will have updted the numatoplogy blob to also show the second numa nodes as full but in the case of host A both are valid from its point of view
16:55:02	bauzas	I guess my fears are coming from the fact we litterally try to draw something out of nowhere, and without good testing for making sure we don't trample folks
16:55:14	openstackgerrit	Eric Berglund proposed openstack/nova master: PowerVM Driver: DiskAdapter parent class https://review.openstack.org/549053
16:55:28	bauzas	if I was able to just test what I write, I wouldn't be trying to consider all the edge cases
16:55:55	openstackgerrit	Eric Berglund proposed openstack/nova master: WIP: PowerVM Driver: Localdisk https://review.openstack.org/549300
16:56:05	bauzas	and I just feel I'm just trying to sink all the ocean's water
16:57:36	sean-k-mooney	bauzas: i think you have raised a valid issue here. the virt driver will likely need to tag the resouce providres with a trait or aggregat to allow it to map its internal view(in the resouce tracker/numa toploygy bob) to the view it gets back in the allocation canditates
16:58:25	bauzas	sean-k-mooney: how do you see that ?
16:59:40	sean-k-mooney	how would you do it? when the virt driver create the RP for the numanode in the provider tree update it would include a CUSTOM_HOST_NUMA_ID_X trait where X is its internal identify for the numa node e.g. 0 or 1
17:00:11	sean-k-mooney	then in the allocation canditates resoponce the numa topology filter can use that trait to map the the correct cell in the numa topology blob
17:00:43	sean-k-mooney	you could also use an agregate but that would be harder to map to the topoploy blob unless we add teh aggregate uuid to the blob
17:01:32	bauzas	sean-k-mooney: ouch.
17:01:37	sean-k-mooney	that trait/aggreage would be uses soly by the virtdriver/filter and never passed in any request to placement
17:02:12	bauzas	sean-k-mooney: the problem is that the virt.hardware module is a pleasure to modify
17:02:39	bauzas	I'd really want to avoid any subsequent modification
17:03:26	sean-k-mooney	bauzas: yes well if we use a trait then we dont need to modify it
17:03:43	sean-k-mooney	we just need to modify the update provider tree stuff to also include the cell id
17:03:57	sean-k-mooney	as a trait on the numa node
17:04:23	bauzas	not sure I'm getting you
17:04:48	sean-k-mooney	i have not looked but im assumeing the numa patches where going to use the info from the numa topology blob to create teh RPs for the NUMA node and the sub resouces of that node
17:04:56	bauzas	because the virt.hardware module gets a topology from both the hoststate and the instance proposed topolgy
17:05:33	bauzas	here, we would need to hack the module to look at the resource providers, right ?
17:05:42	bauzas	instead of the host state
17:06:22	bauzas	anyway, I'm running out of fuel for my brain
17:06:49	sean-k-mooney	bauzas: i think we would need to pass in the allocation candiates to the filter yes and then pass that down into the fit_instance_to_host fucntion or whatever it is called so that it could make a descission based on the allcoation candiate
17:07:10	bauzas	right, that's what I meant
17:07:41	sean-k-mooney	ya that fuction is a pain to modify or debug but its going to need to be scoped to the allocation candiate to work correct when numa is in placement
17:09:54	jaypipes	bauzas, stephenfin, sean-k-mooney, efried: sorry, done with call now.
17:10:13	bauzas	jaypipes: I'm just rat-holing
17:11:44	bauzas	my concern is, how to make sure we can still have all the NUMA features be workable in a world with nested RPs albeit all things solved in the future++ with placement resources
17:12:00	sean-k-mooney	jaypipes: the issue is basically how to correlate placement RPs with the compute node resouce tracker so that when the the numa topology filter or pining code runs we only look at the resouce selected by placement in the allocation candidate and not all numa nodes for example.
17:20:22	jaypipes	efried: iota?
17:33:24	jaypipes	sean-k-mooney: I don't think it's really a big issue. Basically, let the NUMA topology filter just run as-is. It will "pick" a NUMA node to pin the instance to (and then promptly forget about its pick). The scheduler will claim resources against one of the NUMA nodes on the host (via the normal allocation request claim_resources() process). The build request gets to the compute host. During the instance_claim() process, the numa_fit_instance_to_
17:33:24	jaypipes	host() is run again. If that picks a different NUMA node than what is in the allocation_request that is sent along with the build request, then we raise an exception and just retry the scheduling.
17:35:11	dansmith	efried: so, questions about L371 here: https://review.openstack.org/#/c/547990/10/nova/scheduler/client/report.py
17:35:16	dansmith	efried: what 406 are you talking about?
17:35:38	dansmith	the only one I know of is if the version isn't supported that we need for member_of
17:36:10	dansmith	efried: and, I'm only running the intersection and setting of the member_of if aggregates is non-empty, which is what you're saying I'll need to do
17:36:15	sean-k-mooney	jaypipes: thats one option be se should really not have to retry here
17:36:32	sean-k-mooney	jaypipes: we should be able to just look at the cell that was selected by placement
17:38:00	sean-k-mooney	also when numa_fit_instance_to_host runs how to you tell if it picked a different node to the one in the allocation_request if you cant correlate between them
17:39:44	efried	dansmith: The functionality you want is to be able to do the set logic on aggregates. We plan to allow placement to do that via some new syntax (your pending spec delta). Once that happens, there'll be a microversion for that. And at that time, if you try to use that new microversion, you'll also have to handle the case where placement is downlevel, just like we do everywhere else where we might be straddling versions.
17:40:25	edleafe	efried: is there a spec/bp/bug for adding the consumer generation?
17:40:40	efried	dansmith: And what I'm saying is that the 406 branch will have to do the placement query without taking member_of into account (or using the intersection thing as a prefilter) and then do the set logic on the candidates that come back for that too-broad query.
17:40:53	dansmith	efried: why? the last couple times we've bumped that version we have't supported an older placement
17:41:04	efried	Whoah. Yes, we do, every time.
17:41:11	dansmith	efried: we've been saying placement goes first in the upgrade stack
17:41:18	efried	edleafe: Not as of the last time I checked, which I think was yesterday.
17:41:35	dansmith	efried: where? that code has changed several times while I was working on this and we don't have any fallback code there
17:42:05	efried	dansmith: I think we say that as a best practice or something, but we've got somewhere else that defines the minimum placement microversion for a given release and it's always lower than the maximum for same release. mriedem help me out here.
17:42:11	jaypipes	dansmith: example? the code has changed in a backwards *incompatible* way and we don't have fallback code?
17:42:16	efried	dansmith: Just look for 406 in report.py
17:42:37	efried	cdent culled the stale ones at the end of Queens, so there aren't many left, but still some.
17:42:51	dansmith	jaypipes: the last couple of times this exact method has changed
17:43:07	sean-k-mooney	efried: the grenagde job in the gate is proably upgradeing placement first so even if it could work its proably not tested today
17:43:13	dansmith	I can go actually dig up reviews if you care
17:43:30	edleafe	efried: yeah, I didn't see anything either. So would you call this a bug fix or a new feature?
17:43:34	dansmith	efried: why are we using different initial versions for any of those calls then?
17:43:53	efried	edleafe: needs a bp fo sho
17:44:38	edleafe	efried: ok. Is there any discussion I can refer to? I'm not clear on the reasons for doing this
17:44:55	efried	dansmith: Because the newer way is usually more efficient or similar; and it gives us an easier delta when we do bump the min
17:44:57	cdent	edleafe: needs a spec, is a new feature and an api change
17:45:09	dansmith	efried: jaypipes this one is the one I have in my head right now: https://review.openstack.org/#/c/536085/9/nova/scheduler/client/report.py
17:45:26	cdent	edleafe: if you want to chat about a bit later this evening I can do a bit of a brain dump on you if you want
17:45:36	cdent	but in the midst of something at the moment
17:45:45	edleafe	cdent: you mean you haven't dumped it in an email yet? :)
17:46:14	dansmith	efried: the problem is that if placement doesn't go first, scheduler depends on it for az calculation, then we have to fail any request that cares about az if placement doesn't support 1.21 _anyway_
17:46:23	cdent	no, because I was trying not to own this one. I did have a to do list item which said "write email about consumer uuid" but deleted it when you "got ownershipw of this" :)
17:47:10	edleafe	cdent: ah, I was just volunteering my cycles to code it. If I gotta write a spec, I should at least understand the motivation behind it.
17:47:22	edleafe	(that usually helps) :)
17:47:36	efried	edleafe: I got you, gimme sec
17:47:48	cdent	edleafe: details! but yeah, I can dump some brain shortly if efried doesn't beat me to it
17:47:48	jaypipes	dansmith: you will note I did not review that.
17:49:00	dansmith	jaypipes: so cdent was culling all the microversions when, at the end of each release?
17:49:07	efried	dansmith, jaypipes: Agree that one should have had the 406-fallback conditions. I shoulda reviewed it too, but didn't. IIRC that got jammed in really late in Q because we weren't going to make granular happen in time.
17:49:16	dansmith	jaypipes: so you want a bunch of microversion handling code for within-release changes?
17:49:17	cdent	jaypipes, dansmith: we've been inconsistent about our expectation with nova-side microversion fallback. I've not been able to discern a pattern, mostly because _some_ of the time we say "we expect placement to upgrade first" at which point why bother falling back?
17:49:34	cdent	I didn't cull them all, just anything less than 1.14 (I think it was 1.14, would need to check to be sure)
17:49:48	dansmith	cdent: yeah I see some places where we do fallback, and some where we don't
17:49:49	dansmith	cdent: it was 1.14
17:50:06	jaypipes	dansmith: I'm just noting I did not review that patch (didn't know about it at all). I believe I was in Dallas that week.
17:50:46	jaypipes	dansmith: I do support the reportclient supporting fallback code (406 handling) for at least a release, yes.
17:50:47	dansmith	so what do you people propose? scheduler fails if you request an az and placement is old? fall back but make sure the old filter is enabled otherwise fail?
17:51:08	jaypipes	dansmith: yes, that sounds reasonable to me.
17:51:11	dansmith	jaypipes: we're not testing nova against an older placement, so it basically doesn't work
17:51:17	dansmith	jaypipes: seriously? that was a joke :)

Earlier   Later