Earlier  
Posted Nick Remark
#openstack-nova - 2018-07-10
15:25:37 mriedem sure, but when the bug is fixed and we're properly pulling group members, it would start to race
15:26:04 mriedem i'd been meaning to write a functional test for https://review.openstack.org/#/c/540258 but need to look at my notes - it's been awhile
15:26:33 mriedem "We need a new bug and test for the multi-cell affinity scenario where two instances are scheduled at the same time in the same affinity group. We need 2 cells with 1 host each, with exactly enough capacity to fit just one instance so that placement will fail the first request and throw it into the other host in the other cell. The late affinity check in the compute won't fail because it can't see the other member in the
15:26:33 mriedem oh i guess i was going to write a new bug for something that came up during review:
15:26:34 mriedem her cell, so it will think it's fine."
15:27:28 mriedem the tricky part there is our functional tests are able to make up-calls from nova-compute
15:27:34 mriedem we don't have a split mq in our tests
15:27:38 mriedem not sure how easy that would be to do
15:27:40 mriedem dansmith: ^ ?
15:28:33 mriedem well, maybe won't need split mq - just the instances being in different hosts in different cell dbs might do it
15:28:34 dansmith ....yeah
15:28:53 dansmith with the workaround flag you can disable the check right?
15:29:09 mriedem yes
15:29:54 dansmith although I guess that doesn't really do much,
15:29:58 gibi mriedem: do I understand correclty that on master server groups only work if the late check using upcall is enabled?
15:30:00 dansmith obviously if you disable that check it's not going to check
15:30:02 mriedem maybe that type of request wouldn't get past scheduling
15:30:03 openstackgerrit Matthew Booth proposed openstack/nova master: Remove irrelevant comment https://review.openstack.org/578821
15:30:04 openstackgerrit Matthew Booth proposed openstack/nova master: Move static _get_power_off_values to compute_utils https://review.openstack.org/578822
15:30:30 mriedem gibi: i don't think so...
15:30:34 mriedem tempest has an anti-affinity test
15:30:50 gibi mriedem: that is the test I changed to show that master is broken
15:30:50 dansmith mriedem: so, scheduler will look at group.members, get the instance.host for each, and affine to those hosts, right?
15:31:02 mriedem gibi: test_create_server_with_scheduler_hint_group_anti_affinity
15:31:03 dansmith which is lossy because those instances might be pre-scheduling
15:31:07 dansmith or rather, pre-build
15:31:24 openstackgerrit Chen proposed openstack/nova master: WIP https://review.openstack.org/581403
15:32:06 gibi mriedem: that tempest test only works because it sends two server create in the same API request
15:32:17 mriedem right, because servers in separate requests is a known race
15:32:34 gibi mriedem: if you wait 10 minutes between the two requests it still doesnt work
15:32:37 gibi mriedem: I tried :)
15:32:46 dansmith gibi: really? why is that?
15:33:19 dansmith the filter is using a potentially stale version of server group, from the beginning of the request, I just noticed
15:33:22 mriedem yeah the HostState.instances dict should be populated per request
15:33:29 dansmith that shouldn't be a problem 10 minutes later though
15:33:36 gibi dansmith: this returns an empty list for the second scheduling https://github.com/openstack/nova/blob/c0350da4a1607d7aa113caceaefb5d29303c7eed/nova/objects/instance_group.py#L422
15:33:51 mdbooth Just working on this patch: https://review.openstack.org/#/c/578846/ I called it out in the commit message: the cut/paste in there is horrible, but avoids a refactor. Am I going to get that landed as is, or do I need to look at refactoring the driver interface?
15:34:01 dansmith gibi: oh on multi-cell?
15:34:01 mriedem gibi: and i think that's the bug melwitt is fixing
15:34:05 dansmith that needs to stripe
15:34:06 dansmith yeah
15:34:24 gibi don't we deploy with multi cell _by default_ ?
15:34:39 mdbooth It goes without saying that I don't mind refactoring :) However, I'd also like to both land and backport the code.
15:34:55 mriedem gibi: in devstack yes
15:34:58 dansmith gibi: yeah, this would hit cell0 if your default is cell0, and always be empty
15:35:14 mriedem it's not really 'multi cell' so much as superconductor mode in devstack and how the services are configured
15:35:30 gibi let me rephrase. I our suggested deployment mode (cell_v2) the server group policies are broken
15:35:41 gibi that is my understanding
15:35:50 dansmith gibi: because your default connection is to cell0, which will never return the list of instances, yeah
15:36:03 gibi dansmith: thanks, that explains what I see
15:36:13 dansmith but striping across cells is what needs to happen, which is that bug afaik
15:36:48 gibi dansmith: yeah. this is why offered my help to melwitt above on the fix
15:36:59 dansmith gibi: are we in violent agreement? :)
15:37:06 gibi dansmith: yes
15:37:48 gibi I was not sure we are on the same page about the size of the problem. server groups are useless now
15:38:17 dansmith gibi: if your controllers are pointing at your main cell by default it'll be fine, fwiw
15:38:24 dansmith but it definitely needs changing
15:38:51 mriedem gibi: so, if you want to help, we need a functional test in-tree that recreates the regression
15:39:03 mriedem that's what i was -1 on the change for - the mock based unit tests had logic flaws in them
15:39:54 gibi dansmith: how can a deployer change the default cell to cell1?
15:40:08 gibi mriedem: ack. I offered a tempest test :)
15:40:10 dansmith gibi: [database]/connection :)
15:41:05 gibi dansmith: in the super conductor conductor conf?
15:41:17 dansmith gibi: in any controller config
15:41:33 dansmith gibi: you don't have to be running a superconductor layout with only one non-cell0 cell
15:41:58 mriedem gibi: yeah, but we can't use that tempest change long-term because of the otherwise known race issue
15:42:28 gibi dansmith: I see
15:42:32 gibi dansmith: thanks
15:43:18 gibi mriedem: is it racy even if the test waits for the first server to go to ACTIVE state before start creating the second one?
15:43:49 mriedem gibi: in that case maybe not
15:43:59 mriedem because per scheduling request, we should pull the set of instances per host
15:44:28 mriedem https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L781
15:44:49 mriedem that's if the compute isn't sending it's instance info to the scheduler (which it's not in default devstack b/c we disable that ability)
15:45:38 gibi OK, so the second scheduling will see the first instance sitting on the host
15:45:41 mriedem which reminds me of https://review.openstack.org/#/c/569247/ but haven't had good large scale performance testing to see if that's justified
15:45:47 mriedem gibi: it should yeah
15:46:09 gibi mriedem: then I don't see how the currently proposed tempest test be racy
15:47:28 openstackgerrit Jay Pipes proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139
15:49:03 mriedem gibi: ok updated comments
15:50:15 gibi mriedem: ack, I can cover both single and multi request cases in tempest
15:50:49 gibi mriedem: if we have the tempest do you still require the functional test in melwitt's patch?
15:51:53 mriedem maybe not
15:52:06 mriedem does your tempest test also cover affinity?
15:52:42 mriedem looks like it doesn't for separate requests
15:52:44 gibi mriedem: both the affinity and the anti-affinity case fails now
15:53:00 gibi mriedem: both uses the same helper to create the servers
15:53:50 gibi mriedem: but in case of affinity we might want to fill the compute to see NoValid host
15:54:04 mriedem that's what i was going to try with a functional test
15:54:19 mriedem to set available vcpu inventory to 1 or something
15:54:34 gibi mriedem: sure we have plenty of such functional tests already but they are not cell aware and passing
15:55:08 gibi mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py
15:55:31 openstackgerrit Matt Riedemann proposed openstack/nova master: Document differences and similaries between extra specs and hints https://review.openstack.org/581410
15:55:34 mriedem right, i plan on writing a multi-cell affinity regression functional test
15:56:18 gibi mriedem: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py#L292
15:57:27 gibi mriedem: ohh there is one case where functional is needed. multiple non-cell0 cells case
15:57:43 gibi mriedem: as in tempest we have only cell0 and cell1
15:58:02 mriedem yes, that's what i've been saying :)
15:58:14 mriedem when i say 'multi-cell' i mean multiple non-cell0 cells
15:58:15 gibi mriedem: now I understand ;)
15:58:28 mriedem i don't consider cell0 a 'real' cell since it doesn't contain compute hosts

Earlier   Later