| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-14 | |||
| 17:12:28 | jaypipes | cdent: done | |
| 17:12:36 | cdent | thanks | |
| 17:17:08 | cfriesen | cdent: jaypipes: I'm nervous about defaulting to multiple workers...seems like a good way to hit races even with placement | |
| 17:17:46 | cdent | cfriesen: a) how?, b) that's why we're merging early | |
| 17:19:01 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Add 'numa-aware-vswitches' spec https://review.openstack.org/541290 | |
| 17:19:03 | cfriesen | cdent: numa resources aren't allocated until you actually hit the compute node, so can fail late. Also, do we model server group policies in placement currently? | |
| 17:19:56 | cfriesen | cdent: in order to fix server group policy stuff we had to serialize scheduling of instances in the same server group, otherwise we hit races even with just a single scheduler worker. | |
| 17:21:10 | jaypipes | cfriesen: no, we do not model server group policies in placement. | |
| 17:21:56 | cfriesen | cdent: jaypipes: the problem with server groups is that the group membership isn't updated until the instance actually hits the node, so there's a big window from when the scheduler made the decision until the membership changes | |
| 17:22:02 | cfriesen | (in the DB) | |
| 17:22:35 | cfriesen | sorry, not group membership but the list of compute nodes being used by the group members | |
| 17:22:53 | jaypipes | cfriesen: yes, that's group membership. | |
| 17:23:33 | jaypipes | cfriesen: so because of a crappy affinity implementation and crappy numa resource tracking, we'll continue to slow down the rest of the world... | |
| 17:24:31 | jaypipes | cfriesen: can't we tell folks using that functionality to run a single worker? | |
| 17:24:46 | sean-k-mooney | cfriesen: defulting to muliple works has other issue. like we used to hit the db max conncetion limits because several service in openstack defualted to use multiple works for things and defulted to 1 worker per cpu. | |
| 17:24:47 | cfriesen | jaypipes: just pointing out that this could cause unexpected races if it's enabled by default. We might want to put something in the release notes. | |
| 17:25:05 | cdent | also, if the aforementioned serialization is already present, will it help? | |
| 17:25:28 | jaypipes | sean-k-mooney: that's unrelated. | |
| 17:25:40 | cfriesen | cdent: I don't think that serialization is upstream yet. How would we serialize across multiple workers? | |
| 17:25:51 | sean-k-mooney | jaypipes: proably i am just skimming the scoll back now | |
| 17:26:00 | cdent | cfriesen: ah, sorry, I had misunderstood which "we" you meant :) | |
| 17:26:07 | sean-k-mooney | i was in meeting all day up until this point | |
| 17:27:02 | jaypipes | cfriesen: a single nova boot request is only handled by a single worker, no? the case you're worrying about is multiple clients calling nova boot with the same server group. | |
| 17:27:44 | jaypipes | cfriesen: as for the NUMA issues, I don't think that changing to multiple workers will make a difference in the number of retries that happen. | |
| 17:27:59 | sean-k-mooney | jaypipes: or perhaps the nova multi boot support we you say boot x instance of y flavour on network z ? | |
| 17:28:22 | jaypipes | sean-k-mooney: I don't understand how that's relevant? | |
| 17:28:39 | jaypipes | sean-k-mooney: that would be handled in a single thread. | |
| 17:29:04 | jaypipes | sean-k-mooney: the only situation cfriesen is worried about is when multiple nova boot requests involving the same server group were executed simultaneously. | |
| 17:29:12 | sean-k-mooney | jaypipes: i was wondering if that was a single boot request form the api point of view or x independet ones and a client feature | |
| 17:29:35 | jaypipes | sean-k-mooney: a single nova boot is a single thread of execution. | |
| 17:30:47 | sean-k-mooney | jaypipes: ya that makes sense. and nova boot --min 3 --max 3 --server-group... is considered a singel boot request | |
| 17:31:32 | sean-k-mooney | jaypipes: so the only race would be if two client tried to boot servers in the same group concureently without the server group already having running instances | |
| 17:32:17 | jaypipes | sean-k-mooney: yes, that is considered a single boot request. | |
| 17:33:51 | jaypipes | sean-k-mooney: almost. the only race is two clients concurrently attempting to add instances to the same server group (regardless of whether the server group has members). and that is a situation I find pretty rare and not worth shooting the rest of the world in the foot for. | |
| 17:35:26 | cfriesen | jaypipes: the scenario I'm worried about is where two instances race to schedule and get put onto the same compute node, but then one of them claims a mix of resources that causes the other to fail it's resource allocation. With a single sched worker this is less likely (though still possible). | |
| 17:35:57 | jaypipes | cfriesen: unless those are NUMA resources, it's not possible to do that. | |
| 17:36:01 | mriedem | arvindn05: ok what's up | |
| 17:36:21 | cfriesen | jaypipes: yes, numa resources. CPUs, hugepages, PCI devices with strict affinity | |
| 17:37:22 | mriedem | arvindn05: oh i'll read your replies | |
| 17:37:37 | cfriesen | I'm totally fine with the change to enable multiple workers, I just thing we might want to mention in the release notes that it could increase races in some cases. | |
| 17:39:17 | bauzas | jaypipes: cfriesen: I'm totally up for deprecating https://developer.openstack.org/api-ref/compute/#create-multiple-servers y'know | |
| 17:39:28 | melwitt | mriedem: on this bp, when you say you'd like to see the proposed code for deleting a server, did you mean you want to wait for proposed code before approving the bp? https://blueprints.launchpad.net/nova/+spec/add-server-group-remove-member-notifications | |
| 17:39:31 | bauzas | it creates more problems than it solves | |
| 17:39:56 | cfriesen | jaypipes: for server groups there's also the case where you resize/migrate/evacuate instances in a server group at the same time, or at the same time a new instance is created. I don't think we have late validation for all those scenarios. | |
| 17:40:11 | bauzas | and when you say --min 1 --max 5, you'd expect that if you have room for 1, then only one would be created and 4 in ERROR, but that's not the case | |
| 17:40:13 | cfriesen | bauzas: me too. :) but last time I brought it up people wanted to keep it. | |
| 17:40:27 | mriedem | melwitt: kind of yeah | |
| 17:40:29 | bauzas | because min/max are only relevant wrt quotas | |
| 17:40:34 | cfriesen | bauzas: actually I'd expect that 1 would be created and the other 4 would be like they never existed | |
| 17:40:42 | arvindn05 | mriedem: thanks....any questions for me? | |
| 17:40:46 | bauzas | cfriesen: that's absolutely not the current behaviour | |
| 17:40:54 | melwitt | mriedem: okay, I'm cool with that. just wanted to make sure I understood | |
| 17:41:00 | cfriesen | bauzas: I know, but it should be given how it's documented in the API | |
| 17:41:01 | mriedem | arvindn05: not yet, still reading | |
| 17:41:17 | bauzas | the current behaviour is that if you asked for 10 instances but your quota only tells 2 left, then it will just check that | |
| 17:41:25 | arvindn05 | mriedem: great....just let me know when you have questions. | |
| 17:41:27 | bauzas | but honestly, who cares ? | |
| 17:41:34 | mriedem | bauzas: heh "Error handling for multiple create is not as consistent as for single server create, and there is no guarantee that all the servers will be built. This call should generally be avoided in favor of clients doing direct individual server creates." | |
| 17:41:39 | mriedem | so actually, | |
| 17:41:49 | mriedem | concurrent creates is actually a problem for the affinity scenarios | |
| 17:41:58 | mriedem | but not a single multi-create request | |
| 17:42:02 | mriedem | damned if you do, damned if yo udon't | |
| 17:42:28 | bauzas | I'm just saying that the "minimum" value is useless | |
| 17:42:46 | bauzas | you ask for a max of 10, the scheduler will try to find you room for 10, or kick all of them | |
| 17:42:53 | mriedem | maybe we should amend that to say, "unless of course you're creating servers in a server group, then because of race issues we have'nt fixed yet, you should use multi-create" | |
| 17:43:04 | cfriesen | bauzas: and it shouldn't kick all of them if some of them pass | |
| 17:43:21 | bauzas | cfriesen: I'm not sure | |
| 17:43:29 | bauzas | cfriesen: it's a capacity problem | |
| 17:43:37 | cfriesen | bauzas: the user has said "give me as many instances as you can, up to a max of 10" | |
| 17:43:39 | bauzas | and we shouldn't leak the capacity to the user | |
| 17:43:42 | mriedem | how is the min useless? give me at least 2, but 10 if you can | |
| 17:43:55 | bauzas | mriedem: the "if you can" is only quota-wise | |
| 17:44:04 | cfriesen | bauzas: no, it's in the boot request | |
| 17:44:31 | bauzas | cfriesen: but the info we give to the scheduler is "how many the user asked" | |
| 17:44:45 | bauzas | cfriesen: not "how many the user is agreeing to only have" | |
| 17:44:55 | mriedem | also depends on the port quota, don't forget | |
| 17:45:04 | bauzas | yeah | |
| 17:45:05 | cfriesen | bauzas: I totally agree it's all messed up. but the API describes it as the min/max nubmer of servers to be created | |
| 17:45:15 | bauzas | the API docs are wrong | |
| 17:45:28 | cfriesen | the API docs are by definition correct, the implementation is wrong. :) | |
| 17:45:49 | cfriesen | the user wants at least 2 and at most 10 instances. If the quota is 4, we should give them 4 | |
| 17:45:54 | bauzas | that's an interesting PoV | |
| 17:45:56 | cfriesen | If we can only schedule 3, we should give them 3 | |
| 17:46:08 | mriedem | i'm pretty sure we've said in the past that the docs don't define the api | |
| 17:46:11 | dansmith | cfriesen: I think the actual behavior, which people may have built dependencies on, is the canonical thing | |
| 17:46:13 | mriedem | the current behavior defines the api | |
| 17:46:16 | dansmith | right | |
| 17:46:16 | mriedem | right | |
| 17:46:18 | mriedem | right | |
| 17:46:21 | dansmith | right | |
| 17:46:22 | melwitt | right | |
| 17:46:23 | mriedem | right | |
| 17:46:24 | bauzas | right | |
| 17:46:26 | bauzas | right | |
| 17:46:28 | bauzas | right | |
| 17:46:42 | dansmith | (man we're dorks) | |
| 17:46:43 | mriedem | remember the docs of yore for the compute api were written by people that didn't actually code up nova api | |
| 17:46:57 | melwitt | I think we've said in the past that it makes more sense for the --min to be honored if we can but if we change that, we have to do it with a microversion | |
| 17:46:58 | mriedem | and until sdague lifted them in tree they were all sorts of wrong | |
| 17:47:02 | mriedem | and still are in parts | |