| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-27 | |||
| 18:02:54 | dansmith | first hit on google.. looks gross though because it has sweet potato in it | |
| 18:03:01 | sdague | heh | |
| 18:03:09 | sdague | oh, so you don't actually eat that :) | |
| 18:03:27 | dansmith | sweet potato is gross | |
| 18:03:41 | dansmith | regular potato == perfect | |
| 18:05:54 | vdrok | thanks for the help with multicell, all jobs green :) | |
| 18:08:15 | dansmith | woot | |
| 18:09:01 | mriedem | final novaclient release is up https://review.openstack.org/487966 | |
| 18:10:59 | mriedem | onto the claims in the scheduler patch, | |
| 18:11:08 | mriedem | i see the move accounting happening on that change in the multinode patch | |
| 18:11:08 | mriedem | http://logs.openstack.org/66/483566/20/check/gate-tempest-dsvm-neutron-multinode-full-ubuntu-xenial-nv/374e3c3/logs/screen-n-sch.txt.gz#_Jul_27_14_36_56_138843 | |
| 18:11:22 | mriedem | New allocation request containing both source and destination hosts in move operation: {'allocations': [{'resource_provider': {'uuid': u'209a32d3-f240-4bcc-9d9d-8ae371b97d42'}, 'resources': {u'VCPU': 1, u'MEMORY_MB': 64}}, {u'resource_provider': {u'uuid': u'fdba3ea4-883a-4dc4-a2d6-49d723f9559e'}, u'resources': {u'VCPU': 1, u'MEMORY_MB': 64}}]} | |
| 18:11:56 | mriedem | this one is a resize New allocation request containing both source and destination hosts in move operation: {'allocations': [{'resource_provider': {'uuid': u'209a32d3-f240-4bcc-9d9d-8ae371b97d42'}, 'resources': {u'VCPU': 1, u'MEMORY_MB': 64}}, {u'resource_provider': {u'uuid': u'fdba3ea4-883a-4dc4-a2d6-49d723f9559e'}, u'resources': {u'VCPU': 1, u'MEMORY_MB': 64}}]} | |
| 18:11:58 | mriedem | oops | |
| 18:12:02 | mriedem | http://logs.openstack.org/66/483566/20/check/gate-tempest-dsvm-neutron-multinode-full-ubuntu-xenial-nv/374e3c3/logs/screen-n-sch.txt.gz#_Jul_27_14_48_57_853377 | |
| 18:12:10 | mriedem | New allocation request containing both source and destination hosts in move operation: {'allocations': [{'resource_provider': {'uuid': u'fdba3ea4-883a-4dc4-a2d6-49d723f9559e'}, 'resources': {u'VCPU': 1, u'MEMORY_MB': 64}}, {u'resource_provider': {u'uuid': u'209a32d3-f240-4bcc-9d9d-8ae371b97d42'}, u'resources': {u'VCPU': 1, u'MEMORY_MB': 128}}]} | |
| 18:12:16 | mriedem | memory bumps up | |
| 18:12:23 | mriedem | so that all seems cool | |
| 18:13:11 | mriedem | i don't expect anything to be busted with soft delete, since with soft delete we do'nt delete the instance until it's reclaimed | |
| 18:13:17 | mriedem | so the allocations shouldn't change until that happens | |
| 18:13:19 | cdent | gibi’s test suggeests that cleanups are not happening | |
| 18:13:35 | cdent | i’m experimenting with them now to see if I can see anything wrong/weird | |
| 18:15:20 | mriedem | i do see the source node cleaning up allocations during live migration | |
| 18:15:20 | mriedem | http://logs.openstack.org/66/483566/20/check/gate-tempest-dsvm-neutron-multinode-full-ubuntu-xenial-nv/374e3c3/logs/screen-n-cpu.txt.gz#_Jul_27_14_35_26_077233 | |
| 18:15:49 | mriedem | jaypipes: dansmith: do we need to worry about this? https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1068 | |
| 18:16:03 | mriedem | wiping out all of the allocations for an instance because it's no longer on the source node | |
| 18:16:33 | dansmith | hmm, I thought not because of the claim at the end on the destination, but let me look | |
| 18:17:16 | dansmith | mriedem: yeah, we should check the allocations before we delete to see if we own any of them I think, or delete the ones that pertain to us | |
| 18:17:19 | dansmith | instead of just nuking them all | |
| 18:17:23 | dansmith | good call | |
| 18:20:16 | mriedem | it also seems that https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1068 could simply updated to be: | |
| 18:20:25 | mriedem | if not instance.host or instance.host != CONF.host: | |
| 18:20:28 | mriedem | continue | |
| 18:21:24 | jaypipes | err, not sure about that... | |
| 18:21:39 | mriedem | otherwise yeah, pass the cn uuid to https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L999 and we could make sure to only remove allocations for the source node + instance | |
| 18:22:07 | mriedem | note that if we have to change anything here in the compute, it wouldn't be there for ocata computes | |
| 18:22:15 | jaypipes | mriedem: an allocation is an all-or-none thing, though. | |
| 18:22:31 | mriedem | jaypipes: i don't know what that means | |
| 18:22:47 | mriedem | we're doubling up allocations here https://review.openstack.org/#/c/487589/6/nova/scheduler/client/report.py | |
| 18:22:54 | mriedem | to maintain the source node allocations | |
| 18:23:07 | mriedem | but https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1068 will clearly blast those away | |
| 18:23:08 | jaypipes | mriedem: You can't delete "part of an allocation". | |
| 18:23:19 | mriedem | why not? we amended part of an allocation | |
| 18:23:27 | mriedem | here https://review.openstack.org/#/c/487589/6/nova/scheduler/client/report.py | |
| 18:23:28 | jaypipes | no, we replaced it. | |
| 18:23:38 | mriedem | so we patched something in, we can't patch something out? | |
| 18:23:43 | jaypipes | PUT /allocations overwrites. | |
| 18:23:50 | mriedem | yes i know | |
| 18:23:51 | jaypipes | mriedem: hold up. | |
| 18:23:53 | mriedem | i'm saying, | |
| 18:24:01 | mriedem | we have to do the same thing for https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1068 | |
| 18:24:07 | mriedem | to remove the allocations for the instance on the source node | |
| 18:24:14 | mriedem | but leave the allocations for the instance on the dest node | |
| 18:24:25 | mriedem | is it hangout time? | |
| 18:25:00 | dansmith | we either have to not delete, | |
| 18:25:05 | jaypipes | mriedem: the dest host will end up writing the allocation entirely (only including the allocated resources on the dest host) when the move operation completes successfully. | |
| 18:25:08 | dansmith | or put the allocation with our part removed | |
| 18:25:19 | dansmith | jaypipes: right but the source will then delete it without checking it | |
| 18:25:32 | dansmith | and I think those two things probably race with each other | |
| 18:25:44 | jaypipes | mriedem: so I think what we need to do is just ensure _update_usage_from_instances() does not call that _remove_deleted_instances_allocations() for instances currently in a move operation | |
| 18:26:10 | jaypipes | dansmith: understood. we just need to ensure we don't call that delete for any instances in a move operation | |
| 18:26:11 | dansmith | jaypipes: or instances that are just finishing a move operation that it thinks have been deleted | |
| 18:26:26 | dansmith | jaypipes: I don't think you can know that it's in a move operation if you're late to the party | |
| 18:26:43 | mriedem | dansmith: the instance would have a migration_context? | |
| 18:26:53 | bauzas | mriedem: back there | |
| 18:27:01 | jaypipes | guys, won't https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1060 skip instances that are currently being mnoved? | |
| 18:27:01 | openstackgerrit | Merged openstack/python-novaclient master: Allow tuple as for nics value https://review.openstack.org/475816 | |
| 18:27:02 | dansmith | mriedem: not if you'reon the late end of the race | |
| 18:27:15 | dansmith | currently being moved is not the problem | |
| 18:27:23 | dansmith | "just moved a half second ago" is the problem right? | |
| 18:27:35 | jaypipes | but instance.host will be not None. | |
| 18:27:48 | jaypipes | the only time instance.host is None is when the instance is deleted. | |
| 18:28:38 | dansmith | jaypipes: actually not | |
| 18:28:50 | dansmith | jaypipes: that's reversed.. None means "not yet booted".. it's still $host after you delete | |
| 18:28:55 | dansmith | it's skipping not yet booted instances | |
| 18:29:09 | dansmith | if the hostname doesn't match you'll continue on to delete it because it's not on our host right? | |
| 18:29:17 | jaypipes | ah, shit. | |
| 18:29:21 | dansmith | since this logic was all based on just accounting for the local host, I don't think it quite works | |
| 18:29:22 | dansmith | plus, | |
| 18:29:32 | dansmith | there is a race between fetchign the instance and its host flipping to the other side | |
| 18:30:09 | mriedem | right the not instance.host is for scheduling instances the first time | |
| 18:30:11 | mriedem | per the comment | |
| 18:30:17 | mriedem | "# Allocations related to instances being scheduled should not be # deleted if we already wrote the allocation previously." | |
| 18:30:22 | dansmith | yeah | |
| 18:30:27 | dansmith | this was the "leafe race" | |
| 18:30:28 | mriedem | "already wrote the allocation previously" == in the scheduler | |
| 18:30:32 | mriedem | right | |
| 18:30:58 | jaypipes | but allocations_to_delete won't contain migrating instances. Because migrating instances are in RT.tracked_instances, no? | |
| 18:31:16 | dansmith | jaypipes: not at the instance after we complete the migration right? | |
| 18:31:47 | mriedem | that i don't know | |
| 18:31:49 | mriedem | b/c it's f'ed | |
| 18:31:50 | dansmith | jaypipes: either way, this is not a big deal: | |
| 18:32:10 | dansmith | just pull the allocation like we do, and if we're not in it, we don't touch it | |
| 18:32:28 | dansmith | and maybe something else about if there are two in there, then don't delete either, although not sure if that's important or not | |
| 18:32:43 | jaypipes | dansmith: sure, that's a fairly safe thing. | |
| 18:33:30 | jaypipes | ok, guys, lemme update the move patch. | |
| 18:33:31 | mriedem | just looking at where this starts from, the periodic is in here https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L667 | |
| 18:33:40 | mriedem | jaypipes: just do it on top of that one since it's in the gate | |
| 18:33:49 | mriedem | we get the instances for the host and node (source) | |
| 18:34:09 | mriedem | clear out all tracked instances https://github.com/openstack/nova/blob/master/nova/compute/resource_tracker.py#L1030 | |