20:00:35 #startmeeting horizon 20:00:36 Meeting started Wed Oct 18 20:00:35 2017 UTC and is due to finish in 60 minutes. The chair is ying_zuo. Information about MeetBot at http://wiki.debian.org/MeetBot. 20:00:37 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 20:00:39 The meeting name has been set to 'horizon' 20:00:45 Hi everyone o/ 20:01:13 hi 20:01:30 Hi 20:01:32 hi! 20:01:38 o/ 20:01:41 Hi 20:01:44 o/ 20:01:51 o/ 20:01:59 hi 20:01:59 o/ 20:02:30 o/ 20:02:32 #topic Notices 20:02:41 #topic Queens-1 milestone 20:02:46 #link https://launchpad.net/horizon/+milestone/queens-1 20:02:53 I will be tagging Horizon for the Queens-1 release shortly 20:02:56 o/ 20:03:06 We got a good list of tickets resolved and the backlog has been reduced from over 800 open tickets at the PTG to 634 right now. 20:03:18 Thank you all for your contribution towards this achievement! 20:03:19 Even lower if you exclude the Incomplete bugs 20:03:28 Since we have to wait 60 days for those to expire :) 20:03:33 that's right :) 20:03:46 good news! 20:03:50 amotoki did a check after removing Incomplete and Wishlist and it was around 400, iirc 20:03:56 Which is a huge improvement 20:04:00 👏 20:04:13 I'm really happy that people are helping to shrink the bug list down :) 20:04:32 robcresswell: +1 20:04:37 +1 20:05:00 +1 20:05:09 #topic Mox to Mock migration 20:05:32 ying_zuo: thanks! 20:05:55 I just want to get some attention to my patches from the all team 20:05:59 #link https://review.openstack.org/509618 20:06:10 ^^ it's the first patch 20:06:22 amotoki helped me a lot with reviewing 20:06:48 the first two patches are in a pretty good shape (I hope) 20:06:54 I promised to help, but didn't have the time, I'm sorry 20:07:12 so we need to decide how do we want to go forward with it 20:07:24 rdopiera: np 20:08:15 mostly, it's about code style: there are several ways how it can be done 20:08:45 the last patch (https://review.openstack.org/510118) should be refactored a bit, to be more clean. I'll do it this week 20:09:46 I'll be happy to get any feedback 20:10:12 e0ne: thanks for working on this 20:10:16 as discussed with robcresswell earlier, we want to get all patches to be merged together 20:10:32 so we won't be in a half way on implementing this 20:11:19 what still needs to be decided? 20:11:49 ying_zuo: codestyle, I guess 20:12:10 e.g. use @mock.patch or 'with mock.patch()' statement 20:12:23 I mean what to use in general 20:12:42 I think they are both okay 20:12:51 I don't think that we should allow only one 20:12:59 ying_zuo: +1 20:13:47 I think, that's all from my side. I'll be waiting for reviews to go forward with this as it was discussed at PTG 20:13:59 cool. thanks 20:14:14 #topic Changes Made in https://review.openstack.org/#/c/509038/ 20:14:33 some of you may have seen the discussion in irc regarding this 20:14:45 It seemed okay since the quota check is also done when the modal is opened and it would mitigate the bad performance on this panel 20:15:03 amotoki raised the concern about the inconsistency that this patch introduced since we normally check the quota for such button 20:15:31 I’d like to hear what everyone thinks of this 20:15:52 I don't really like it, personally :/ 0.1 seconds is so minor. 20:16:15 is it 0.1 seconds per row or per page? 20:16:16 There are bigger issues with panels taking literally minutes to load 20:16:28 What quota check completes 0.1 seconds? 20:16:32 that depends on the number of instances 20:16:51 it would be great to have the same behavior for all such buttons, not only for instances 20:17:22 It's a single nova api call afaik 20:18:04 0.1 seconds for one instance 20:18:05 robcresswell: imo, we have to decrease number of calls to nova, neutron and glance APIs 20:18:20 Even a single nova api call in a deployed env will take longer than that 20:18:43 ying_zuo: Isn't it just one API call overall? 20:18:58 Is 0.1 on devstack? 20:19:01 Its just the Launch Instance button at the top of the instances page 20:19:16 "which takes about 0.14 seconds based on our prod env (prod API with local horizon)" 20:19:20 robcresswell: oh right. just that call 20:19:20 From the patch commit msg 20:19:27 https://review.openstack.org/#/c/509038 20:19:44 I can understand it if its doing it per-row, perhaps like in the Images table or volumes or something 20:20:16 Personally I feel like this is a bit of an over-optimisation. Its kinda stripping functionality for the sake of raw speed. 20:20:38 alright. we can revert it then 20:20:56 joined now. we call server_list() to count the number of servers. if you say server_list() is one call, it is true. 20:21:01 0.14 Is much less than we discussed in the horizon channel 20:21:15 ¯\_(ツ)_/¯ 20:21:36 yes, that sounded like a big performance improvement 20:21:42 Can we just replace the page already :) 20:22:21 amotoki: We use server_list to count instances? Don't we just use the limit value? That makes no sense 20:22:43 I know in Neutron they dont do quotas properly so thats not an option 20:22:53 robcresswell: I think _get_tenant_compute_usages() in openstack_dashboard/usage/quotas.py is still used 20:23:02 but I might be missing something 20:23:05 did anybody so some perf testing what exactly takes a lot of time> 20:24:19 I think the owner of the patch did some performance testing 20:24:39 flwang has a few patches on the go 20:24:43 I agree with some, disagree with others 20:25:05 It totally depends on a scale of deployments, so I think it is better to introduce a setting if it really needs it 20:25:15 it would be great to have something like https://cl.ly/1C381K3V2U21 20:25:36 I think robcresswell suggested a setting for one of the tickets 20:25:37 unfortunately, I don't have more profiler results by the hand at the moment 20:26:11 filter by image name or something 20:26:16 Tenant limits call was removed 20:26:18 Its just a single limits api call then checking if the current is less than the absolute 20:26:28 This is usage and limits 20:26:29 unless our API layer is doing some funky stuff 20:26:29 amotoki: I don't thinks that introducing cofig params for different scale of deployments is good idea 20:26:38 s/cofig/config 20:28:01 robcresswell: ah, tenant_limit_usages is used. 20:28:54 Yeah 20:29:02 I really don't think this is an expensive issue 20:29:16 The patch itself even says 0.14 seconds... 20:29:58 At 0.14 I would agree that doesn't make sense. I would verify the 0.14 wasn't a typo though 20:30:22 That would be wise :) 20:30:29 I never had a nova api call in production ever go that quickly 20:30:44 I would like to know how long it takes as a whole and 0.14 occupies what portion of the whole. 20:31:29 the ticket said the page load is very slow but 0.14 wouldn't be noticeable... 20:31:46 I'm kinda surprised you could even do a network roundtrip in 0.14s :p 20:32:30 the problem is we have too much unnecessary 0.14s 20:32:37 david-lyle_: :) 20:33:00 flwang around? 20:33:02 flwang: but that's just one call 20:33:12 david-lyle: yep 20:33:15 just in office 20:33:53 ying_zuo: sorry for the interrupt, i mean we have some unnecessary api calls 20:34:03 I would assume the issue is usually many small calls happening per-row, so its unnecessary api calls many times 20:34:17 flwang: what calls do you mean? 20:34:21 for most of the cases as I know, user just want to see a list and don't really care about all the details on one page 20:34:26 robcresswell: +1 20:34:33 robcresswell: exactly 20:34:42 flwang: But this is just one API call, one time 20:34:47 Not per-row, afaik 20:34:50 flwang: it's not True for my customers 20:35:20 like robcresswell said above, it depends on the scale 20:35:37 just double check, are you talking about the image list call or nova list? 20:35:54 we are talking about this patch https://review.openstack.org/#/c/509038 20:36:51 ok, for that patch, I think even there is only 0.1s, we should remove it 20:37:02 because there are duplicated calls when launch instnace 20:37:37 especially when your nova list take more than 1s, at that moment, you just want to remove any unnecessary api call 20:38:11 but the launch resource button usually has quota check 20:38:18 this raises another inconsistency: buttons are disabled in some tables and not disabled in others. 20:38:26 0.14s could be a small time for some deployments, but for a serious prod env, it does matter 20:38:58 amotoki: we should improve all of them if it's the right thing 20:39:07 what is the right thing? 20:39:20 remove duplicated calls and improve the perf 20:39:20 If your prod env is seriously bottlenecked by 0.14s, I think you should be documenting everything you have done so the world can marvel at its efficiency :p 20:39:41 robcresswell: it's not fair 20:40:28 it sounds like "my nova list api call only takes 0.5s so I don't care about the extra 0.14s" 20:40:38 My personal feeling is that this introduces an inconsistency, its ripping code out of Horizon for the sake of tiny savings when there are much, much bigger issues you / we could spend our coding / reviewing / investigating time on 20:40:39 it's true for somebod 20:41:04 Its more like, my Instances page takes 2 minutes to load, lets spend time reducing it by 0.14s :/ 20:41:29 robcresswell: i think we're talking about if it should save 20:41:59 I understand that your use case is clearly based around barebones data and max performance; but I dont think that should be what Horizon is. 20:42:36 We definitely need performance improvements, I just think this crosses the line between performance / utility, personally. Anyway I'll stop speaking so others can :) 20:42:39 okay, then feel free revert it and we can keep the patch in our private repo 20:43:19 flwang: I appreciate that you are trying to improve the performance, but sorry, consistency is important for the UI 20:43:25 it's my bad 20:43:48 I somehow thought that's more than one call 20:44:01 I think we would like to revert it 20:44:53 I think we will skip the bug reports review for this week 20:44:53 ying_zuo: that's all good. most of the work I'm proposing is to get a better performance based on current status, before the perfect angularized instance panel 20:45:02 but we can't wait 20:45:43 alright 20:45:51 #topic Open Discussion 20:46:17 if you have any code review requests or questions, this is the perfect time to raise them 20:46:47 ying_zuo: and all cores:-) https://review.openstack.org/#/q/status:open+project:openstack/horizon+branch:master+topic:bp/neutron-trunk-ui 20:47:32 thanks lajoskatona. I was going to mention that since you have asked earlier in the channel 20:47:40 In Denver we asked for help to get review even without perfect coverage, as we need feedback to see if the direction for the trunk actions are good. 20:47:50 In Denver we asked for help to get review even without perfect coverage, as we need feedback to see if the direction for the trunk actions are good. 20:47:53 Could you please help us in that? 20:48:02 it will be great if somebody can test and/or review my IE-related patches: https://review.openstack.org/505617 and https://review.openstack.org/505710 20:48:20 what is this IE you speak of? ;-) 20:48:39 ying_zuo: Thanks 20:48:40 gary-smith: I said the same to customer 20:48:50 but had to fix these bugs :( 20:49:52 ying_zuo: Me or Bence can collect the fishy parts (as I remember there are TODOs and NOTEs for them, but need to doublecheck) and send in mail or irc 20:50:02 ying_zuo: Me or Bence can collect the fishy parts (as I remember there are TODOs and NOTEs for them, but need to doublecheck) and send in mail or irc 20:50:35 lajoskatona: or you file bugs on them with tags like trunk and/or neutron 20:50:43 lajoskatona: maybe it's in the etherpad 20:50:58 lajoskatona: or you can add them to the blueprint page 20:51:27 9 mins reminder 20:51:54 i have one question on ngdetail reload fix. we rejected the fix(es) proposed as it is not a good approach. 20:52:07 on the other hand, we haven't fixed an high priority bug. can we compromise on an approach at some point? 20:52:17 yeah some is really on the pad 20:53:01 amotoki: I think you are talking about this patch https://review.openstack.org/#/c/491346/ 20:53:15 Ok, We can add them to the blueprint page 20:53:46 ying_zuo: actually I am not sure what is the exact patch we are now moving forward. several changes have been proposed. 20:54:28 yes. I think this is only active one though 20:54:32 amotoki: the patches in question are still open, but we were lazy to fix the missing tests without knowing that the direction is good or not (i.e.: using the stepModels in a perhaps "new" way) 20:56:09 lajoskatona: is there a problem for the current approach? 20:57:15 I mean if you have any specific question or ran into some issue 20:57:22 ying_zuo: you mean by listing the most questionable parts in the blueprint for the open trunk panel changes? 20:57:58 yes, would be great if you leave the questions on the blueprint. 20:58:07 there's a whiteboard section 20:58:15 ying_zuo: ok we do that, thanks for the help 20:58:36 lajoskatona: btw, as a quick look https://review.openstack.org/#/c/493070/ looks unrelated to the trunk-ui blueprint. is it related? 20:59:17 i wonder there is any dependency. 20:59:33 amotoki: it is, as the creat and edit button uses the transfer-table and Bence found some issues in that 20:59:58 amotoki: so to make the create and edit work that was included to the patch chain 21:00:16 lajoskatona: thanks for clarification 21:00:40 I am glad that we had some great discussion today 21:00:45 thanks everyone for joining 21:01:03 amotoki: thanks for the attentions 21:01:07 #endmeeting