| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-14 | |||
| 14:47:42 | figleaf | mriedem: I like smartifying stuff | |
| 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 | |