| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-11-01 | |||
| 18:10:58 | mriedem | that's in the API | |
| 18:11:18 | efried | but we're not using that in the scheduler yet? | |
| 18:11:43 | mriedem | the mirrored aggregates stuff? yes there are pre-request placement filters that rely on it (or something external doing the mirroring) | |
| 18:11:56 | mriedem | i'm not sure what that has to do with the cache / refresh for aggregates in all the computes | |
| 18:12:39 | mriedem | iow, i'm not sure what the cache in the compute buys us | |
| 18:12:45 | efried | yeah, I'm actually trying to think what we actually use the cache for at all... right. | |
| 18:13:50 | efried | cdent has been grousing for a while that we should just be able to make placement calls when we need 'em. | |
| 18:14:14 | sean-k-mooney | if we really wanted to make the storm less likely we could use a random prime ofset for the update | |
| 18:14:43 | efried | I was thinking it, but then you said it. | |
| 18:15:10 | mriedem | oslo already does something like that for periodics | |
| 18:15:28 | efried | Trying to think what it would take to rip out the cache completely. | |
| 18:15:38 | belmoreira | I'm changing this option, it will take ~2h to propagate. Will let you know the result | |
| 18:15:44 | efried | ack | |
| 18:16:00 | belmoreira | I have to leave now for some minutes. Thanks for all your help | |
| 18:16:22 | efried | o/ | |
| 18:17:34 | efried | mriedem: We use the cache data so the virt driver has the opportunity every periodic to update the provider tree. | |
| 18:18:19 | efried | mriedem: assuming stable placement, no _refresh'ing, we would be doing a helluva lot fewer calls | |
| 18:18:31 | efried | and that's also why we cache agg data. Because upt gets to muck with that stuff also. | |
| 18:18:45 | mriedem | but nothing does right now right? | |
| 18:18:48 | mriedem | for aggregates | |
| 18:19:00 | mriedem | and assuming inventory isn't wildly changing on a compute node, we don't really need the cache | |
| 18:19:49 | efried | not sure I'm following. | |
| 18:20:03 | efried | are you saying "as long as nothing is changing, we don't need to call update_provider_tree" ? | |
| 18:20:25 | mriedem | update_provider_tree is what returns the inventory from the driver to the RT to push off to placement every 60 seocnds | |
| 18:20:27 | mriedem | *seconds | |
| 18:20:28 | mriedem | right? | |
| 18:21:04 | efried | Yes | |
| 18:21:05 | mriedem | and assuming that disk/ram/cpu on a host doesn't change all that often, at least without a restart of the host, it seems odd we need to cache that information | |
| 18:21:18 | efried | But how else would we know whether to push the info back to placement? | |
| 18:21:51 | mriedem | in the before upt times, didn't the RT/report client just pull inventory, compare to what was reported by the driver, and the PUT it back if there were changes? | |
| 18:22:08 | efried | What does "pull inventory" mean, though? | |
| 18:22:22 | efried | pull from placement | |
| 18:22:28 | mriedem | GET /resource_providers/{rp_uuid}/inventories | |
| 18:22:29 | efried | i.e. GET /rps/UUID/inventory | |
| 18:22:32 | efried | yeah | |
| 18:22:42 | sean-k-mooney | efried: well the driver could have a perodic check but rememebr the last value it sent and only send a value if it detactes there was a chage | |
| 18:22:50 | efried | sean-k-mooney: ^ cache | |
| 18:22:58 | sean-k-mooney | that not the same as a cache | |
| 18:23:00 | efried | and that's what we do | |
| 18:23:39 | mriedem | get_provider_tree_and_ensure_root is what gets the provider tree from the report client and pulls the current inventory from placement, yes? | |
| 18:23:48 | efried | yes | |
| 18:23:48 | mriedem | and also checks to see that the provider exists on every periodic | |
| 18:23:51 | mriedem | which we should actually know | |
| 18:24:37 | efried | yeah, we could conceivably expect the compute RP not to disappear once we've created it. | |
| 18:24:45 | efried | I mean, I don't know how resilient we're trying to be in the face of OOB changes. | |
| 18:24:59 | efried | we do offer a placement CLI, not just for GETs but for writes as well | |
| 18:25:47 | mriedem | the compute service record, compute node record, and rp can all be deleted if the compute service record is deleted | |
| 18:25:56 | mriedem | but to get the compute service record back, you have to restart the compute service | |
| 18:26:02 | mriedem | to recreate the record which would also re-create the compute node | |
| 18:26:04 | mriedem | and then the RP | |
| 18:26:13 | mriedem | since we know https://github.com/openstack/nova/blob/1e823f21997018bcd197057ebd4d6207a5c54403/nova/compute/resource_tracker.py#L780 | |
| 18:26:14 | sean-k-mooney | efried: true but in that case could we do what we do with neutron and have placment send a notificaiton to nova that it was changed instead of polling | |
| 18:26:16 | mriedem | we can pass that down | |
| 18:26:22 | mriedem | i'll hack something up quick | |
| 18:26:24 | efried | IOW, only have the create code path on start=True | |
| 18:26:36 | efried | and don't bother with the existence check otherwise | |
| 18:26:45 | mriedem | not even start true | |
| 18:26:59 | mriedem | since https://github.com/openstack/nova/commit/418fc93a10fe18de27c75b522a6afdc15e1c49f2 we have a flag to pass through when we create the compute node | |
| 18:27:05 | mriedem | we just don't plumb it far enough | |
| 18:27:10 | mriedem | i can push up a change that does | |
| 18:27:25 | efried | mriedem: That's what pike looked like, though. The stuff that's causing the spike is necessary for *enablement* of nrp, which we haven't started using yet | |
| 18:27:29 | mriedem | that might save precious ms for belmoreira :) | |
| 18:27:31 | efried | so it seems useless atm | |
| 18:27:52 | efried | but as soon as we get e.g. neutron or cyborg adding shit to the tree, we're going to need to do that ?in_tree call every periodic. | |
| 18:28:14 | efried | unless there's some kind of async notification hook to trigger a refresh | |
| 18:28:24 | efried | yeah, what sean-k-mooney said. | |
| 18:28:25 | mriedem | i'm saying we can resolve this todo i think https://github.com/openstack/nova/blob/1e823f21997018bcd197057ebd4d6207a5c54403/nova/scheduler/client/report.py#L1011 | |
| 18:28:28 | mriedem | can we agree on that? | |
| 18:29:04 | mriedem | neutron sending an event is possible, but it's also per instance... | |
| 18:29:06 | mriedem | not per host | |
| 18:29:22 | efried | mriedem: unfortunately not anymore, because we plan to allow other-than-nova to edit the tree. | |
| 18:29:39 | mriedem | including delete the compute node root provider? | |
| 18:29:44 | mriedem | that nova creates? | |
| 18:29:50 | efried | no, not that. | |
| 18:29:58 | mriedem | well isn't that what that todo is all about? | |
| 18:30:06 | mriedem | create the resource provider for the compute node if it doesn't exist | |
| 18:30:13 | efried | no | |
| 18:30:28 | sean-k-mooney | mriedem: we are going to allow them to manage there onw subtrees only so the wont be allowed to modify any nodes created by nova | |
| 18:31:09 | efried | mriedem: You could probably factor out *just* the root provider part of that; but you can't get rid of the whole method. | |
| 18:31:42 | mriedem | i'm not saying remove the method | |
| 18:31:47 | efried | and the GET that _ensure_resource_provider is doing is the ?in_tree one that we can't get rid of anyway. | |
| 18:35:13 | mriedem | because of something external adding/removing things from the root | |
| 18:35:24 | mriedem | right? | |
| 18:36:56 | sean-k-mooney | well external entitiy can only leagally add nested resouce providers to the root node they cant add invetories or traits | |
| 18:37:27 | sean-k-mooney | technicall the api does not enforce that as we dont have owner of resouce proivers in the api however | |
| 18:38:28 | mriedem | what i'm hearing is we can't remove this todo https://github.com/openstack/nova/blob/1e823f21997018bcd197057ebd4d6207a5c54403/nova/scheduler/client/report.py#L1011 even if we know we didn't just create the root compute node, because we need to call it anyway to determine if there are new nested providers under that pre-existing compute node | |
| 18:38:40 | mriedem | s/remove/resolve/ | |
| 18:38:50 | mriedem | iow, the todo should be removed b/c we can't do anything about it | |
| 18:38:56 | mriedem | even if we *know* the compute node record was just created | |
| 18:39:23 | sean-k-mooney | am i dont know if we need to know if there are new nested resouce providers | |
| 18:39:38 | mriedem | that's the whole in_tree thing i thought | |
| 18:39:41 | sean-k-mooney | in fact i would asser as the compute node we dont need to know that | |
| 18:40:48 | mriedem | ok well what i'm saying is we (the RT) know when we created a new compute node record, and thus need to create its resource provider, i guess i'll wait for someone to tell me if that's worth doing so we can avoid the GET /resource_providers?in_tree=<uuid of the thing we just created and thus doesn't exist yet> case | |
| 18:42:00 | sean-k-mooney | mriedem: in that case i think you are right wew dont need the /resource_providers?in_tree=<thing i just created> call | |
| 18:43:11 | sean-k-mooney | the placement api will not allow me to create a resouce provider with a parrent uuid that does not exist | |
| 18:44:18 | efried | sean-k-mooney: We *do* need to know if there are new nested providers. | |
| 18:44:52 | sean-k-mooney | efried: there cant be nested resouce providers of a compute node if we have not created the compute node yet right | |
| 18:45:08 | efried | That's what ?in_tree is about, though. | |
| 18:45:26 | efried | ?in_tree=$compute_rp gives me the compute RP and any descendants. | |
| 18:45:35 | sean-k-mooney | yes | |