Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-07-14
14:47:57 mriedem can't we figure out if it's a move by checking if there are already existing allocations for that consumer?
14:48:12 mriedem or does that tread on retry territory
14:48:38 leakypipes mriedem: yes, but that would mean another set of HTTP communications for each scheduler request.
14:48:51 leakypipes mriedem: and we are already calling GET /allocation_candidates once per request.
14:48:53 mriedem yeah, so let's not do that then
14:49:11 mriedem if reqspec.movify:
14:49:12 figleaf leakypipes: true, but it might not mean dirtying as much code now
14:49:17 melwitt superdan: do you know if instance.old_flavor is a thing I can rely on to be there for an instance that's in VERIFY_RESIZE state? resized but not confirmed nor reverted?
14:49:27 leakypipes mriedem: especially since it's easy enough for the nova-api or conductor to add a field to the RequestSpec saying it's a move operation.
14:49:32 mriedem figleaf: it does mean a perf hit though
14:49:38 mriedem i'd rather pass a flag than take a perf hit
14:49:42 mriedem for ALL requests
14:49:43 superdan melwitt: yep, should be find if it's in any resize-related state I think
14:49:49 melwitt yesss
14:49:56 figleaf mriedem: yes, but small, but it will be smartified later!
14:50:13 mriedem figleaf: how do you know it would be small?
14:50:28 leakypipes mriedem: but do we have all that information during scheduling? I don't think the old flavor stuff is in the RequestSpec is it?
14:50:35 figleaf mriedem: GET /allocations is pretty lightweight
14:50:43 mriedem multicreate request with 100 instances for a new boot, would hit all of these 'do i already have allocations' requests for nothing
14:51:06 mriedem leakypipes: you'd put it on the request spec in conductor i thought
14:51:09 mriedem like you just said
14:51:17 mriedem migrate/resize/rebuild all go through conductor
14:51:23 mriedem with a reqspec
14:51:29 leakypipes mriedem: ah, sorry, I thought you were saying we should query that information for the instance within the scheduler.
14:51:45 mriedem leakypipes: that was an idea to avoid a flag being passed in on the reqsspec
14:51:51 mriedem but as noted it's extra overhead for perf
14:51:55 leakypipes right
14:51:57 mriedem which i don't tink we want long-term
14:52:20 leakypipes mriedem, figleaf: a simple flag "is_move" would be cool with me. I think that's actually pretty useful anyway.
14:52:32 mriedem seems that needs to be there anyway, long-term
14:52:41 mriedem if you don't want to make these GET /allocatoins calls in the shceduler to tell if you're moving
14:52:53 figleaf leakypipes: agreed
14:53:00 mriedem i don't care how lightweight GET /allocations is, we should avoid anything unnecessary within the scheduler as possible
14:53:11 leakypipes mriedem: FYI, since we don't support multiple instances doing resize/move, we'd never do >1 GET /allocations in the scheduler because we'd never call it if num_instances > 1.
14:53:20 mriedem we've already had reports of perf regression in server create times since mitaka
14:53:35 mriedem leakypipes: i meant on server create
14:53:41 mriedem b/c you'd need to tell if you're doing a server create or move right?
14:53:48 mriedem or you mean, we just check that once and know, regardless of number?
14:53:57 mriedem oh i see what you're saying
14:53:59 mriedem yeah nvm
14:54:13 leakypipes mriedem: no, I mean if num_instances > 1 then we'd have no need to call GET /allocations/{instance_uuid}
14:54:15 mriedem well you'd still do the possibly unnecessary check in the scheduler for a single instance create
14:54:21 leakypipes correct.
15:05:16 mriedem gdi, the versioned notification functional tests seem to be going nutty
15:06:57 melwitt yeah they are
15:09:27 gibi mriedem: do you have a link to the notification test issue?
15:09:48 mriedem gibi: http://logs.openstack.org/39/483939/1/check/gate-nova-tox-functional-ubuntu-xenial/95a614f/console.html#_2017-07-14_14_16_48_905908
15:09:59 mriedem and http://logs.openstack.org/17/483917/1/check/gate-nova-tox-functional-ubuntu-xenial/f92a89d/console.html#_2017-07-14_13_14_22_687228
15:10:10 mriedem i hit the first while trying to push a patch to debug the second
15:10:43 mriedem of course i'm also hitting random testtools.matchers._impl.MismatchError: 200 != 401 stupidity
15:10:47 mriedem b/c of the eventlet thing
15:11:09 gibi mriedem: seems you are the lucky winner today
15:11:16 mriedem melwitt: maybe we throw a hack retry loop in the functional api fixture if we get a 401
15:13:52 gibi mriedem: the host things seems like some kind of race condition. We can hack it away with by ignoring the host param during the sample matching
15:14:02 mriedem but,
15:14:12 mriedem we want to know the host is set in the sample i'd think
15:14:23 gibi mriedem: yeah, this is why it is just a hack
15:14:29 mriedem do we need to use a unique host maybe? or put a lock on the test?
15:17:32 gibi mriedem: honestly, I don't know
15:18:17 mriedem ok well i think i can push one change here
15:18:29 mriedem these metadata api functional tests aren't setting wsgi keepalive=false
15:19:57 mriedem melwitt: do you know if we have a bug for these random 401s in the functional tests?
15:20:14 mriedem maybe https://bugs.launchpad.net/nova/+bug/1696834
15:20:16 openstack Launchpad bug 1696834 in OpenStack Compute (nova) "Intermittent "KeyError: 'allocations'" in functional tests" [Medium,Confirmed]
15:20:19 mriedem same symptom i think
15:20:36 melwitt I dunno, maybe if sdague linked to one with his change? I don't know of one
15:20:49 mriedem yeah i was going to use that too
15:20:52 mriedem alright working on something
15:22:07 gibi mriedem: I will play a bit with the unshelve host mistmatch but it is already after 5pm in my time so not promising much for today
15:22:09 cdent this may be the same problem that the multicell scheduler functional test that dan’s working on is seeing (requests to the placement service and compute api get intermingled)
15:22:31 cdent “this” == the thing that mriedem is looking at
15:23:01 cdent that functional test: https://review.openstack.org/#/c/452006/
15:23:25 mriedem yeah
15:23:31 mriedem i hate "this"
15:23:38 mriedem also hate "that" though...
15:24:48 cdent long term if we switch the fixtures to use wsgi-intercept instead of installing an eventlet service, it _might_ help
15:24:50 openstackgerrit Balazs Gibizer proposed openstack/nova master: use already loaded BDM in instance.create https://review.openstack.org/483969
15:25:50 cdent but unless the the keepalive = false is being used everywhere, doing just that might fix it
15:26:22 mriedem it's not used everywhere yeah,
15:26:28 mriedem so that's what i'm going to try, throwing that into the ConfFixture
15:26:33 cdent mriedem++
15:26:34 mriedem which is used for default config in all testes
15:26:38 mriedem *tests
15:26:39 mriedem heh
15:27:25 cdent keepalive = false on testes is a complicated thought
15:27:40 cdent birth control?
15:27:50 figleaf vasectomy
15:28:10 mriedem come on guys, i try to keep a clean channel here
15:28:28 figleaf mriedem: that's the funniest thing you've said all year
15:34:05 openstackgerrit Merged openstack/nova master: Stop counting hw_video:ram_max_mb against quota https://review.openstack.org/483133
15:34:14 superdan woot
15:34:45 melwitt how'd that make it past the gauntlet
15:35:08 superdan melwitt: the universe wanted that to merge as bad as we did
15:35:25 melwitt heh
15:43:18 openstackgerrit Matt Riedemann proposed openstack/nova master: Set wsgi.keep_alive=False globally for tests https://review.openstack.org/483972
15:47:14 gibi mriedem: I have a theory about the unshelve host mismatch
15:47:41 gibi mriedem: the test first shelve offload the instance then unshelve it
15:48:06 gibi mriedem: the compute manager sets the state of the instance to SHELVE_OFFLOADED earlier than clearing the host of the instance
15:48:10 gibi mriedem: here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4446-L4457
15:48:15 openstackgerrit melanie witt proposed openstack/nova master: Make security_group_rules use check_deltas() for quota https://review.openstack.org/477700

Earlier   Later