Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-06-24
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
20:50:35 efried so yeah, trap and ignore ValueError
20:50:37 efried done
20:50:55 openstackgerrit Merged openstack/python-novaclient master: Revert "Add irrelevant files in dsvm job" https://review.opendev.org/667151
20:51:26 mriedem blech
20:51:35 efried you could do that from within the new-and-improved get_provider_traits
20:51:37 mriedem but yeah that's what i meant - called from conductor where the rp isn't in the cache
20:51:39 efried or check the cache first.
20:52:13 mriedem i'm going to avoid all of that for now because then you could have questions like, "should the caller say if they want the cache and should fail if the rp isn't in the cache?"
20:52:15 efried if self._provider_tree.exists(rp_uuid):
20:52:37 efried "avoid all of that for now" is going to be harder and introduce more technical debt.
20:52:39 mriedem sure - but if i'm compute and expect it in the cache and it's not, we'd want that to fail, not be swallowed
20:53:05 mriedem that's why i said a use_cache kwarg on get_provider_traits seems easier
20:53:07 mriedem the caller says what it wants
20:53:43 efried but you need two knobs
20:53:45 mriedem the very first thing i want to do is just split this large-ish change up into several pieces so i can test in isolation and reason about it
20:53:55 efried 1) retrieve from the cache
20:53:55 efried 2) update the cache
20:54:02 efried one does not imply the other
20:54:18 mriedem ...
20:54:38 mriedem meaning refresh if it's not in the cache and return the cached results,
20:54:53 mriedem or don't get from the cache, get from the API, but update the cache...
20:54:53 mriedem ?
20:55:51 mriedem i'm just going to work on splitting this all up and put a TODO in my code that uses get_provider_traits to somehow leverage the cache - it can be debated in review at that point i think when i actually have something working end to end (which the change is now - the func test shows that, but i'm going to be splitting up and possibly breaking things)
20:56:10 efried okay
20:57:07 spatel sean-k-mooney: hey
20:58:27 efried nts: http://specs.openstack.org/openstack/nova-specs/specs/train/approved/pre-filter-disabled-computes.html
21:08:00 mriedem heh well i've got a chicken-and-egg so i likely can't use the cache anyway
21:08:27 mriedem rt init needs the driver https://github.com/openstack/nova/blob/914ee91a5caedcaba64472762c9f08d7bb644d11/nova/compute/manager.py#L538
21:08:37 mriedem and the driver init needs the virtapi https://github.com/openstack/nova/blob/914ee91a5caedcaba64472762c9f08d7bb644d11/nova/compute/manager.py#L535
21:09:52 mriedem unless i do something hacky like self.virtapi.reportclient = self.reportclient....
21:14:22 efried that's what we've done all over the place, innit?
21:14:47 efried I guess mostly the other way around, self.reportclient = self.otherthingy.reportclient
21:15:09 efried you could pass the reportclient into the virtapi constructor
21:16:09 mriedem yeah i'm constructing the singleton reportclient in ComputeManager and passing that to RT and ComputeVirtAPI
21:16:36 efried ++
21:42:47 tonyb sean-k-mooney: We needed to do it in requirements in order to *generate* constraints
21:45:04 tonyb sean-k-mooney: You say it's breaking the requirements-check job but I don't see that
21:45:37 tonyb sean-k-mooney: Given our, lack, of TZ overlap might be best to do this via email?
21:45:53 sean-k-mooney im here
21:46:14 sean-k-mooney we do not generate constratis to project anymore
21:46:23 sean-k-mooney at least the lower constratits
21:47:21 sean-k-mooney the reason i say i breaks the requirement check is https://review.opendev.org/#/c/666651/2/doc/requirements.txt
21:47:56 sean-k-mooney simply addint a comment or otherwise modifying the requiremetns file cause the requirements-check job to fail
21:48:42 sean-k-mooney we can see the error here
21:48:45 sean-k-mooney http://logs.openstack.org/51/666651/2/check/requirements-check/c8c82c5/job-output.txt.gz#_2019-06-20_17_39_08_901350
21:48:49 sean-k-mooney tonyb: ^
21:48:52 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Add placement request filter for disabled computes https://review.opendev.org/654596
21:49:09 tonyb sean-k-mooney: okay so there are a couple of things
21:49:53 tonyb sean-k-mooney: the requirements team needs to generate constraints (upper) daily and without capping sphinx (and others) in g-r we can't do that
21:50:32 sean-k-mooney ah so when you said generate you ment upper-constraitns not requirement files for projects
21:50:48 tonyb sean-k-mooney: and also the 'fix' to that error is to just merge the chnage that caps sphinx, which is the expected process albeit a little ugly now that we don't auto-sync
21:51:08 tonyb sean-k-mooney: correct
21:51:30 sean-k-mooney yes we have a patch for that but my understdaint is project are not ment to cap any package in ther own reqiuremets file
21:51:57 sean-k-mooney the can balcklis tthing the know do not work but if cap we can break compatiobliy for the intergrated services
21:52:07 tonyb sean-k-mooney: In general we try very hard not to cap in g-r or per project but it isn't "forbidden"
21:52:25 tonyb sean-k-mooney: and with this whole no more py2 support thing we don't have a choice
21:52:53 sean-k-mooney well that is due to useing gloabl-requiremtns for two thing
21:53:13 sean-k-mooney generatihing upper-constratints and capping python2 things
21:53:36 tonyb sean-k-mooney: and listing "approved libraries" so that's 3 things
21:53:42 sean-k-mooney well no it because we use it for generating uppercontratitgs
21:53:48 sean-k-mooney ya
21:54:14 sean-k-mooney so the last point is what i thought its priamary pourpous was
21:54:35 sean-k-mooney listing the appoved libs that wee know are compatiable and are manged by the requiremetns project
21:54:51 tonyb sean-k-mooney: I'm interested in your statement "if cap we can break compatiobliy for the intergrated services" can you elaborate?
21:55:33 sean-k-mooney os-vif say mock < X some other poject requires mock > x
21:55:48 sean-k-mooney os-vif and that service can nolonger be installed together
21:56:10 tonyb sean-k-mooney: ahhh okay .... that's handled in the generate-constraints job so it *shouldn't* be possible
21:56:34 tonyb sean-k-mooney: for *upper* constraints for lower all bets are off

Earlier   Later