Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-11
15:13:03 bhagyashris yes i restart n-cpu
15:14:28 bhagyashris but it's giving problem in second patch now that the provider_tree has no attribute data
15:14:35 efried bhagyashris: Well, there *is* no get_inventory in the libvirt driver anymore, so if you're seeing inventory at all, it must have come from here.
15:15:03 efried bhagyashris: I didn't run any of this code, even in a unit test context, so I very well may have effed something up. Are you seeing an exception?
15:16:48 bauzas efried: jaypipes: thanks for the early reviews of https://review.openstack.org/#/c/552924/7
15:17:14 bauzas efried: jaypipes: I think I need to understand more how numbered request groups would work
15:17:20 bhagyashris Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager File "/opt/stack/nova/nova/compute/manager.py", line 7311, in update_available_resource_for_node
15:17:20 bhagyashris Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager Traceback (most recent call last):
15:17:20 bhagyashris efried: I saw this exception at n-cpu Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager [None req-b17184a8-7d31-4c3d-9b8b-d1f1ddb6860e None None] Error updating resources for node openstack-VirtualBox.: AttributeError: 'ProviderData' object has no attribute 'data'
15:17:22 bhagyashris Apr 11 20:46:08 openstack-VirtualBox nova-compute[5703]: ERROR nova.compute.manager rt.update_available_resource(context, nodename)
15:17:32 efried bhagyashris: okay, fixing...
15:17:57 efried whoops, that was a flub
15:18:13 openstackgerrit Eric Fried proposed openstack/nova master: WIP: libvirt: Don't report DISK_GB if sharing https://review.openstack.org/560459
15:18:14 efried bhagyashris: ^
15:20:26 efried bauzas: Have you memorized the granular spec?
15:20:44 bauzas efried: tbh, I need to look at it again
15:21:08 openstackgerrit Matt Riedemann proposed openstack/nova master: RT: replace _instance_in_resize_state with _is_trackable_migration https://review.openstack.org/560467
15:21:09 mriedem this was a fun bit of history ^
15:21:16 efried bauzas: Numbered request groups work like this:
15:21:16 efried Resources specified in *different* numbered request groups may or may not land on same/different providers.
15:21:16 efried Resources specified *within* a single numbered request group are *guaranteed* to land on the *same* provider.
15:21:42 bauzas efried: I thought asking resources1:STH=1&resources2:STH_ELSE=2 would make those two things on separate RPs
15:21:54 efried bauzas: That has been a source of confusion, but no.
15:21:58 bauzas ok, that's the second assumption I made
15:22:02 bauzas I made wrong
15:22:21 bauzas but may or may not is not really a big deal to me
15:22:31 efried bauzas: We will eventually need that semantic - and "deliberate sharding" would be a use case driving it - but we don't have it with the spec as currently written.
15:22:32 bauzas the first assumption is crucial to me
15:22:55 bauzas the second one (different groups leading or not on same RP) is not really important to me
15:23:09 bauzas what I want is affinity, not anti-affinity
15:23:15 efried bauzas: Perfect.
15:23:36 efried bauzas: In case you missed it, I summarized four use cases and what we can/can't do about them. It's at the top part of my review.
15:23:38 bauzas either customers care about NUMA specifics and then numbered req groups is the perfect answer
15:23:56 bauzas or, they don't care about NUMA specifics, and then they don't care where the resources land
15:23:57 efried (I know sometimes those don't get seen because folks dive right into the comments within the file)
15:24:11 bauzas efried: I saw your top-level comment, yup
15:24:11 efried bauzas: Good
15:24:21 bauzas thanks for that, appreciated
15:24:28 bauzas I sometimes do that to justify a -1
15:24:41 bauzas when for example there are 30 comments but only one means -1 to me
15:24:57 efried bauzas: So the use case you *don't* care about is my third bullet.
15:25:05 bauzas correct
15:25:16 bauzas efried: I gave one example of NUMA affinity for vGPUs
15:25:23 bauzas https://devblogs.nvidia.com/performance-portability-gpus-cpus-openacc/
15:25:34 bauzas efried: just for your context understanding ^
15:25:36 kashyap mriedem: When you get a moment, a small update on this: https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bump_min_libvirt_and_qemu
15:25:56 efried bauzas: Right; that's my second bullet. If that's a critical use case, then we need to invent something more.
15:25:59 kashyap mriedem: There's about 14 patches that I need to queue in that drop the compat code. Right now there are only 4
15:26:26 efried bauzas: Concept of "same subtree"
15:26:28 kashyap mriedem: I am resolving broken tests as I find them. 4 more to go (that are bit more involved, noted them here: https://review.openstack.org/#/c/558783/7)
15:26:41 kashyap mriedem: Finally, a question on unit tests for you here: https://review.openstack.org/#/c/560365/
15:26:43 bauzas efried: why ?
15:26:53 bauzas efried: VGPU is a standard class
15:27:04 bauzas like VCPU and MEMORY_MB
15:27:07 efried Because VGPU and VCPU are in different resource providers.
15:27:20 efried bauzas: So you can't put them in the same numbered request group.
15:27:27 bauzas efried: that will depend on the operator
15:27:30 kashyap mriedem: Also, I can understand if you are of the view of: "I won't touch them until Zuul is +1 on them, and you've dropped all no-more-relevant compat code" :-)
15:27:35 efried bauzas: And if you put them in different request groups, they aren't guaranteed to land on the same NUMA node.
15:27:39 bhagyashris efried: it's working now :)
15:28:03 efried bhagyashris: Like, fully working? Like the DISK_GB is being reported by the sharing provider and not by the compute node provider?
15:28:08 bauzas efried: but if they want NUMA affinity for VGPUs, then the doc will say : enable the conf opt saying "I want VCPU and VGPU to be NUMA specific"
15:28:17 bauzas efried: in that case, they will be on the same RP
15:28:25 bauzas efried: which will make the thing work
15:28:39 mriedem kashyap: i'd prefer to not dig into those until tests are passing
15:29:05 efried bauzas: It would. But saying that the VGPU inventory has to be on the same RP as the VCPU inventory - that's not a restriction that's going to remain viable for very long.
15:29:05 bauzas efried: we could later add a new param that specifically request the notion of the "subtree" parenting, but I leave that out of that spec then
15:29:25 kashyap mriedem: Sure, that's perfectly fine. I will be able to full spend time on it only from Friday afternoon onwards.
15:29:39 bauzas efried: sure, but I don't want to boil the ocean and keep the NUMA spec minimalist
15:29:42 bhagyashris efried: yes it removes the CN RP DISK_GB inventory from inventory table and now it's reporting the resource against the CN and shared RP :)
15:29:48 efried bhagyashris: WOHOO! jaypipes ^^
15:30:11 bauzas efried: I can leave a note explaining the caveat, and someone in the future (possibly being me) could write another spec describing the problem and the solution
15:30:14 efried bauzas: Okay, then the spec needs to state that restriction, for sure, cause that's significant.
15:30:41 bauzas efried: that's correct, I'll amend the spec accordingly and explicitely state the limitation, exactly like I did for huge pages
15:30:51 bauzas efried: thanks for your help!
15:31:02 bhagyashris efried: thanks for your inputs :) Now it's EOD for me leaving...
15:31:29 efried bauzas: TBC: the restriction means that the ascii diagram you've shown on L125 (where the VGPU and VF inventories are separated from the VCPU/PCUP/MEMORY_MB) will *not* work.
15:31:44 bauzas that's right, I need to amend the diagram too
15:31:50 efried bauzas: Cool cool.
15:31:57 efried bhagyashris: Thank you very much for working this.
15:32:55 efried jaypipes: To summarize, bhagyashris validated that these two patches DTRT wrt sharing DISK_GB for libvirt: https://review.openstack.org/560444 https://review.openstack.org/560459
15:33:19 jaypipes efried: DTRT?
15:33:22 jaypipes do the right thign?
15:33:29 efried yes
15:33:33 jaypipes you kids...
15:33:56 jaypipes efried, bhagyashris: ok, cool. that's kind of exactly what I expected. :)
15:35:53 efried jaypipes: Now somebody ought to finish up the tests on those patches and put 'em to bed.
15:42:43 jaypipes bauzas: you realize that https://devblogs.nvidia.com/performance-portability-gpus-cpus-openacc/ is not talking about CPU and GPU sharing memory, right?
15:42:56 bauzas right
15:43:04 bauzas it's about portability
15:43:28 bauzas memory isn't shared on a SOC for a nvidia card
15:43:44 bauzas so you're right, that's a separate memory map
15:44:57 jaypipes bauzas: you wrote "I have many internal customers that are actually very interested in having NUMA affinity for having the GPU memory close as possible to the NUMA memory controller used by the virtual CPU" <-- that is just not the case.
15:45:17 jaypipes bauzas: either that, or your internal customers don't actually know what they're asking for.
15:45:45 bauzas jaypipes: I can certainly ask them for clarification about their needs
15:46:15 jaypipes bauzas: in any case, that wasn't the reason for the -1 :) was just mentioning the use case could have been tighter.
15:46:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove RequestContext.instance_lock_checked https://review.openstack.org/554378
15:47:32 bauzas jaypipes: yeah, no worries, I got it
15:47:43 bauzas jaypipes: and tbh, I appreciate you challenging me on the need
15:47:57 bauzas because it helps me understand why my customers are pushing for it

Earlier   Later