Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-08-06
12:58:30 sean-k-mooney actully its not broken
12:58:32 sean-k-mooney its correct
12:58:45 sean-k-mooney rather ((0 if is_bfv else flavor.root_gb) +
12:58:47 sean-k-mooney swap_in_gb + flavor.ephemeral_gb)
12:58:49 sean-k-mooney is correct
12:58:55 stephenfin how?
12:59:10 sean-k-mooney the bracket after root_gb
12:59:16 sean-k-mooney the if only applies to the root gb
12:59:50 sean-k-mooney i guess if you had not swap or ephemeral_gb it woudl still be wrong
12:59:58 stephenfin ah, indeed
13:00:04 stephenfin I missed that. Good spot!
13:00:28 stephenfin Yeah, we should be checking for that but it's not as serious
13:00:33 sean-k-mooney that is handel in the other implemataion here https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L456-L459
13:01:19 sean-k-mooney if you want to fix it just condtionally append https://github.com/openstack/nova/blob/master/nova/scheduler/utils.py#L406
13:18:42 efried dustinc: left a comment. Better as a @property so you don't have to do icky things like passing arbitrary *args, **kwargs or method names as strings
13:19:26 openstackgerrit Chris Dent proposed openstack/nova master: single pass instance info fetch in host manager https://review.opendev.org/623558
13:19:26 openstackgerrit Chris Dent proposed openstack/nova master: add InstanceList.get_all_uuids_by_hosts() method https://review.opendev.org/623557
13:19:27 openstackgerrit Chris Dent proposed openstack/nova master: DNM: Play with cache of host mappings in nova scheduler https://review.opendev.org/674811
13:19:27 openstackgerrit Chris Dent proposed openstack/nova master: DNM: add profiling to scheduler https://review.opendev.org/674810
13:19:48 cdent mriedem: then of that ^ stack has some interesting observations about host states
13:19:56 cdent s/then/the end/
13:21:17 sean-k-mooney cdent: sylvain is on vaction for a few weeks so i or someone else on the team will pick up his placement audit patch
13:21:25 sean-k-mooney cdent: quick question on https://review.opendev.org/#/c/670112/2/nova/cmd/manage.py@2764
13:21:39 sean-k-mooney cdent: are you asserting that pagination should not be required
13:21:45 cdent hi sean-k-mooney , reading
13:21:58 sean-k-mooney e.g. i can just just remove that not/question
13:22:19 cdent sean-k-mooney: yes, just remove that note, I think. as there's no plan to do pagination (server-side) in placement
13:22:41 sean-k-mooney cool i think the other comment does need to be address
13:22:47 cdent yes
13:23:11 sean-k-mooney i wont get to this today more then likely but ill try and get to it tomrrow or the day after assumeing someone else doesnt pick it up in the mean time
13:23:19 mriedem cdent: where have i seen host mapping caching before.... https://review.opendev.org/#/c/663388/ :)
13:23:37 cdent mriedem: somehow I am not surprised
13:23:53 zhouyao hi sean-k-mooney, could you check this patch? it's been two weeks. https://review.opendev.org/#/c/621646
13:24:26 cdent mriedem: i'm confused about the track_instance_changes stuff. why be dependent on that?
13:25:29 sean-k-mooney zhouyao: yes i can take a look but you will need to get input form the core team also. ill try and review again today.
13:26:04 cdent mriedem: hrmm, yours is much more...robust
13:26:12 cdent or at least wordy
13:26:22 efried [1] https://review.opendev.org/#/c/605237/4/specs/train/approved/instance-ops.rst
13:26:22 efried sean-k-mooney: I tend to agree re cyborg blocking instance operations. I went to figure out what had been committed to in the spec, and it's a tad wobbly, but the approved spec points to this (open) document in the cyborg repo [1] that seems to declare support for things like pause and shelve.
13:26:28 sean-k-mooney zhouyao: i see you have added mriedem and stephenfin to the review list i would reach out to alex_xu too
13:27:02 sean-k-mooney efried: we had it in the spec for last cycle. or at least discussed it but then we removed it form the spec this cycle
13:27:03 mriedem cdent: "i'm confused about the track_instance_changes stuff. why be dependent on that?" you mean yours or mine?
13:27:11 mriedem mine isn't dependent on jay's series
13:27:43 sean-k-mooney efried: a yes we moved it to cyborg this cycle intead of in the nova spec
13:28:29 efried it should really have been in this spec, but people kept complaining it was too long.
13:29:01 sean-k-mooney efried: ya. that said we did disucss this at some lent both in person at the ptg and last cycle
13:29:14 efried sean-k-mooney: Do you remember what was agreed upon?
13:29:36 sean-k-mooney i though it was everything bar live migration
13:30:12 sean-k-mooney but basically https://review.opendev.org/#/c/605237/4/specs/train/approved/instance-ops.rst i think is more or less what i was expecting
13:30:19 cdent mriedem: I mean in yours. Why even mention that config setting? The idea is that if you got to HostMapping, use the cache. it seems that at least some of the time we hit "# Host is running old version, or updates aren't flowing."
13:35:24 efried sean-k-mooney: I maybe remember Sundar talking about this being an interim step in the overall strategy, where subsequent patches will implement support and remove the blockers one by one.
13:35:58 sean-k-mooney efried: perhaps but normally we just dont expost the feature to the user until its done
13:36:18 sean-k-mooney which in this case would mean putting the patch that reads the flavor extra spec at the end
13:36:25 mriedem "updates aren't flowing" == track_instance_changes=False
13:36:34 mriedem when HostState.updated could be Falsey I'd have to check
13:36:47 mriedem i don't mind removing the mention of the config option in my commit message though
13:37:04 efried sean-k-mooney: In any case, I put a hold on the bottom patch, which is something we do anyway on a series like this until it's closer to done.
13:37:35 sean-k-mooney i saw that there has been some work started on a fake driver and some tempset tests
13:38:18 efried yes. I understand they've been having a tough time with... follow through from... previously committed resources...
13:38:26 sean-k-mooney i only have a 3 day week this week but im hoping to be able to try out some of this in the next week or two
13:38:32 mriedem hmm, i think set_update_time_on_success isn't using what it once did (RequestSpec vs HostState)
13:39:07 mriedem oh nvm
13:39:42 sean-k-mooney efried: well it does look like at least in the last week or so that progess has been picking up a bit
13:40:36 sean-k-mooney i need to try and review more of this work but i also need to review gibi port migrtion stuff so time is a factor.
13:41:15 mriedem cdent: i think the "older computes" comment is from when edleafe wrote this https://review.opendev.org/#/c/160511/ back in kilo :)
13:49:03 mriedem cdent: but yeah i suppose we have to hit that method to get the host mapping at least once per host to determine which cell the host is in so we know where to pull the instances from (which cell db), so in your test with 25 fake computes the profiler shows that method was called 25 times
13:49:04 cdent mriedem: the patch prior to my last one there shows how to do profiling in an eventlet situation, in case you want to try it elsewhere
13:49:30 mriedem yup i saw, thanks. that would be useful with something like this god awful mess https://review.opendev.org/#/c/674422/
13:49:34 cdent (I tried to do 100 fake computes and my compute explored)
13:50:04 cdent exploded even
13:50:05 mriedem i don't think i've been able to go over 30 fake computes in a devstack with 8GB RAM and 8 VCPU
13:50:13 mriedem and 1 API_WORKER per service
13:50:59 cdent I think I could probably make 50, I'm pretty smooth at 25 with 16GB,16vcpu
13:51:23 mriedem you know, i'd give a scratch n sniff sticker if someone threw up a small doc about how to do profiling like this in nova somewhere in https://docs.openstack.org/nova/latest/contributor/#testing
13:51:44 cdent I think I could do that, but it would really depend on the smell
13:52:13 efried gibi, melwitt: Y'all okay if I continue working on the series at https://review.opendev.org/#/c/524306/ ?
13:54:07 gibi efried: it is OK for me. I can keep reviewing the patches.
13:54:16 efried gibi: I'd be fixing the func failures, either in your poison patch or by slotting another one underneath. And melwitt I'd be addressing gibi's comment on the bottom by removing that fixture.
13:54:41 efried gibi: You think the ComputeDriver.get_available_resource signature change is worth doing?
13:56:22 gibi efried: I need a refresh why we started this series in the first place. What is wrong with generating a new context. I got involved into it due to the testing impact of it
13:56:53 dustinc efried: good point thanks
13:57:13 efried gibi: I'm not completely sure why the series was *started*, but I can tell you why I got involved.
13:58:10 gibi efried: I'm going to dig into the past to see the reason..
13:58:14 efried gibi: The global_request_id we're logging is coming from whatever RequestContext is in oslo.context's thread local store, which is *not* the same context we're using to pass the X-Openstack-Request-Id header around to other services like Placement.
13:58:33 efried which means that, without additional hacking, we can't actually follow the request from e.g. n-cpu to placement
13:58:38 efried which was the whole point of global_request_id in the first place.
13:58:50 gibi efried: thanks, that seems to be a worthy cause
13:59:00 efried So I've been trying to track down how that whole mechanism works and how we can get it so you can actually grep logs for a request ID and see the flow of an operation.
13:59:36 efried gibi: My latest attempt actually diverged from that series a little bit: https://review.opendev.org/#/c/674138/
13:59:41 gibi is there any alternative to passing the context to the ComputeDriver.get_available_resource ?
14:00:42 efried To avoid having that one libvirt method call get_admin_context? Yes, melwitt and I talked about storing the context on, like, the compute manager object and accessing it from the virt driver via the virtapi.
14:00:55 efried Questionable which is a worse hack, really.
14:01:13 gibi I see
14:01:13 efried Having it as a param to get_available_resource seems appropriate from a zoomed-out perspective
14:01:18 efried but it's a fairly intrusive change as you can see.
14:01:26 efried but
14:01:27 gibi yepp
14:01:42 efried that's assuming we need to be putting restrictions on get[_admin]_context like this at all.
14:02:02 efried which, if we're doing that more carefully, we might not need to bother with
14:02:40 mriedem cdent: typically root beer and grape are all i offer

Earlier   Later