| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-19 | |||
| 14:21:39 | leakypipes | fried_rice: we detect conflicts in terms of whether capacity is exceeded, that's all. | |
| 14:22:14 | leakypipes | fried_rice: i.e. does the original constraint that there was existing capacity for the allocated resources on these set of resource providers still hold. | |
| 14:23:07 | fried_rice | leakypipes That's cool. But if two threads are trying to e.g. modify allocations for the same consumer_uuid at the same time, the second one will "win". | |
| 14:23:54 | fried_rice | leakypipes Not a problem in today's world where everything is funneled through report client, and there's one compute node RP, and the consumer is an instance. | |
| 14:24:25 | leakypipes | fried_rice: no, that's not possible. one of those threads would get a 409 because the provider generation increment would fail. | |
| 14:24:43 | fried_rice | leakypipes How and why would the increment fail if we're not passing the generation into the API? | |
| 14:26:12 | leakypipes | fried_rice: because the provider generations are *read* at the start of the transaction that inserts/deletes allocation records. after those allocation records are changed, the provider generations are incremented for all providers involved in the transaction. if another thread had inserted/deleted allocation records (for that consumer or another consumer) against those same resource providers, the increment would fail. | |
| 14:26:57 | leakypipes | fried_rice: specifically, this would happen: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L281-L282 | |
| 14:27:30 | fried_rice | leakypipes I'm not talking about multiple threads in the placement code at the same time. I'm talking about multiple consumer threads making single individual atomic calls to completion. | |
| 14:29:10 | leakypipes | fried_rice: I'm going to go back to reviewing the patch series I need to get reviewed today :) you are welcome to walk through the code paths in there and identify a particular concurrency bug. | |
| 14:30:03 | leakypipes | fried_rice: you are also welcome to put a discussion on all the concurrency topics up at the PTG. I think that would be a good discussion. it's just I need to focus today on these reviews. | |
| 14:30:39 | fried_rice | isk-less) payload and PUTs back. Placement happily replaces with B's version of the allocation, which has the vif but doesn't have the disk. | |
| 14:30:39 | fried_rice | leakypipes Here's the flow: Existing instance. Shared disk manager A wants to add some disk resource. sean-k-mooney's neutron thingy B wants to add a vif or whatever. A does a GET of the current allocations. B does a GET of the current allocations. A adds his disk and PUTs back. Placement happily replaces the allocations, which now include the disk, and increments the generation. B adds his vif to the original (d | |
| 14:30:59 | leakypipes | alex_xu: are you still around? that patch you wanted a review on is dependent on this one which needs a rebase: https://review.openstack.org/#/c/479776/. are you planning on getting to that? | |
| 14:32:18 | sean-k-mooney | fried_rice: why would you not request both the disk and vif in the same allocation request? | |
| 14:32:45 | fried_rice | sean-k-mooney Because different entities are "owning" those resources. | |
| 14:32:53 | leakypipes | alex_xu: never mind. it's not a dependent patch, it's just in the same topic. damn gerrit... :( | |
| 14:33:32 | sean-k-mooney | fried_rice: differnet entityes are creating the resouce providers but the nova sechduler/conductor will be claiming both on behalf of the instance | |
| 14:33:48 | fried_rice | sean-k-mooney Sure, you *could* have it set up that way. Or, you know, not. | |
| 14:33:58 | fried_rice | sean-k-mooney At the moment, this is not a danger. | |
| 14:34:05 | fried_rice | sean-k-mooney In the future.... | |
| 14:34:28 | fried_rice | Certainly for the general case usage of the placement API. | |
| 14:34:36 | sean-k-mooney | we could technically make the neutron port the owner of the vif which would then result in two owners | |
| 14:35:09 | sean-k-mooney | *vif allocation | |
| 14:35:22 | fried_rice | The solution would be fairly simple I think: just add the generation for each RP to PUT /allocations/{consumer_uuid} and POST /allocations | |
| 14:37:43 | openstackgerrit | Merged openstack/nova master: libvirt: create vGPU for instance https://review.openstack.org/528832 | |
| 14:38:05 | fried_rice | Basically, it works as long as all the allocations for a given consumer are always only managed by one thread at a time. Which is a constraint that happens to be true today, but certainly isn't documented as a limitation of using the API. | |
| 14:38:49 | sean-k-mooney | leakypipes: alex_xu https://review.openstack.org/#/c/535642 seams to add suport for required to get /allocation_candiates but i dont see support for the Resouce1..,Required1 syntax needed for different traits per resouce. is there a followup patch for that | |
| 14:39:16 | leakypipes | sean-k-mooney: yes, we're not targeting that for queens. fried_rice has a patch up for that. | |
| 14:40:17 | fried_rice | leakypipes Quit saying that. We *are* targeting it for Queens, until something changes. We have an approved blueprint and 95% of the code merged. | |
| 14:41:05 | leakypipes | fried_rice: I won't hold up non-granular requests with traits for granular requests. | |
| 14:41:15 | leakypipes | fried_rice: does that sound more accurate? | |
| 14:41:23 | sean-k-mooney | leakypipes: ya i saw fried_rice's patch i assumed it superceeded alex's instead of building on it.thats problematic but ok. ill cherry pick fried_rice patch if i need it. | |
| 14:41:24 | fried_rice | leakypipes Agree with that (see -dev ml) | |
| 14:42:20 | fried_rice | sean-k-mooney ditto (http://lists.openstack.org/pipermail/openstack-dev/2018-January/126393.html) | |
| 14:43:11 | sean-k-mooney | so how does required work wiht nrp | |
| 14:43:28 | fried_rice | sean-k-mooney The reason my patch doesn't build on alex_xu's is because mine is 2.5 months older :) | |
| 14:44:31 | fried_rice | sean-k-mooney You get candidates such that a) all resources come from providers in the same tree, and b) the union of all the traits on providers you actually get resources from is a superset of the traits you asked for. | |
| 14:45:11 | sean-k-mooney | fried_rice: ok the union is the important bit | |
| 14:45:13 | fried_rice | sean-k-mooney Which is obviously limited e.g. you can't say I want one VIF with trait CUSTOM_PHYSNET_A and one with CUSTOM_PHYSNET_B -- for that you would need granular, period. | |
| 14:46:17 | sean-k-mooney | yes but i can say one vif and one cpu with CUSTOM_PHYSNET_A and cpu_trait_x | |
| 14:46:43 | fried_rice | sean-k-mooney Yes. And you'd better hope that your CPU doesn't have trait CUSTOM_PHYSNET_A | |
| 14:46:54 | fried_rice | (silly example in this case) | |
| 14:47:12 | sean-k-mooney | why what would happen if it did? | |
| 14:47:19 | fried_rice | You could still get a vif from physnet B and placement will think it did its job. | |
| 14:47:28 | sean-k-mooney | ah | |
| 14:47:30 | sean-k-mooney | ok | |
| 14:48:29 | sean-k-mooney | well without traits at all we would get to the compute node try to bind the port and get a vif_binding_failed from neutron then hit the retry loop until we hit max retries or got lucky | |
| 14:48:43 | fried_rice | heh, yeah | |
| 14:49:01 | fried_rice | Oh, I'm not disputing that having traits support is better than not having traits support. | |
| 14:49:31 | fried_rice | Just that single-request-group has limitations no amount of trait-finagling will overcome. | |
| 14:49:49 | sean-k-mooney | fried_rice: ya i know but that example is precisly the one im trying to avoid in my poc as a first pass use of traits | |
| 14:50:04 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: WIP: hacky negotiation of ironic microversion https://review.openstack.org/535786 | |
| 14:50:04 | fried_rice | sean-k-mooney Well, that's why we need granular :) | |
| 14:50:40 | sean-k-mooney | the minimal support will work for the simple case e.g. all interfaces on same physnet which is enough for demos | |
| 14:50:46 | fried_rice | yuh | |
| 14:50:59 | fried_rice | on the same physnet and same everything else that matters. | |
| 14:51:36 | sean-k-mooney | well we can have multiple physnets in the cloud and show that placement found the correct node | |
| 14:51:48 | fried_rice | Yup. | |
| 14:51:59 | sean-k-mooney | we would have had to use availablity zones with out tratis to do the same thing | |
| 14:52:26 | sean-k-mooney | or atleast some host_aggregate based filter | |
| 14:52:32 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: QEMU native LUKS decryption for encrypted volumes https://review.openstack.org/523958 | |
| 14:52:45 | fried_rice | And you can *probably* even ask for multiple vifs on separate subnets in the same request. As long as you only want one of each. And don't share those traits with any other provider. Which is probably doable. | |
| 14:53:16 | fried_rice | brb | |
| 14:54:01 | sean-k-mooney | ill cross that bridge when i come to it but ya granular resouce requests in queens will support all of these usecase more cleanly/correctly | |
| 14:54:08 | sean-k-mooney | * in rocky | |
| 15:01:48 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: WIP: hacky negotiation of ironic microversion https://review.openstack.org/535786 | |
| 15:02:44 | leakypipes | alex_xu: review done. | |
| 15:14:59 | rabel | hm. why is a PUT to /os-quota-sets/default working? this does not make sense in my brain | |
| 15:25:52 | leakypipes | fried_rice: just confirming... mgoddard's patches for "Call get_traits() in the resource tracker" and "Add get_traits() method to ComputeDriver" are obseleted with the update_provider_tree() work, correct? | |
| 15:26:53 | fried_rice | leakypipes Eventually, but when I asked him about it, apparently he's wanting to move forward more quickly than that. | |
| 15:27:16 | fried_rice | leakypipes Right now he's working toward what's described in his blueprint. | |
| 15:27:33 | fried_rice | leakypipes And that stuff will coexist with update_provider_tree until it doesn't. | |
| 15:28:11 | leakypipes | hmm, ok | |
| 15:28:17 | fried_rice | leakypipes Though xen is counting on using update_provider_tree, and I just found out vmware is as well. | |
| 15:28:48 | leakypipes | fried_rice: yeah, my preference for sure would be to have everyone on update_provider_tree() | |
| 15:28:49 | leakypipes | asap | |
| 15:28:51 | fried_rice | leakypipes I didn't love the idea of that redundant/extra compute driver method coming into existence for such a limited use and (hopefully) limited time... | |
| 15:28:54 | fried_rice | yeah, same | |
| 15:29:02 | leakypipes | k, at least we're on the same page. | |
| 15:29:11 | leakypipes | fried_rice: your series is looking great | |
| 15:29:18 | fried_rice | Thanks! | |
| 15:29:27 | leakypipes | I'm hoping we can get gibi or stephenfin on it. | |
| 15:30:41 | fried_rice | Late on a Friday... and both of them forgot their Friday nicks... | |
| 15:31:32 | leakypipes | fried_rice: well that is COMPLETELY UNACCEPTABLE. | |
| 15:32:09 | fried_rice | ikr | |
| 15:32:19 | fried_rice | At least figleaf is on board | |
| 15:34:01 | rgerganov | fried_rice, i am trying to implement update_provider_tree for the vmware driver and I'll be happy to work with you on https://review.openstack.org/#/c/520246 and the rest | |
| 15:34:04 | openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Cleanup redundant want_version assignment. https://review.openstack.org/535801 | |
| 15:34:34 | fried_rice | rgerganov Ah, I was looking for rado - is that not also you? | |
| 15:34:57 | rgerganov | fried_rice, well i was also looking for efried but found fried_rice :) | |
| 15:34:58 | fried_rice | gibi You can redeem yourself by reviewing the series behind leakypipes | |
| 15:35:13 | fried_rice | rgerganov I make no apologies for my Fun Friday nickname | |
| 15:36:27 | fried_rice | rgerganov I'm going to need to rebase the top of the series to account for some stuff takashin did for including request IDs in deletion calls. | |
| 15:37:00 | fried_rice | Which leaves me in kind of a bind because the bottom of the series is close to mergeable, and I don't want to rebase it and set back the review process. | |
| 15:37:16 | fried_rice | Though I guess if I do it now, as long as it's auto-rebase-able, I keep reviews... | |
| 15:37:40 | rgerganov | fried_rice, yeah understood | |
| 15:38:16 | finucannot | leakypipes: What was this now? | |
| 15:38:45 | fried_rice | finucannot Welcome! | |
| 15:38:49 | leakypipes | finucannot: starting here: https://review.openstack.org/#/c/533244/ | |