Earlier  
Posted Nick Remark
#openstack-nova - 2018-07-17
19:05:13 mriedem the scheduler processes the batch as one request,
19:05:21 mriedem and as a host is selected per instance,
19:05:46 mriedem the scheduler 'consumers' resources for the in-memory representation of that host,
19:05:51 mriedem then filters the hosts for the next instance in the list
19:06:44 mriedem which is why in my change here https://review.openstack.org/#/c/582976/ i'm mapping the instance to the selected host in its HostState.instances dict
19:07:00 mriedem which is used in the anti-affinity filter to determine which members of the group on a given host
19:07:04 sean-k-mooney right so for the second time true the anti afinity filter the instances = set(host_state.instances.keys()) line retrives the updated inmemory copy of the instance state not the current state
19:07:08 mriedem https://github.com/openstack/nova/blob/master/nova/scheduler/filters/affinity_filter.py#L101
19:07:23 mriedem that's the idea
19:10:01 sean-k-mooney hum ya reading the code of that change it makes sense in that context.
19:14:22 mriedem i think this is the killer https://github.com/openstack/nova/blob/master/nova/scheduler/filters/affinity_filter.py#L98
19:15:46 mriedem when we're doing multi-create, we have 1 request spec per instance, so in this failing test we have 2
19:15:47 sean-k-mooney checking if the instance uuid is already in the current host.
19:16:01 mriedem but the conductor just randomly picks the first request spec from the list when passing it to the scheduler,
19:16:07 mriedem and that request_spec has a random instance_uuid on it
19:16:33 mriedem so if i've got hosts A and B and instances X and Y, and using reqspec with instance X,
19:16:44 mriedem then go through host filtering and select host A for instance X,
19:17:03 mriedem when i go through host filtering for instance Y and host A, it will say: if spec_obj.instance_uuid in host_state.instances.keys():
19:17:05 sean-k-mooney oh realy should we not be looping over the requests specs in the multicreate request
19:17:15 mriedem if X in A.instances(): return True
19:17:35 mriedem the problem is when this code was added https://github.com/openstack/nova/blob/master/nova/scheduler/filters/affinity_filter.py#L98 it assumes resize, which is a single instance,
19:17:57 mriedem and it worked because before https://review.openstack.org/#/c/571166/ the filter didn't look at host_state.instances within a multi-create create
19:18:08 mriedem s/look at/depend on/
19:19:02 mriedem i'm going to see if i can make this a predictable order through the scheduler to tickle the failure
19:23:46 sean-k-mooney ya so that looks wrong https://github.com/openstack/nova/blob/039f7e055ee2b47e96be4e86d6f48d9ce469c123/nova/conductor/manager.py#L1191-L1194
19:24:05 sean-k-mooney its using the request spec from the first instance only
19:24:22 mriedem well, it's not wrong, it's just very brittle assumptions in the anti-affinity filter that didn't used to be a problem
19:24:38 sean-k-mooney fair point :)
19:27:47 sean-k-mooney i guess you do want to use only 1 request spec so that state is preseved across instances
19:28:22 mriedem well, things have just sort of been hacked in over time
19:28:38 mriedem reqspecs are 1:1 with instances,
19:28:50 mriedem but we're scheduling for a list of instances with the reqspec from the first one,
19:29:01 mriedem so any filter relying on reqspec.instance_uuid is out of context
19:29:07 mriedem for a multi-create scenario anyway
19:30:36 sean-k-mooney i wonder are there other edgecase there also. such as multi attach volumes that this would cause issues for
19:31:45 mriedem we currently don't support multi-create bfv with the same multiattach volume
19:31:52 mriedem nor do we do any filtering on volumes
19:32:16 mriedem but that's #1 here https://specs.openstack.org/openstack/nova-specs/specs/rocky/approved/volume-multiattach-enhancements.html#problem-description
19:33:50 sean-k-mooney mriedem: ok just wondering if we will find more edgecase like this if we keep using the same request spec for all instance in the list
19:34:20 sean-k-mooney i could not think of one off the top of my head but multiattached volumes was the close thing that came to mind
19:35:07 openstackgerrit Merged openstack/nova master: Merge server create for scheduler hint extension https://review.openstack.org/579067
19:35:27 openstackgerrit Merged openstack/nova master: Fix "XLibvirt KVM (ppc64)" typo in feature support matrix docs https://review.openstack.org/583267
19:41:38 sean-k-mooney mriedem: we have the list of instance_uuids up untill we call self._get_sorted_hosts() here https://github.com/openstack/nova/blob/1f0e2bef296d860e6875aa148d3a86629d872256/nova/scheduler/filter_scheduler.py#L190
19:42:10 sean-k-mooney mriedem: could we pass in the current instance uuid and feed it true the schduler
19:43:36 mriedem maybe,
19:43:40 sean-k-mooney we would basically have to set the sepc_obj.instance_uuid to instance_uuids[num]
19:43:45 mriedem i was just going to add a spec_obj.num_instances == 1 check to https://github.com/openstack/nova/blob/master/nova/scheduler/filters/affinity_filter.py#L98,
19:44:07 mriedem but, num_instances is persisted across the request spec so it would be re-used on a resize to same host, which would break that logic,
19:44:30 mriedem num_instances should probably *not* be persisted since it's per-operatoin
19:45:45 sean-k-mooney ya i was going to say if we did that it would break on a rezie correct because it could resize to the same host wich is presumable whtat that check is ment to prevent
19:46:14 mriedem right that's what it was added for https://review.openstack.org/#/c/299045/
19:47:36 mriedem we do some things like reset force_hosts/force_nodes in the request spec during a move operation so it doesn't screw with the filtering
19:47:56 mriedem https://github.com/openstack/nova/blob/master/nova/conductor/tasks/migrate.py#L184
19:50:23 mriedem i remember making a similar type of fix for that recently but can't find the patch
19:52:10 mriedem oh it was something to do with failed hosts in the requestspec.retry
19:52:25 mriedem would prevent you from being able to later live migrate the instance to those hosts
19:52:58 mriedem https://review.openstack.org/#/c/559447/
19:55:05 mriedem num_instances is a bit different in that it's set in the api, persisted to the group in the api, and then conductor reads the group back out of the db and puts it in the request spec sent to the scheduler
19:55:20 mriedem we could just set num_instances in conductor since we know how many instances we're creating at that point
19:55:22 openstackgerrit sean mooney proposed openstack/nova master: random hacking for bug 1781710 https://review.openstack.org/583347
19:55:24 openstack bug 1781710 in OpenStack Compute (nova) "ServersOnMultiNodesTest.test_create_server_with_scheduler_hint_group_anti_affinity failing with "Servers are on the same host"" [High,Fix released] https://launchpad.net/bugs/1781710 - Assigned to Matt Riedemann (mriedem)
19:55:38 sean-k-mooney mriedem: does ^ make any sense
19:56:17 mriedem yeah but could make that cleaner with enumerate and resetting the instance_uuid field on the request spec so it's not persisted
19:56:31 mriedem i also don't know how to recreate this bug to show this fixes it
19:57:30 sean-k-mooney what about setting the allow server per host to 2 and only have one host?
19:57:38 sean-k-mooney not sure that would work actully...
19:59:17 openstackgerrit Merged openstack/nova master: Add unshelve instance error info to fault table https://review.openstack.org/579747
20:02:47 sean-k-mooney anyway its time for me to log off for the day. ill try and think of a way to recreate tomorow.
20:03:44 mriedem ok i'll clean up this patch and add a test
20:03:56 mriedem but it'll just be unit test since i can't recreate with functional
20:20:51 openstackgerrit Matt Riedemann proposed openstack/nova master: Additional/alternative fix for bug 1781710 https://review.openstack.org/583351
20:20:52 openstack bug 1781710 in OpenStack Compute (nova) "ServersOnMultiNodesTest.test_create_server_with_scheduler_hint_group_anti_affinity failing with "Servers are on the same host"" [High,Fix released] https://launchpad.net/bugs/1781710 - Assigned to Matt Riedemann (mriedem)
20:33:08 melwitt mriedem: ack
21:29:43 openstackgerrit Merged openstack/nova master: Call generate_image_url only for legacy notification https://review.openstack.org/564528
21:58:34 openstackgerrit Matt Riedemann proposed openstack/nova master: Update RequestSpec.instance_uuid during scheduling https://review.openstack.org/583347
21:58:35 mriedem sean-k-mooney: updated ^ with a giant commit message because this is hairy as hell
22:02:56 mriedem yikun: looks like we have a few more places in nova that are using InstanceGroup.policies
22:02:58 mriedem nova/scheduler/weights/affinity.py: policies = request_spec.instance_group.policies
22:02:58 mriedem nova/scheduler/utils.py: request_spec.instance_group.policies = group_info.policies
22:02:58 mriedem osboxes@osboxes:~/git/nova$ grep -R spec.instance_group.policies
22:25:19 mriedem stephenfin: check out this fun oslo.config output https://docs.openstack.org/nova/latest/configuration/config.html#upgrade-levels
22:27:10 mriedem the group help is dumped multiple times for lots of groups https://docs.openstack.org/nova/latest/configuration/config.html#service-user
22:28:26 sean-k-mooney mriedem: that a slightly more comprehsive commit message then i was expecting.
22:32:32 openstack Launchpad bug 1782253 in oslo.config "Group help text is output multiple times during sphinx docs generation" [Undecided,New]
22:32:32 mriedem stephenfin: https://bugs.launchpad.net/oslo.config/+bug/1782253
22:32:49 mriedem sean-k-mooney: it's a shitty bug so there is lots of context to reason through it
22:33:48 sean-k-mooney that was not a complaint its well worded.
22:36:01 sean-k-mooney i also feel like thre should be some function in base nova/oslo object that combines an assignment to x with the reset
22:36:09 sean-k-mooney so that we can reduce
22:37:22 sean-k-mooney the spec_obj.instance_uuid = x followed by spec_obj.obj_reset_changes([instance_uuid]) call into one function
22:38:30 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Compute: Handle reshaped provider trees https://review.openstack.org/576236
22:39:43 sean-k-mooney that or some kind of context manager. the whole spec_obj.obj_reset_changes pattern feels like c memory manament with having to call free.
22:56:05 mriedem i thought about using nova.utils.temporary_mutation but i don't think that will work properly with the reset
22:56:24 mriedem but agree we could do something like temporary_mutation in ovo
#openstack-nova - 2018-07-18
00:20:09 openstackgerrit Takashi NATSUME proposed openstack/nova master: Transform metrics.update notification https://review.openstack.org/580567
00:50:19 openstackgerrit Brin Zhang proposed openstack/nova stable/queens: Add unshelve instance error info to fault table https://review.openstack.org/583418
01:16:34 openstackgerrit Brin Zhang proposed openstack/nova stable/ocata: Add unshelve instance error info to fault table https://review.openstack.org/583422
01:35:39 openstackgerrit Brin Zhang proposed openstack/nova stable/pike: Add unshelve instance error info to fault table https://review.openstack.org/583426
01:47:01 openstackgerrit Zhenyu Zheng proposed openstack/nova master: Wrong href link returned when providing non-existed version in GET version API https://review.openstack.org/486850
02:26:57 openstackgerrit Yikun Jiang (Kero) proposed openstack/nova master: Change deprecated policies to policy https://review.openstack.org/583434

Earlier   Later