| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-03 | |||
| 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 | mriedem | which is correct | |
| 20:59:19 | arvindn05 | if we want to run the validation always...we would have to do outside the block, 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 | mriedem | anti-affinity is much easier, different host in same cell, or different cell, doesn't matter | |
| 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: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, | |
| 21:16:13 | mriedem | 2. we're creating multiple instances concurrently where there are no existing members in the affinity group - iterating the cells is fine but we can still have the scheduler put the members in the wrong cells | |
| 21:16:29 | mriedem | i.e. it's the same affinity race for concurrent creates that we've always had, | |
| 21:16:34 | mriedem | but the late check in the compute can't fix that one | |
| 21:16:48 | mriedem | because we don't reschedule across cells, only within the same cell we're already in | |
| 21:16:50 | melwitt | I see, yeah | |
| 21:17:52 | melwitt | I think we have to check all cells though right? we have to pull instances to find out what host they're on | |
| 21:17:59 | mriedem | so in case 2 you get a novalidhost / maxretriesexceeded and have to delete and recreate the instance, which at that point should work | |
| 21:18:28 | openstackgerrit | Oliver Walsh proposed openstack/nova stable/pike: Handle @safe_connect returns None side effect in _ensure_resource_provider https://review.openstack.org/566164 | |
| 21:18:30 | melwitt | that is, I don't think there's a way to avoid having to look in all cells unless you mean do it iteratively instead of a scatter-gather and stop early when found | |
| 21:18:43 | mriedem | melwitt: yeah i think the latter, | |
| 21:18:44 | openstackgerrit | karim proposed openstack/nova master: Pushing image traits to ironic node https://review.openstack.org/565620 | |
| 21:18:47 | melwitt | gotcha | |
| 21:19:17 | mriedem | like, if policy == 'affinity' lookup the instance mappings for the group members, and they should all be in the same cell, so just get hosts from that one | |
| 21:19:42 | mriedem | or just get one instance mapping for one member in the affinity group and pick it's host, because the new instance *has* to go on that same host | |
| 21:19:53 | mriedem | anyway, like i said, perf optimization for later if we care | |
| 21:20:12 | mriedem | i just wanted to make sure i was thinking through this correctly | |
| 21:21:46 | melwitt | oh, I see what you mean now, get the group members and choose a cell based on that | |
| 21:21:51 | openstackgerrit | Jay Pipes proposed openstack/nova master: rework how we pass candidate request information https://review.openstack.org/566166 | |
| 21:22:35 | melwitt | yeah, I think the only potential wrinkle there is, that would mean affinity always means one cell forever | |
| 21:23:47 | melwitt | and I'm not 100% sure that's how it's supposed to work considering cells are transparent, that is, if you imagine you have some application that will run faster if its instances are affined to a host, does that mean you want your application to only be able to run in one cell? or do you want it to run in multiple cells where each cell will have its instances running on the same host | |
| 21:24:20 | dansmith | if you want the latter you need two groups regardless of cells right? | |
| 21:24:23 | melwitt | and have multiple instances of your application running from multiple cells? maybe that's not a thing that makes sense but just for the sake of thinking about it | |
| 21:24:31 | dansmith | if you asked for host affinity, then "same cell" is kinda implied no? | |
| 21:24:42 | melwitt | maybe, I'm not that much of an affinity expert | |
| 21:24:52 | dansmith | affinity means "same host" currently | |
| 21:24:53 | melwitt | if it is, then that's cool. I just didn't know | |
| 21:24:59 | dansmith | anti-affinity means "any other host" | |
| 21:25:04 | dansmith | or rather "not the same host" | |
| 21:25:56 | melwitt | right, okay. that makes it clear then. I don't know why I was thinking about multiple per cell affinity | |
| 21:32:08 | mriedem | looking at https://github.com/openstack/nova/blob/0ef3c685b9d2e0049f38fcf1a268870e69a5b9cf/nova/compute/manager.py#L1289 again, | |
| 21:32:26 | mriedem | i think it's possible that you could have 2 affinity group members in different cells and we wouldn't fail | |
| 21:33:10 | mriedem | because if the group has instA and instB in cell1 and cell2 respectively, group_hosts = group.get_hosts(exclude=[instance.uuid]) for instA won't return the host for instB because it's in another cell, and we're not targeted for that other cell | |
| 21:33:20 | mriedem | so group_hosts would be empty and we'll say all is clear | |
| 21:34:29 | melwitt | yeah ... you're talking about the current state of affinity with multiple cells? | |
| 21:34:43 | mriedem | yeah regardless of this change | |
| 21:34:58 | melwitt | yeah, what I found is that since we're not targeted, we just get an empty hosts list every time | |
| 21:35:28 | mriedem | hmm, is the group targeted when we get down to compute? https://github.com/openstack/nova/blob/0ef3c685b9d2e0049f38fcf1a268870e69a5b9cf/nova/compute/manager.py#L1289 | |
| 21:36:25 | melwitt | no because the group is pulled early before we target anything. I hope I wrote comments in the patch about that, I'm having trouble remembering it even now | |
| 21:37:06 | melwitt | oh, that in compute manager, that will be local to a cell | |
| 21:37:06 | dansmith | that'd be a bug, of course, | |
| 21:37:36 | dansmith | if two instances in an affinity group were in different cells | |
| 21:37:48 | dansmith | which maybe could have happened if we did something dumb in scheduler | |
| 21:37:54 | dansmith | (maybe related to melwitt's patch) | |
| 21:38:03 | dansmith | but that would be, like, bad, not a legit arrangement | |
| 21:38:12 | melwitt | yeah. well, it definitely happens today because the query for hosts will always be empty | |
| 21:38:16 | mriedem | dansmith: yeah it's totally a bug but i think it's something that could happen today with multiple cells and concurrent requests to create instances in the same affinity group | |
| 21:38:25 | dansmith | ack, okay | |
| 21:39:11 | melwitt | it doesn't have to be concurrent because no matter what, when you go to boot an instance and say "give me the hosts that are in this group" it will hand you an empty list because it's an untargeted DB access by nova-api | |