Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-11-26
21:35:23 mriedem bnemec: have you ever heard of requests for something like a PostitiveIntOpt or PositiveFloatOpt in oslo.config? we have some options which can be set to 0.0 as the min, but really shouldn't be <= 0.
21:35:32 mriedem but we can't really describe that with just min
21:37:27 dansmith choices=range(1,1000) ? :P
21:37:36 bnemec mriedem: So an opt where min is a < comparison instead of a <=?
21:38:21 mriedem something like that
21:38:28 mriedem for context https://review.openstack.org/#/c/602804/9/nova/conf/compute.py
21:38:36 mriedem initial_cpu_allocation_ratio should never be 0.0
21:38:39 dansmith we really just want a validation function parameter, right?
21:38:43 mriedem yeah
21:38:44 dansmith we wanted that for something else recently
21:38:53 dansmith validator=lambda str: ...
21:38:53 openstackgerrit Mohammed Naser proposed openstack/nova master: Drop cruft code for all_tenants behaviour https://review.openstack.org/620165
21:41:19 bnemec A validator callback seems like something we could do.
21:41:32 bnemec Alternatively, in this case min=0.000001 is probably also sane.
21:46:16 bnemec You could also create a custom type that did the validation in the constructor.
21:46:44 bnemec Subclass Float and put whatever logic you need in there: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L409
21:47:14 mriedem I wasn't sure how kosher subclassing oslo.config opt types was
21:47:16 bnemec Then create the opt as Opt(type=MyCustomType, ...).
21:47:55 bnemec They're part of the public API so I'd say they're fair game.
21:49:20 mriedem ok yeah that's probably cleanest
21:49:25 bnemec They danger might be creating a completely new class as a type, which could theoretically be done, but if we ever added to the type API you might get broken.
21:49:32 bnemec As long as you inherit from an existing type you should be okay though.
21:49:57 bnemec They all descend from a single ABC.
21:50:53 mriedem the adam of config opts?
21:51:14 bnemec Indeed.
21:51:30 bnemec Also, I think I was wrong. You want to do validation in __call__.
21:51:40 bnemec That's where we're doing it in the existing types: https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L830
21:52:19 mriedem ah yeah https://github.com/openstack/oslo.config/blob/master/oslo_config/types.py#L305
21:52:51 bnemec Yeah, better example. :-)
22:03:19 mriedem welp https://review.openstack.org/#/c/613126/ kind of blows up https://specs.openstack.org/openstack/nova-specs/specs/stein/approved/initial-allocation-ratios.html#manually-set-placement-allocation-ratios-are-overwritten
22:03:23 mriedem how am i not surprised
22:05:21 mriedem efried: you'll have some context on ^
22:06:52 mriedem i'm not exactly sure what we should do about it, outside of passing an initial flag to update_provider_tree or something gross like that...although maybe upt can check the provider tree to see if it already has inventory with allocation_ratio set, and if so, don't provide a value unless CONF.*_allocation_ratio is not None
22:09:27 openstackgerrit Artom Lifshitz proposed openstack/nova master: Give drop_move_claim() correct docstring https://review.openstack.org/620170
22:09:43 artom mriedem, ^^ related to the numa live migration spec. I *think* I'm right, and it might clarify the confusion around where we're removing usages.
22:10:31 mriedem artom: totally forgot you pinged me earlier, sec
22:10:52 mriedem artom: superconductor does not reschedule if live migration fails, no
22:10:59 mriedem it reschedules if the pre-checks on the dest/source fail
22:11:34 artom Right, sorry, I wasn't being precise enough. Though I can't find it rescheduling *anywhere*
22:11:45 mriedem so here conductor asks the scheduler for a host https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L370
22:12:01 mriedem in this try block it does the pre-checks for dest/source https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L385
22:12:15 mriedem if that fails, blacklist the host https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L391
22:12:16 artom *facepalm*
22:12:21 artom Got it, sorry.
22:12:28 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L366
22:12:28 mriedem set host=None and hit the while again
22:12:38 mriedem np
22:12:42 artom My brain skipped over _find_destination() as an atomic thing
22:13:32 artom Thanks for taking the time to walk me through this :)
22:14:00 artom Now that comment patch, if you have time ;) The patch itself is trivial, but requires thinking to make sure I got it right.
22:14:07 openstackgerrit Merged openstack/nova master: Add debug logs when doubling-up allocations during scheduling https://review.openstack.org/617016
22:14:14 openstackgerrit Merged openstack/nova stable/rocky: Default embedded instance.flavor.is_public attribute https://review.openstack.org/619349
22:16:45 openstackgerrit Artom Lifshitz proposed openstack/nova master: Give drop_move_claim() correct docstring https://review.openstack.org/620170
22:31:14 mriedem artom: ok some random comments inline
22:31:40 mriedem i guess drop_move_claim called from the source on confirm_resize always confuses me,
22:31:47 mriedem because the claim is made on the dest during prep_resize
22:31:52 efried mriedem: Sorry, I'm back now. We're trying to figure out how to know whether allocation ratios were defaulted by placement or conf or nova?
22:32:57 mriedem efried: yeah - need to determine when initial_*_allocation_ratio config should be used (first time the compute was created) and report that to placement, or if we should set allocation ratios because CONF.*_allocation_ratio is some non-default value, or if we should leave it alone because the admin set some allocation_ratio via the placement API directly
22:33:24 mriedem i think we can know the initial case from upt if the provider tree doesn't have those inventory resource class keys in it
22:33:44 mriedem otherwise only ever overwrite if CONF.*_allocation_ratio is not None
22:34:05 efried "if the provider tree doesn't have those inventory resource class keys" <== I don't think this can happen within upt
22:34:30 mriedem on initial startup, wont upt just have a root provider with no inventory on it?
22:34:35 mriedem the driver reports the inventory initially
22:34:41 efried I don't think so, because of the (legacy) code that's bootstrapping the CPU/MEMORY_MB/DISK_GB inventories before we get to upt
22:35:03 mriedem i'm not sure what you're referring to
22:35:06 efried I could be wrong, it's possible we've removed that at this point.
22:35:35 efried but there was a time when code *outside* of virt driver space would do the initial populate of placement
22:35:52 mriedem by calling driver.get_inventory() right?
22:36:04 mriedem that no longer happens if the driver implements upt
22:36:05 efried get_available_resource
22:36:14 efried which *does* still get called I believe.
22:36:14 mriedem yeah, get_available_resource was before get_inventory
22:36:33 mriedem i think it's upt -> get_inventory -> get_available_resource
22:36:37 efried gar doesn't get called in the rt update flow, but it still gets called somewhere else, lemme find it.
22:37:03 mriedem https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L711
22:37:50 mriedem we'd create the provider root initially here https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L921
22:38:05 mriedem then flush inventory from upt here https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L944
22:38:13 mriedem else this https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L951
22:38:20 mriedem failing those, this https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/compute/resource_tracker.py#L961
22:38:23 mriedem which would use the values from gar
22:38:44 mriedem https://github.com/openstack/nova/blob/c1de096098344c733565c244163fc3ebf8c35e68/nova/scheduler/client/report.py#L1442
22:39:26 mriedem might be interesting to drop that final path,
22:39:39 mriedem looks like maybe only the hyperv driver hasn't implemented an alternative
22:40:11 mriedem oh and zvm
22:40:15 mriedem forgot that was in tree...
22:40:52 efried yeah, I remember looking at this when I was working on https://review.openstack.org/#/c/615705/ to see if I could just real quick implement upt for all the drivers.
22:41:11 efried and realizing that was going to be more work than I was ready to undertake at the time.
22:41:50 efried but unwinding the gar stuff, that's going to require some synapses I haven't yet explored.
22:42:01 mriedem heh, good thing you noted that because i was just thinking about removing that old code path to see what would break
22:42:33 mriedem i haven't seen hyper-v run on the latest version of that, but zvm failed
22:42:35 efried any case, I think the point is that we can't rely on upt being in the code path for initial population of the root provider inventories.
22:42:59 efried yet
22:43:05 mriedem no it doesn't need to be though
22:43:21 efried if we want upt to be able to decide whether to use initial_*_allocation_ratio it does.
22:43:22 mriedem as of https://review.openstack.org/#/c/613126/, if a driver implements upt, it sets the allocation_ratio on the inventory it reports,
22:43:28 mriedem otherwise that normalize method in the RT does
22:43:31 mriedem _normalize_inventory_from_cn_obj
22:43:52 efried right, it sets the allocation ratio based on the non-initial_* conf values.
22:43:53 mriedem i think upt can determine if initi allocation ratios can be used though
22:43:57 efried how?

Earlier   Later