| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-03 | |||
| 19:44:00 | mriedem | dansmith: what do you think a safe batch size is for this heal allocations CLI? at first i default to CONF.api.max_limit but that's 1000 which seems way too big https://review.openstack.org/#/c/565886/5/nova/cmd/manage.py@1776 - map_instances defaults to 50 | |
| 19:44:05 | mriedem | so was thinking about using 50 | |
| 19:44:21 | dansmith | yeah 1000 is too much | |
| 19:44:24 | dansmith | 50 is probably good | |
| 19:44:25 | mriedem | online_data_migrations also does 50 | |
| 19:44:32 | dansmith | yup | |
| 19:44:54 | melwitt | did we do an audit of other uses of safe_connect where None can be returned? | |
| 19:45:11 | mriedem | melwitt: jaypipes has been working on untangling that | |
| 19:45:18 | melwitt | k, cool | |
| 19:59:36 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Fix wrong arguments for 'detach_volume' https://review.openstack.org/566152 | |
| 20:06:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-manage placement heal_allocations CLI https://review.openstack.org/565886 | |
| 20:11:38 | mriedem | owalsh: if you want to do the backport to queens and pike for https://review.openstack.org/#/c/566096/ that would speed things along so i can +2 the backports | |
| 20:11:52 | mriedem | owalsh: beware: the provider tree stuff in rocky will likely mean merge conflicts for the backports | |
| 20:16:59 | arvindn05 | dansmith: looks like there was agreement on the rebuild instance with traits thread. Can you send out an update on ML on the final approach? | |
| 20:17:03 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in virt/test_block_device.py https://review.openstack.org/566153 | |
| 20:17:43 | dansmith | arvindn05: I started it a bit ago but got distracted.. if there is agreement you're not blocked right? | |
| 20:23:15 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Create volume attachment during boot from volume in compute https://review.openstack.org/541420 | |
| 20:24:17 | arvindn05 | dansmith: i am blocked at this point because my approved patch https://review.openstack.org/#/c/560596/ is also holding based on the decision on the rebuild issue | |
| 20:25:15 | arvindn05 | my next patches would be update the spec with decision on rebuild and propose the code patch for the same | |
| 20:25:24 | dansmith | arvindn05: you mean you're blocked because your patches don't do what mriedem wants yeah? | |
| 20:25:48 | dansmith | arvindn05: if you'd do what we said in the meeting this morning, then he'd remove his -W and everything would move along, AFAICT | |
| 20:26:53 | arvindn05 | dansmith: yup. but i was unfortunately not in the meeting and not entirely sure what approach was decided | |
| 20:27:12 | dansmith | if only there was a log... | |
| 20:27:34 | arvindn05 | dansmith: is it fair to summarize it as we want to go check the allocations route | |
| 20:27:50 | arvindn05 | (8:25:17 AM) efried: arvindn05: That was the impression I got. But yeah, let's see what dansmith has to say. | |
| 20:27:59 | dansmith | arvindn05: yes | |
| 20:28:39 | arvindn05 | dansmith: great. Thanks for confirming....i will start with the spec and the code patch. glad the deadlock was resolved :) | |
| 20:28:44 | efried | ++ | |
| 20:30:02 | mriedem | arvindn05: fyi http://eavesdrop.openstack.org/meetings/nova/2018/nova.2018-05-03-14.00.log.html#l-161 | |
| 20:31:05 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix being able to hard reboot a pausing instance https://review.openstack.org/566143 | |
| 20:31:57 | owalsh | mriedem: sure | |
| 20:32:20 | mriedem | thanks, hopefully it's not too bad, the patch is pretty isolated | |
| 20:32:40 | arvindn05 | mriedem: thanks got that from gibi as well :) last statement was <dansmith> I shall commentificate upon the threadage and reviewage so wanted to confirm :) | |
| 20:34:57 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Cleanup placement policy generator docs https://review.openstack.org/565225 | |
| 20:41:26 | arvindn05 | dansmith: one other clarification, we want to validate the allocations vs traits always correct? Even when we are rebuilding with the same image and no traits have changed | |
| 20:48:55 | melwitt | I would think so -- while the image hasn't changed, the underlying resource provider traits could have | |
| 20:50:12 | mriedem | arvindn05: that gets tricky depending on where you do the validation, | |
| 20:50:31 | mriedem | if the image doesn't change, i'm not sure if you get to the point in conductor where we'd be calling the scheduler | |
| 20:50:47 | mriedem | i realze you're not calling the scheduler to do the validation, but i assumed it would be in the same block | |
| 20:51:03 | arvindn05 | mriedem: i am thinking it would be in the conductor | |
| 20:51:32 | arvindn05 | conductor already has the placement client and all its dependencies need to make the validation | |
| 20:52:13 | arvindn05 | conductor.manager.ComputeTaskManager#rebuild_instance somewhere in here is where the validation logic would lie | |
| 20:55:06 | mriedem | arvindn05: you'd have an else block here https://github.com/openstack/nova/blob/0ef3c685b9d2e0049f38fcf1a268870e69a5b9cf/nova/conductor/manager.py#L944 when recreate is False | |
| 20:55:07 | arvindn05 | in case of rebuild we would add logic here https://github.com/openstack/nova/blob/master/nova/conductor/manager.py#L897 | |
| 20:56:26 | arvindn05 | mriedem: sorry...looking at your code pointer now | |
| 20:56:59 | mriedem | melwitt: so i'm looking at https://review.openstack.org/#/c/540258/ and something is bugging me, | |
| 20:57:13 | mriedem | with an affinity group policy, the group members would all have to be in the same cell because they have to be on the same host | |
| 20:57:25 | mriedem | also soft-affinity might screw with that... | |
| 20:57:42 | mriedem | for an anti-affinity group, we could have instances in different cells i'd thinkg | |
| 20:57:43 | mriedem | *think | |
| 20:58:09 | melwitt | yeah, I think so | |
| 20:58:35 | melwitt | are you suggesting that if policy == affinity then limit to same cell? | |
| 20:58:37 | mriedem | so if i'm doing a move operation on an instance in an anti-affinity group, we have a targeted context for the cell that instance being moved is in, | |
| 20:58:50 | arvindn05 | mriedem: from the comment in line 916 that whole block executes only in case we are doing a rebuild with a new image.... | |
| 20:59:08 | mriedem | and we'll only get group hosts in that cell to make sure the instance doesn't go to a host that another member of the same group in the same cell is in, since we can't move across cells | |
| 20:59:17 | melwitt | right | |
| 20:59:19 | arvindn05 | if we want to run the validation always...we would have to do outside the block, correct? | |
| 20:59:19 | mriedem | which is correct | |
| 21:00:13 | mriedem | arvindn05: well, rebuild + new image or evacuate | |
| 21:00:18 | mriedem | evacuate is a rebuild on another new host | |
| 21:00:20 | mriedem | but same image | |
| 21:00:35 | mriedem | in that case, the scheduler will go through GET /allocation_candidates and do a claim, | |
| 21:00:47 | mriedem | so we don't need to do any image validation for traits in conductor for evacuate b/c that would be redundant | |
| 21:01:39 | arvindn05 | yup...i got that for evacuate...how about the case of rebuild without the image being changed | |
| 21:01:59 | mriedem | melwitt: ok and if we're not targeted (not a move, just instance create), then we need to get group hosts from all cells because we don't know in which cell the instance is going to land in | |
| 21:02:10 | melwitt | mriedem: right | |
| 21:02:52 | mriedem | and this happens before we ever run the affinity/anti-affinity filters right? | |
| 21:03:26 | mriedem | yeah called from schedule_and_build_instances | |
| 21:03:33 | melwitt | yes, this is the setup actually pretty early on in compute/api, I think | |
| 21:03:51 | melwitt | okay maybe not (sorry I forgot) | |
| 21:04:04 | mriedem | ok yeah so if i'm creating an instance an in anti-affinity group then the other members of the group could be in any cell and i need to iterate all cells to find which hosts those other members are in, | |
| 21:04:18 | melwitt | setup_instance_group is called in some places and that's what does it (if group.hosts is not yet populated) | |
| 21:04:26 | melwitt | it will do that the first time it's called in a code path | |
| 21:04:50 | mriedem | and if i'm creating an instance in an affinity group, if that group already has one member in it, we should know exactly which cell the new instance and new member of that group should get scheduled to | |
| 21:05:12 | mriedem | so in that case, we wouldn't really need to scatter/gather all cells, | |
| 21:05:22 | mriedem | i don't think... | |
| 21:05:56 | melwitt | hm, well, I don't know actually. affinity just means once the cell is selected, affine with an instance in the group if there is one. I think that's how it would behave | |
| 21:06:15 | melwitt | or does it actually choose the cell based on where the members are? I would have doubted that | |
| 21:06:53 | melwitt | you could have group members in different cells even with the affinity policy, I think | |
| 21:07:05 | melwitt | like, they affine with each other on a per cell basis | |
| 21:07:48 | melwitt | and if you're the first group member to land in a cell, you can build anywhere. it's just after that, new members have to affine if they land in that cell | |
| 21:09:30 | openstackgerrit | Oliver Walsh proposed openstack/nova stable/queens: Handle @safe_connect returns None side effect in _ensure_resource_provider https://review.openstack.org/566161 | |
| 21:10:48 | mriedem | i left some comments, | |
| 21:11:04 | mriedem | but the strict affinity policy should filter such that members of the same group are on the same host | |
| 21:11:09 | mriedem | which implies same cell | |
| 21:11:49 | mriedem | if you built 2 instances in concurrent requests to the same strict affinity group, we don't guarantee that those end up on the same host via the scheduler b/c it doesn't have that information; that's why we have the late affinity check in the compute, | |
| 21:11:50 | mriedem | HOWEVER, | |
| 21:12:25 | mriedem | with multiple cells, if instance A goes on host 1 in cell1 and instance B goes on host 2 in cell2, i'm not sure the late affinity check will catch that | |
| 21:12:37 | melwitt | yeah, that's a good question. I don't think this has been brought up before. I hadn't been thinking to target an instance to a cell where its affinity member lives | |
| 21:12:45 | mriedem | although it should.....the late check will ask the instance group for hosts that the other members are in, and if they are in another cell, it will fail | |
| 21:13:16 | mriedem | https://github.com/openstack/nova/blob/0ef3c685b9d2e0049f38fcf1a268870e69a5b9cf/nova/compute/manager.py#L1287 | |
| 21:13:55 | mriedem | yeah so i think we'll fail the late affinity check, and reschedule, however, we'll never find a host to fit that policy b/c it's going to be in the wrong cell at that point | |
| 21:14:20 | melwitt | yeah ... I think my patch makes it so that check would do a "all cells" query and maybe it shouldn't ... getting confused | |
| 21:14:20 | mriedem | anti-affinity is much easier, different host in same cell, or different cell, doesn't matter | |
| 21:14:36 | mriedem | well, | |
| 21:14:43 | mriedem | i think there are two cases here: | |
| 21:14:46 | mriedem | for affintiy | |
| 21:15:10 | mriedem | 1. a group member is already in a host (so a cell) and your lookup finds it, we know our new instance has to go in that cell to affine to the existing member | |
| 21:15:28 | mriedem | worst case there is we're needlessly iterating multiple cells when we already know which one we want | |
| 21:15:40 | mriedem | so, perf optimization for later maybe, | |