| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-04-19 | |||
| 14:45:24 | mriedem | the libvirt overhead one kind of screws us | |
| 14:45:24 | melwitt | mriedem: is anyone working on the intermittent ironic test failure? I want to take a stab at it if not | |
| 14:45:28 | mriedem | melwitt: no | |
| 14:45:30 | bauzas | mriedem: exactly this | |
| 14:45:41 | mriedem | melwitt: i was considering just skipping it for now | |
| 14:45:41 | dansmith | hmm, I don't remember the config bit | |
| 14:45:50 | mriedem | dansmith: might not be config, but it's based on image meta | |
| 14:46:00 | gcb | mriedem, Do you get error from https://review.openstack.org/#/c/456397 ? It seems oslo.config have recursion calls | |
| 14:46:02 | johnthetubaguy | melwitt: I saw talk of something in the neutron channel about ironic, but seemed more like a hard failure | |
| 14:46:09 | dansmith | mriedem: yeah image meta | |
| 14:46:24 | mriedem | gcb: it's multiple changes failing on this | |
| 14:46:27 | mriedem | according to logstash | |
| 14:46:45 | melwitt | mriedem: okay. I think the right thing to do is just raise the Timeout exception. we shouldn't be doing a real timeout | |
| 14:46:55 | melwitt | (as discussed on the bug) | |
| 14:47:11 | mriedem | melwitt: so remove all of the fake stub methods that are doing logic to trigger the timeout? | |
| 14:47:15 | mriedem | melwitt: that's the easy fix | |
| 14:47:34 | mriedem | i think i get the point of why the test was doing that though | |
| 14:47:39 | mriedem | to stub and mimic what the driver is doing | |
| 14:47:43 | mriedem | but it's pretty fragile | |
| 14:48:07 | melwitt | mriedem: yeah, stub the looping call to raise the timeout. I thought the test wants to check that it does the right thing when a timeout occurs, that it raises the ConsoleNotAvailable | |
| 14:49:21 | melwitt | that is, I think we can assume that oslo_service looping calls work correctly, else we're also testing oslo functionality | |
| 14:49:30 | mriedem | melwitt: yeah it's a fine line | |
| 14:49:49 | bauzas | mriedem: dansmith: we also lookup the instance if we don't have the numa topology set in the instance object for knowing the overhead | |
| 14:49:53 | mriedem | melwitt: i'm fine with doing that | |
| 14:49:56 | dansmith | mriedem: so, a couple things | |
| 14:50:00 | mriedem | melwitt: because my solution was skipTest :) | |
| 14:50:18 | gcb | mriedem, it's really strange, nova.tests.unit.test_rpc.TestRPC.test_add_extra_exmods is simple test. what I found just deprecated warning from oslo.messaging | |
| 14:50:41 | dansmith | mriedem: (1) presumably we'll do this without the cache first, (2) we can cache on flavorid/hv/ver first, (3) we might just make a hash key function that generates a key based on all the things currently known to matter | |
| 14:51:09 | mriedem | dansmith: where (3) could take into account the image id too? | |
| 14:51:12 | dansmith | mriedem: and (4) the penalty for missing means we *might* fail to build because of overhead on the compute, which sucks, but.. builds can fail | |
| 14:51:17 | dansmith | mriedem: yeah | |
| 14:51:26 | mriedem | dansmith: and the image meta could change w/o changing the image id | |
| 14:51:37 | dansmith | mriedem: or just hash in the actual fields we know are ever looked at | |
| 14:51:45 | dansmith | mriedem: right well, that's why I was thinking ^ instead of id, but yeah | |
| 14:51:55 | bauzas | mriedem: dansmith: so just to make it clear, we introspect the instance every time we claim the resource in the RT if libvirt | |
| 14:51:57 | melwitt | mriedem: heh, okay. I'll upload it and we'll see | |
| 14:52:02 | mriedem | changing meta on an image w/o dumping the old image seems like a bad idea | |
| 14:52:10 | dansmith | because if you have two images, but without cpu policy set, those shouldn't need to be done separately | |
| 14:52:17 | mriedem | bauzas: if libvirt/hyperv/xen | |
| 14:52:18 | bauzas | mriedem: dansmith: for knowing the numa topology | |
| 14:52:37 | mriedem | bauzas: where do we even set instance.numa_topology during instance creatE? | |
| 14:52:41 | dansmith | bauzas: I know, what's your point? hopefully that all goes away anyway | |
| 14:52:50 | dansmith | mriedem: it's during virt build I think | |
| 14:52:58 | mriedem | heh | |
| 14:53:00 | mriedem | so... | |
| 14:53:01 | dansmith | well, that's where we fill it out | |
| 14:53:05 | bauzas | dansmith: the fact that we could potentially not be able to cache things | |
| 14:53:22 | bauzas | because there could be only one occurence | |
| 14:53:33 | mriedem | sfinucan: do you know where we set instance.numa_topology during instance build? | |
| 14:53:36 | bauzas | I mean a cache hit of 1 occurrence | |
| 14:53:46 | sfinucan | mriedem: We set it twice | |
| 14:54:06 | sfinucan | Once during the filtering stage (to see if an instance would fit on the host) | |
| 14:54:08 | dansmith | bauzas: the looking at the instance.numa part is just to calculate the same thing, but without having flavor/image meta | |
| 14:54:17 | sfinucan | and once when actually booting the instance | |
| 14:54:23 | sfinucan | I'll get the two calls now | |
| 14:54:31 | dansmith | bauzas: you would get the same answer from the instance as from the metadata | |
| 14:54:57 | dansmith | mriedem: ^ | |
| 14:55:16 | mriedem | sfinucan: looks like RT.instance_claim sets it on the instance | |
| 14:55:59 | bauzas | dansmith: mmm, you're right, two instances sharing same flavors/images would necessarly have the same overhead even with numa | |
| 14:56:08 | dansmith | right, | |
| 14:56:17 | dansmith | it's just that we get the value either calculated from meta, or already set if the instance has had it done | |
| 14:56:21 | dansmith | it's still the same answer | |
| 14:56:23 | bauzas | right | |
| 14:56:31 | bauzas | so, the key is the tuple (image, flavor) | |
| 14:56:35 | bauzas | well | |
| 14:56:38 | dansmith | no | |
| 14:56:46 | bauzas | (image, flavor, hv_type, hv_version) | |
| 14:56:51 | dansmith | yeah | |
| 14:56:51 | dansmith | the key is a hash value of flavorid, a set of image properties, hv, version) | |
| 14:57:00 | dansmith | not image id though | |
| 14:57:08 | mriedem | what other nasty things is the RT doing that we aren't thinking about? | |
| 14:57:12 | mriedem | cpu pinning stuff on the host? | |
| 14:57:21 | edleafe | Can we simplify things and get a good estimate of what the max total overhead could ever be on a particular host? | |
| 14:57:24 | bauzas | mriedem: I think we CPU pin indeed | |
| 14:57:27 | edleafe | And then just reserve that? | |
| 14:57:48 | mriedem | edleafe: as cdent noted earlier, we don't get the reserved inventory for this | |
| 14:57:58 | bauzas | mriedem: AFAIK, we just verify if we can accept a host for CPU pinning, but we do set the pin between pCPU and vCPU at the RT stage | |
| 14:58:08 | sfinucan | mriedem: Yeah, you're looking for calls to 'numa_fit_instance_to_host' | |
| 14:58:08 | bauzas | sfinucan: correct ? ^ | |
| 14:58:32 | edleafe | mriedem: I mean when creating the resource provider, add the total overhead it might need to the reserved part for the hypervisor | |
| 14:58:33 | sfinucan | bauzas: Correct | |
| 14:58:37 | cdent | mriedem: that's not what I said. What I said was _if_ we change reserved, we don't change the allocations. I actually think making a good guess at what global overhead is at boot time, and adding that to reserved is the right way to go | |
| 14:58:56 | cdent | s/boot/host boot/ | |
| 14:59:01 | sfinucan | We call 'numa_fit_instance_to_host' to make sure that the instance would fit on a host with a given CPU pinning permutation | |
| 14:59:09 | dansmith | cdent: meaning claim more and update later? | |
| 14:59:31 | mriedem | edleafe: cdent: i don't really know how we can safely estimate that, | |
| 14:59:31 | edleafe | dansmith: no, just claim what the instance uses | |
| 14:59:42 | dansmith | edleafe: eh? | |
| 14:59:44 | mriedem | w/o knowing which flavors will be used, which scheduler filters are being used, etc | |
| 14:59:46 | cdent | dansmith: no, continue making allocations using the non-overhead values. incorporate the expected in 'reserved' of inventory | |
| 14:59:47 | sfinucan | It's the easiest way to check that out for things like cpu_thread_policy, where we can't do a simple "have I X free CPUs" check | |
| 14:59:51 | dansmith | edleafe: we're claiming before we know that, that's the whole point | |
| 15:00:02 | bauzas | okay, here is a thinh | |
| 15:00:04 | edleafe | dansmith: it would already be accounted for | |
| 15:00:09 | dansmith | cdent: oh, that's what I initially was saying, yeah | |
| 15:00:12 | bauzas | given all the nasty and fancy things we do with RT | |
| 15:00:15 | edleafe | in the RP's reserved | |
| 15:00:17 | bauzas | I have a though | |
| 15:00:38 | bauzas | why not just posting an allocation without all that stuff, and do modify the allocation at the RT state ? | |
| 15:00:43 | dansmith | cdent: not claim the flavor+overhead, but claim flavor, and require reserved to be set high enough to cover it | |