| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-06-24 | |||
| 17:36:23 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add database schema upgrade check https://review.opendev.org/667047 | |
| 17:36:30 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Add database schema upgrade check https://review.opendev.org/667047 | |
| 17:53:21 | dansmith | mriedem: sorry was in a meeting | |
| 18:42:40 | openstackgerrit | sean mooney proposed openstack/nova master: Revert resize: wait for events according to hybrid plug https://review.opendev.org/667177 | |
| 19:14:35 | openstackgerrit | Merged openstack/nova master: Revert "Revert resize: wait for events according to hybrid plug" https://review.opendev.org/667035 | |
| 19:24:17 | openstackgerrit | Eric Fried proposed openstack/nova master: hacking: Resolve W503 (line break occurred before a binary operator) https://review.opendev.org/651555 | |
| 19:24:17 | openstackgerrit | Eric Fried proposed openstack/nova master: hacking: Resolve E741 (ambiguous variable name) https://review.opendev.org/652103 | |
| 19:24:18 | openstackgerrit | Eric Fried proposed openstack/nova master: hacking: Resolve W605 (invalid escape sequence) https://review.opendev.org/652104 | |
| 19:24:18 | openstackgerrit | Eric Fried proposed openstack/nova master: tests: Use consistent URL regex substitution https://review.opendev.org/665949 | |
| 19:24:19 | openstackgerrit | sean mooney proposed openstack/nova master: Revert resize: wait for events according to hybrid plug https://review.opendev.org/667177 | |
| 19:26:36 | sean-k-mooney | efried: thanks for reviewing the libvirt pqos spec. ill review and responed tomorrow | |
| 19:26:46 | efried | yahyoubetcha | |
| 19:30:30 | openstackgerrit | Dustin Cowles proposed openstack/nova-specs master: Spec: Use OpenStack SDK in Nova https://review.opendev.org/662881 | |
| 19:51:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add InstanceAction/Event create() method https://review.opendev.org/614036 | |
| 19:51:30 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add Instance.hidden field https://review.opendev.org/631123 | |
| 19:51:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add TargetDBSetupTask https://review.opendev.org/627892 | |
| 19:51:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add CrossCellMigrationTask https://review.opendev.org/631581 | |
| 19:51:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Execute TargetDBSetupTask https://review.opendev.org/633853 | |
| 19:51:32 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add prep_snapshot_based_resize_at_dest compute method https://review.opendev.org/633293 | |
| 19:51:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add PrepResizeAtDestTask https://review.opendev.org/627890 | |
| 19:51:33 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add prep_snapshot_based_resize_at_source compute method https://review.opendev.org/634832 | |
| 19:51:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova.compute.utils.delete_image https://review.opendev.org/637605 | |
| 19:51:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add PrepResizeAtSourceTask https://review.opendev.org/627891 | |
| 19:54:37 | efried | johnthetubaguy: Would you please have a (hopefully final) look at https://review.opendev.org/#/c/658716/ and see if it's approvable? | |
| 20:00:01 | openstackgerrit | Matt Riedemann proposed openstack/os-traits master: Add COMPUTE_STATUS_DISABLED trait https://review.opendev.org/667209 | |
| 20:15:59 | mriedem | efried: i have one of your favorite all time questions, | |
| 20:16:17 | efried | airspeed velocity of an unladen European swallow? | |
| 20:16:55 | mriedem | SchedulerReportClient.set_traits_for_provider uses the ProviderTree cache https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1019 | |
| 20:17:08 | mriedem | but get_provider_traits does not https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L373 | |
| 20:17:47 | mriedem | i'm working on splitting up https://review.opendev.org/#/c/654596/ and the first thing I was going to do was https://review.opendev.org/#/c/654596/2/nova/scheduler/client/report.py as a separate patch which was set_provider_traits w/o the cache, | |
| 20:18:06 | mriedem | so the ComputeVirtAPI/virt driver usage for get/set provider traits would not be tied to a cache | |
| 20:18:25 | mriedem | but now i can't remember why i decided to go the no-cache route, | |
| 20:18:45 | mriedem | since i could link the compute manager / virt api / driver / RT reportclient together and then they'd all be using the same instance of the ProviderTree cache | |
| 20:18:51 | mriedem | and maybe that's desirable | |
| 20:19:28 | mriedem | so i guess main question is, is there any reason get_provider_traits isn't using the cache? | |
| 20:22:03 | mriedem | my more nagging question is if i should try to share the reportclient among compute/rt/virtapi and lean on the cache...though i'm not sure i trust the cache (i guess we'd know if the cache was wrong when we try to set traits on the provider using the wrong generation) | |
| 20:22:24 | efried | mriedem: get_provider_traits is in code paths that have already decided whether the cache is in play or not. It's kind of one of those "low-level" methods that probably - I haven't checked - started off as private and was developed before we had fully developed the caching model. | |
| 20:23:26 | efried | set_traits_for_provider is currently only being used in the rt resource update loop, which has a retry on generation conflicts, i.e. "using" the cache "safely". | |
| 20:24:04 | efried | In principle, up to this point I think we've been avoiding using the cache for any report client *outside* of the compute service. | |
| 20:24:15 | efried | are you intending to *write* traits from the *scheduler*? | |
| 20:24:24 | efried | or conductor | |
| 20:24:27 | efried | ? | |
| 20:26:11 | mriedem | no | |
| 20:26:30 | mriedem | ComputeVirtAPI is part of the ComputeManager/ComputeDriver/RT turducken | |
| 20:27:10 | efried | IMO for "compute disabled" filter you should not use the cache, so get_provider_traits ought to be good as is. | |
| 20:27:14 | mriedem | the trait stuff in this case can be managed started from the compute manager (via call from the API), the driver, or the RT - and the former two are using the same VirtAPI instance | |
| 20:28:08 | efried | you may have to walk me through the flow at a high level here, or I can reread the spec quick... | |
| 20:28:12 | mriedem | ok if i'm not using the cache then i do need https://review.opendev.org/#/c/654596/2/nova/scheduler/client/report.py to set w/o the cache as well | |
| 20:28:27 | sean-k-mooney | mriedem: can it be managed from teh compute manager if the host is down | |
| 20:28:34 | mriedem | sean-k-mooney: no | |
| 20:28:41 | sean-k-mooney | that what i assumed | |
| 20:28:55 | sean-k-mooney | so if the host is actully down how would we update the trait | |
| 20:29:01 | mriedem | we wouldn't | |
| 20:29:18 | mriedem | see the note here https://specs.openstack.org/openstack/nova-specs/specs/train/approved/pre-filter-disabled-computes.html#scheduler-changes | |
| 20:29:47 | efried | mriedem: I'm not sure I see the purpose of the split in https://review.opendev.org/#/c/654596/2/nova/scheduler/client/report.py | |
| 20:29:53 | sean-k-mooney | ah right | |
| 20:29:55 | efried | where were you getting the generation from? | |
| 20:30:06 | sean-k-mooney | this is just covering disabled vs enabled not up vs down | |
| 20:30:12 | efried | if not from the cache? | |
| 20:30:26 | mriedem | efried: | |
| 20:30:27 | mriedem | get_provider_traits | |
| 20:30:34 | mriedem | returns a named tuple | |
| 20:30:38 | mriedem | with the traits list and the gen | |
| 20:31:37 | mriedem | i wasn't going to use set_traits_for_provider because if i'm not relying on the cache at all, then i don't want this to f me: | |
| 20:31:37 | mriedem | if not self._provider_tree.have_traits_changed(rp_uuid, traits): | |
| 20:32:03 | mriedem | i'm doing that myself here https://review.opendev.org/#/c/654596/2/nova/compute/manager.py@494 | |
| 20:33:55 | efried | mriedem: compute manager is same process (and using same report client) as RT? | |
| 20:34:02 | efried | (sorry if I'm asking you to repeat yourself) | |
| 20:34:10 | mriedem | yeah | |
| 20:34:16 | efried | then the cache is hot | |
| 20:34:29 | efried | and you don't need to duplicate all that logic. | |
| 20:34:37 | mriedem | https://review.opendev.org/#/c/654596/2/nova/compute/manager.py@573 | |
| 20:34:57 | efried | but you're concerned that the cache could be stale? | |
| 20:35:26 | mriedem | well my initial concern was just why get_provider_traits wasn't using the cache if set_traits_for_provider relies on the cache | |
| 20:35:41 | mriedem | which if i use the cache, just becomes an optimization | |
| 20:35:54 | mriedem | i.e. later adding a use_cache kwarg to get_provider_traits or whatever | |
| 20:37:24 | mriedem | looks like only fill_provider_mapping uses get_provider_traits today which was the stuff gibi added and isn't in compute, so i guess that's why it doesn't use the cache | |
| 20:37:59 | efried | Yes, and _refresh_associations, which is the low-level thing that updates the cache, but it refreshes a bunch of other shit you don't need as well. | |
| 20:38:41 | mriedem | there might have been some other reason i needed to hit placement directly, but it's been too long since i wrote this so i guess i'll just rebase and then try to see if things still work after re-using the same report client in the virtapi | |
| 20:39:16 | mriedem | might have been when the virt driver calls the virtapi, but the virtapi in the virt driver is the same virtapi that ComputeManager creates and passes a reference of itself | |
| 20:41:07 | efried | yeah at some point we made sure the report client was a singleton per process | |
| 20:43:49 | efried | mriedem: I think we want to tweak the report client to make this process more generic & reusable. Stand by. | |
| 20:45:42 | efried | Pull this bit of _refresh_associations | |
| 20:45:42 | efried | https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/client/report.py#L794-L803 | |
| 20:45:42 | efried | into get_provider_traits itself -- iow calling get_provider_traits should also update the cache | |
| 20:46:10 | efried | (...and dedup that part of _refresh_associations) | |
| 20:46:50 | efried | So then your thingy, and other thingies that do similar, can | |
| 20:46:50 | efried | get_provider_traits() to retrieve the trait info you want to muck with | |
| 20:46:50 | efried | muck with it | |
| 20:46:50 | efried | set_traits_for_provider() with the changed trait list | |
| 20:47:37 | efried | If we wanted to optimize that, we could add a refresh=$bool kwarg to get_provider_traits so you could optionally have it *just* return whatever's in the cache rather than refetching. | |
| 20:47:39 | mriedem | we can't have https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/client/report.py#L802 in get_provider_traits for things that aren't using the cache, like gibi's calls from conductor for fill_provider_mapping | |
| 20:47:40 | mriedem | right? | |
| 20:48:05 | efried | I don't see why not. | |
| 20:48:30 | efried | We'd be updating the cache, but not reading from it anywhere. | |
| 20:48:42 | efried | Updating the cache is of negligible cost | |
| 20:48:46 | mriedem | " # NOTE(efried): This will blow up if called for a RP that doesn't # exist in our _provider_tree. " | |
| 20:48:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add placement request filter for disabled computes https://review.opendev.org/654596 | |
| 20:50:13 | efried | mriedem: by that point in _refresh_associations, we're already assured the provider is in our cache. | |
| 20:50:28 | efried | you're saying if it's called from elsewhere where that's not the case | |