Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-12
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 mriedem i know what you're asking
15:03:37 cdent lock _after_ getting the list
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 mriedem so getting 1000+ instances for every instance operation on the same host is blocked by this big lock
15:06:52 sean-k-mooney getting the list shouldnt be the moest expensive part though right its caulating the useage?
15:07:11 mriedem it's 1000+ instances + several joins to other tables
15:07:11 sean-k-mooney e.g. https://github.com/openstack/nova/blob/52b9359d6c6f387b8b9728c723c9a2501136d605/nova/compute/resource_tracker.py#L751-L752
15:07:16 mriedem over rpc as well....
15:07:28 mriedem cdent: i'm curious, what does vio set for rpc_response_timeout?
15:07:39 mriedem considering something like this has to pull 1K instances from the db over rpc
15:07:46 cdent getting the instances is by far the most expensive chunk of _update_avail when you have many instances
15:08:21 mriedem reminds me of this https://review.opendev.org/#/c/633042/
15:08:26 cdent mriedem: something relatively high, but it's not _that_ slow as to cause a problem, rather that the lock can be around for a long time
15:08:27 sean-k-mooney mriedem: 1000+ on vmware because its clustering multiple server under one compute service
15:08:40 mriedem sean-k-mooney: yes
15:08:49 mriedem cdent: but over the default 60 seconds?
15:09:02 sean-k-mooney cdent: ok that is surprising to me but if that is the most expesnive part then yes my suggestion wont help
15:09:22 cdent mriedem: I don't have the details to hand, but yes
15:09:46 mriedem cdent: you could potentially detect if you lost a race if there is something in self.tracked_instances that isn't in the list of instances you just pulled from the db
15:09:48 cdent sean-k-mooney: it depends on whether you are doing the loop the first time or subsequence times. 1st time is_bfv is the expensive part
15:10:16 sean-k-mooney i had expected self._update_usage_from_instances ot be more costly
15:10:32 cdent sean-k-mooney: yes, when bfv cache is cold, not but not otherwise
15:10:40 mriedem cdent: i'm assuming is_bfv is expensive since it has to make the is_volume_backed call which needs to get the bdms
15:10:46 cdent mriedem: yes
15:10:51 cdent and 1000 of those adds up
15:11:24 mriedem yeah...there is a change floating around where i asked someone if they could put a sqla joined field on the instances model to detect in a join query if an instance is volume-backed so we could optimize that
15:11:26 mriedem sec
15:11:50 mriedem the logic is pretty simple i think - is there a boot_index=0 bdm with destination_type=volume for this instance
15:11:52 mriedem if so, it's bfv
15:12:14 cdent once the cache is hot, it disappears as a problem which suggests that a similar instance cache would have similar properties...
15:12:26 cdent but managing such a cache is...tricky

Earlier   Later