Earlier  
Posted Nick Remark
#openstack-nova - 2018-07-10
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
16:00:59 gibi mriedem: OK, I will extend the tempest test tomorrow. Thanks for the explanation
16:05:41 openstackgerrit Eric Fried proposed openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139
16:06:28 openstackgerrit karim proposed openstack/nova master: Handle rebuild of instances with image traits https://review.openstack.org/569498
16:13:50 openstack Launchpad bug 1739482 in Cinder "test_snapshot_backup fails to build backup due to MessagingTimeout" [Medium,Confirmed]
16:13:50 mriedem dansmith: i found another candidate for your long_rpc_timeout https://bugs.launchpad.net/cinder/+bug/1739482
16:14:13 dansmith mriedem: that's on the cinder side though yeah?
16:14:16 mriedem yup
16:14:21 mriedem but i'd do the exact same thing there
16:14:29 mriedem ala oslo.incubator style
16:14:59 dansmith aye
16:19:24 mriedem dansmith: so in https://review.openstack.org/#/c/563375/35/nova/objects/instance_group.py it seems weird that _rules is nullable but we default to {} if it's not set in the db

Earlier   Later