Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-12
12:55:55 openstackgerrit Merged openstack/nova master: Update comments in HostManager._get_instance_info https://review.opendev.org/674929
13:29:11 stephenfin sean-k-mooney:
13:29:20 stephenfin sean-k-mooney: I think we just skip them?
13:29:49 sean-k-mooney ok
13:29:59 stephenfin The alternative is to hardcode a mapping of vif types to traits, but maybe that's needlessly complex?
13:30:10 stephenfin Or we can just add a unit test to validate things
13:30:23 sean-k-mooney well i am validating them
13:30:26 sean-k-mooney just later
13:30:39 sean-k-mooney when i am doing the actual update
13:30:59 sean-k-mooney sorry im validating them in the filter
13:31:06 sean-k-mooney not in the dirver
13:31:26 stephenfin Oh, if they're already validated somewhere then it doesn't matter
13:31:27 sean-k-mooney anyway ill skip them and log a warning
13:31:40 sean-k-mooney i should filter them out in the driver
13:32:08 sean-k-mooney but i want to cache the validation function as its not that cheap and i dont want to pay the cost on every request/update
13:32:53 sean-k-mooney it would also be nice to be able to add new vif_models without needing ot update placement first
13:33:50 sean-k-mooney i should still poing out however the validation only ensure that the version of placemetn install in the nova compute node thinks its valid not that the plamcent service thinks its valid
13:35:11 sean-k-mooney we really dont want to be making restcall to placemetn to validate it direclty
13:35:32 sean-k-mooney that would be more expesive then the current validation
13:38:07 mriedem lyarwood: can you hit this queens backport https://review.opendev.org/#/c/659247/ and then we can release queens
13:38:34 alex_xu efried: dansmith, good morning, I updated the vpmem spec https://review.opendev.org/675024
13:39:23 efried noted
13:39:53 sean-k-mooney oh disabling limit for affinity. i remember talking about that at the ptg
13:40:22 lyarwood mriedem: ack done
13:42:00 sean-k-mooney lyarwood: actully we had a customer issue related to no valid host with anti-affinity that coudl be releated too that? although have said that i dont think we set a limmit so the defualt would be 1000 and then had like 50 host so proably not
13:42:38 sean-k-mooney that would be good for larger deployment in anycase.
13:43:11 lyarwood sean-k-mooney: I can't recall it if we did
13:43:58 sean-k-mooney dhill was handeling it but i tink we concluded they were just running out of space.
13:44:30 sean-k-mooney e.g. the only host that still had enough capsaity were the host the anti afinity filter elminated
13:45:31 sean-k-mooney i dont think there was actully something broken in the end.
13:52:50 openstackgerrit Eric Fried proposed openstack/nova master: Multiple API cleanup changes https://review.opendev.org/666889
13:54:48 efried stephenfin and/or johnthetubaguy, That ^ is pretty much a "rebase" if you want to reapprove.
14:00:31 stephenfin efried: Sure thing
14:01:21 efried thanks
14:06:28 openstackgerrit Stephen Finucane proposed openstack/nova master: DRY get_flavor in flavor manage tests https://review.opendev.org/668281
14:16:05 efried stephenfin, johnthetubaguy: If y'all are still in a reviewy mood, this series has my +2 all the way up and its runway slot expires today: https://review.opendev.org/#/q/topic:bp/placement-req-filter-forbidden-aggregates+(status:open+OR+status:merged)
14:16:42 efried for whatever reason, I think it's a pretty cool feature
14:17:20 efried mriedem: Wanna move cross-cell resize into the runway?
14:18:50 mriedem sure
14:20:03 openstackgerrit Filippo Inzaghi proposed openstack/nova-specs master: Add Python 3 Train unit tests https://review.opendev.org/675908
14:21:36 stephenfin mriedem, efried: Dumb question, but the semaphore here means literally every other kind of operation (delete, (live) migrate, resize, etc.) on that instances is prevented until this eventually, returns, yeah? https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1867-L1868
14:21:50 stephenfin Assuming those operations are all wrapped with the same semaphore
14:22:13 mriedem stephenfin: you can't live migrate/resize an instance that's being built
14:22:31 mriedem delete yes https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L2801
14:23:54 stephenfin Okay, cool. I'm trying to invent reproducers for both https://review.opendev.org/#/c/663382/ and https://review.opendev.org/#/c/667294/
14:24:14 stephenfin Something that doesn't involve Heat, that is
14:25:06 stephenfin and trying to grok what could be stomping on what. Fun times :)
14:28:53 tssurya efried, mriedem: whenever you have time could you circle back to https://review.opendev.org/#/c/645611/ ? I think you were mostly ok on it, but just want to confirm if there is anything more to do there before I go on vacation
14:28:57 tssurya thanks in advance
14:35:37 mriedem tssurya: i won't +2 that until the ironic side is happy, since without that there is no point in the nova changes
14:36:15 tssurya mriedem: they are afaik, I will ask them to leave a comment in today's meeting
14:38:22 dansmith mriedem: tssurya who is/are the ironic person(s) that we should look to for sign-off there?
14:38:47 mriedem tssurya: also, you're going to have to rebase https://review.opendev.org/#/c/666889/
14:39:23 tssurya dansmith: dtantsur, TheJulia are the ones who have been reviewing it
14:39:30 dansmith okay
14:39:54 tssurya mriedem: gdi, thanks for letting me know
14:40:06 dansmith hah
14:40:08 tssurya that means I need to again change the version on the ironic side also
14:40:13 dansmith we've ruined tssurya :)
14:40:27 tssurya dansmith: tear emoji
14:40:28 mriedem used to be so innocent
14:40:45 mriedem https://review.opendev.org/#/q/file:%255Enova/api/openstack/compute/rest_api_version_history.rst+status:open
14:40:49 dansmith tssurya: I meant ruined because we've got you swearing when api versions change :P
14:41:05 tssurya dansmith: true that
14:41:19 dansmith tssurya: I say rebase that one on the cleanup to fix the version, and the ironic side, and let's push to get the acks in place so we can try to get it landed before it happens again
14:41:22 tssurya my office mate thinks I am swearing at him
14:41:28 dansmith heh
14:41:49 tssurya dansmith: ack, on it
14:45:52 TheJulia tssurya: will order of merging matter? Trying to figure out if I should try and get more eyes on the ironic side this week
14:46:32 tssurya nova one has to land first technically, but they just want to know if the ironic client side change looks good
14:47:06 tssurya TheJulia: more eyes this week would be great
14:47:31 dansmith what tssurya said
14:47:52 TheJulia tssurya: ack
14:50:39 efried tssurya: Do you have someone to take over for you if changes are needed while you're sipping mai tais on the beach?
14:58:05 cdent mriedem, dansmith : What do you imagine the consequences of moving the get_by_host_node in _update_available_resource out from under the COMPUTE_RESOURCE_SEMAPHORE would be? https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L744
14:58:25 cdent on a host with many insgtn
14:58:28 cdent oopes
14:58:46 cdent oh a host with many instances, that lock blocks new instance creation
15:00:09 cdent and a large portion of the time under the lock is spent in get_by_host_and_node assuming the is_bfv cache is hot (if it's cold it is _all_ there)
15:01:11 sean-k-mooney cdent: you might be able to optimise it without fully removing the lock. e.g. caulate the usage for each instance without the lock and keep track of the instace checked. aquire the lock and check that all instace check are still present and no others have been added then update the usage and release the lock.
15:01:39 sean-k-mooney if an instance has been added or removed then you can just make that update under the lock.
15:02:27 mriedem cdent: the consequence could be a race where we don't report usage or claim properly because the underlying resource usage changed since we got the lock
15:03:26 cdent mriedem: are you certain? I'm not saying remove the lock. I'm saying not use the lock while getting the list of instances used by _update_available_resources
15:03:37 cdent lock _after_ getting the list
15:03:37 mriedem i know what you're asking
15:03:57 mriedem you're saying move the get here https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L710
15:04:05 mriedem and pass the instances into _update_available_resource
15:04:20 mriedem sean-k-mooney: how would you know if an instance has been added or removed?
15:04:28 mriedem between the time you got the list and got the lock?
15:05:03 mriedem cdent: presumably you could have:
15:05:07 sean-k-mooney you would have to key two list of instances. one that you got outside the lock and then inside it you would get the list again
15:05:39 sean-k-mooney you can then caulated teh added and removed instances
15:05:46 mriedem t0: thread 1 is getting the list of instances on the host, t1: thread 2 gets the lock and is adding a new instance claim to the host which won't be in the list returned in thread 1, t2: thread 1 gets the lock with a list of instances that doesn't include the one from thread2
15:06:05 mriedem sean-k-mooney: getting hte list again within the lock defeats the purpose of what cdent is trying to do
15:06:10 sean-k-mooney if the list are the same the pending updats shoudl be correct if not you either need to recaulate it entirely or comute the delta
15:06:14 mriedem which is reduce the amount of time spent *within* the lock
15:06:26 cdent yes, that is the goal
15:06:38 mriedem b/c vcenter has 1000+ instances on the same compute service host / RT
15:06:52 sean-k-mooney getting the list shouldnt be the moest expensive part though right its caulating the useage?
15:06:52 mriedem so getting 1000+ instances for every instance operation on the same host is blocked by this big lock

Earlier   Later