15:05:05 <yamahata> #startmeeting neutron_northbound 15:05:05 <odl_meetbot> Meeting started Mon Nov 20 15:05:05 2017 UTC. The chair is yamahata. Information about MeetBot at http://ci.openstack.org/meetbot.html. 15:05:05 <odl_meetbot> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 15:05:05 <odl_meetbot> The meeting name has been set to 'neutron_northbound' 15:05:23 <yamahata> #chair manjeets rajivk_ 15:05:23 <odl_meetbot> Current chairs: manjeets rajivk_ yamahata 15:05:30 <yamahata> #topic agenda bashing and roll call 15:05:32 <yamahata> #info yamahata 15:05:34 <manjeets_> #info manjeets 15:05:35 <rajivk_> #info rajivk 15:06:35 <yamahata> The last week ODL M2 on Nov 14. 15:06:41 <yamahata> I have to sent out the report 15:06:48 <yamahata> #action yamahata ODL M2 report 15:07:17 <yamahata> just FYI On Dec 4 and Dec 11, I would be absent. 15:07:44 <yamahata> the video for openstack summit sydney is available. 15:08:04 <yamahata> #link https://www.openstack.org/videos/summits/sydney-2017 openstack summit sydney video 15:08:16 <yamahata> any other announcement? 15:09:11 <yamahata> Oops. any other agenda? 15:09:40 <yamahata> At the last, I'd like to discuss about schedule too. Probably people will be on vacation during December, January. 15:10:15 <yamahata> I suppose Dec 25 and Jan 1, we would skip this meeting unless someone steps up to chair. 15:11:14 <yamahata> any other topic? 15:11:36 <yamahata> move on 15:11:41 <yamahata> #topic Announcements 15:11:51 <yamahata> I mistakenly had two announcement already. 15:11:56 <yamahata> any other announcement? 15:12:51 <yamahata> okay move on 15:12:55 <yamahata> #topic action items from last meeting 15:13:41 <yamahata> mpeterson to create a task or bug to cleanup ODL between tests 15:13:45 <yamahata> we had three items 15:13:53 <yamahata> to decide if we should and who should participate in the different meetings 15:13:59 <yamahata> yamahata update timeslot to 15:00UTC on wiki 15:14:35 <manjeets_> yamahata, you mean other openstack or ODL meetings ? 15:14:54 <yamahata> yes. neutron irc meeting and zuulv3 meeting. 15:14:56 <mkolesni> hi sorry didnt see you started 15:15:19 <manjeets_> ok 15:15:20 <yamahata> mkolesni: no problem. Now we're checking action items. 15:16:07 <yamahata> for cleanups between tests, the item isn't created yet. We'll see mpeterson later. 15:16:28 <mkolesni> he's ill today so he wont be joining us 15:16:43 <yamahata> For meetings, so far from networking-odl team, we don't have attended to neutron irc meeting persistently. 15:17:01 <yamahata> Oh oh. sorry to hear that. 15:17:12 <mkolesni> when is neutron irc meeting? 15:17:30 <manjeets_> openstack neutron meeting 15:17:57 <yamahata> http://eavesdrop.openstack.org/#Neutron_Team_Meeting 15:18:20 <yamahata> Monday at 2100 UTC in #openstack-meeting and Tuesday at 1400 UTC in #openstack-meeting 15:18:49 <yamahata> timeslot is rotating biweekly to accomodate timezones. 15:18:59 <yamahata> So far Isaku attends it intermittently. 15:19:23 <yamahata> At worst we can check it's meeting minutes to follow it. 15:19:35 <manjeets_> so do I 15:19:44 <mkolesni> do you see value to attend it more regularly? 15:20:24 <yamahata> Since we're part of neutron stadium, we should be aware that what's happening in neutron community. 15:20:34 <manjeets_> I guess its good to engage with neutron community, for getting reviews, bringing issues there 15:20:34 <yamahata> At least neutron-lib direction. 15:21:59 <yamahata> maybe our attendance would be best effort basis. 15:22:02 <mkolesni> ok 15:22:28 <mkolesni> i can try to attend the tues one as its in my tz 15:23:00 <yamahata> Cool. 15:23:18 <yamahata> The last item. Isaku updated meeting schedule to Monday at 15:00 UTC on wiki pages. 15:23:36 <yamahata> It's now UTC, not PST/PDT. 15:23:55 <yamahata> okay move on 15:24:00 <yamahata> #topic patches/bugs 15:24:28 <yamahata> we discussed patches on stable branches. 15:24:41 <yamahata> I suppose most patches were reviewed, right? 15:24:54 <yamahata> mkolesni: do you have any patches unreviewed? 15:25:17 <mkolesni> none currently 15:25:29 <mkolesni> mpeterson did want to discuss a related debate we had 15:25:36 <mkolesni> i can raise it in his name 15:25:58 <mkolesni> generally it is about whether we want to test log calls or not in unit tests 15:26:26 <mkolesni> i claim that its auxiliary functionality which should be tested separately if at all 15:26:37 <mkolesni> while he claims its necessary to test all logging 15:26:46 <mkolesni> do you have an opinion on the matter, yamahata ? 15:27:12 <yamahata> So far I've thought it's case-by-case. 15:27:24 <yamahata> Does mpeterson claim all logging? 15:27:26 <mkolesni> im not saying we shouldnt test, just that we probably dont want to test each log line but maybe the important ones 15:27:31 <mkolesni> yes 15:27:47 <mkolesni> let me find that patch 15:28:03 <yamahata> To me for example, logs related to journal analysis makes sense to me. 15:28:31 <yamahata> all logging would be overkill. But I'm open. 15:28:36 <mkolesni> i.e. https://review.openstack.org/#/c/519384/6/networking_odl/tests/unit/journal/test_periodic_task.py 15:28:56 <mkolesni> i thought its not necessary to test the logging in that test case 15:29:09 <mkolesni> since if the log changes, it will break the test 15:29:26 <mkolesni> and the test does do logical validations which are much more important 15:29:49 <mkolesni> also, he doesnt test for other logs which also occur, so to me thats problematic 15:30:11 <mkolesni> i suggested if he is keen on testing the logging there, he can do it in dedicated tests that test the logging 15:30:36 <rajivk_> mkolesni, in this specific case, i agree with mpeterson, because we are just making sure it enters that if condition, of course behavior checking is already done, but this check does not hurt anything and make sure that we returned from that particular statement 15:30:50 <manjeets_> testing all will be a overkill for sure, I guess having tests for critical ones would be good enough 15:31:07 <mkolesni> also im not very fond of mocking logging in general since it's too coupling, IMHO we should use fake logger and be less dependent on the specific messge, level, etc 15:31:32 <rajivk_> +1 mkolesni 15:31:39 <yamahata> that makes sense. 15:31:52 <rajivk_> Our test cases fill up console with logs, a lot of logs appear. 15:32:05 <rajivk_> We can make fixture in base class 15:32:16 <rajivk_> but the problem is, some of them check logs 15:32:23 <mkolesni> if its really important to test that logging message im not against, but i prefer it to be in a separate case to make sure that if tests break its easier to identify the cause 15:32:27 <rajivk_> so we need to undo mocking for those specific test cases 15:32:39 <mkolesni> yea sure i think that would be better 15:33:01 <rajivk_> mkolesni, I differ on separate test. 15:33:02 <mkolesni> also OSLO has some capability to log capture, but i haven't been successful in making it work :/ 15:33:31 <mkolesni> rajivk_, the value of test is the ability to quickly spot the breakage 15:33:56 <mkolesni> rajivk_, thats why its better to have separate tests for separate cases than a single big one 15:34:08 <rajivk_> mkolesni, how will it make difficult to identify breakage 15:34:14 <mkolesni> i argue that logging is quite auxiliary and as such can have a separate case 15:34:18 <rajivk_> we know, where assert statements getting failed. 15:34:53 <rajivk_> mkolesni, Why would we like to maintain extra code for checking only logs? 15:35:04 <rajivk_> yamahata, what is your opinion? 15:35:09 <mkolesni> if i just look at the failed test list it will be hard to quickly know, besides checking the actual test failure 15:35:28 <mkolesni> if the test name indicates its logging testing, its easy to know what messed up 15:35:31 <mkolesni> easier 15:35:31 <yamahata> I prefer small test cases instead of big one. 15:35:47 <yamahata> mkolesni: sounds reasonable. 15:36:20 <yamahata> If we found it's not true, we can change our direction. 15:37:09 <mkolesni> so the decision is still to test on a per-basis need yes? 15:37:23 <yamahata> Yes for now. 15:37:28 <rajivk_> yes 15:37:34 <mkolesni> ok cool 15:37:42 <yamahata> If we find issue with that direction or mpeterson convinced us, we can change the direction. 15:37:43 <mkolesni> ill let mpeterson know 15:38:02 <mkolesni> he can probably voice his opinion next meeting or over the email 15:38:04 <yamahata> if necessary, we can continue discuss ionnext ween with mpeterson. 15:38:29 <yamahata> Cool. 15:38:38 <yamahata> okay, next patches/bugs? 15:38:51 <yamahata> We have recovery patches floating around. 15:39:29 <mkolesni> were in discussion with rajivk_ on this to move it along faster 15:39:39 <yamahata> #link https://review.openstack.org/#/c/520339/ 15:39:39 <yamahata> [Discussion] Design for full sync and recovery of resources 15:40:07 <yamahata> #link https://review.openstack.org/#/c/500366/ Recovery: Moved resource fetching in drivers 15:40:22 <mkolesni> do you want to voice your opinion here, or will we continue to do so on the patch? 15:40:58 <yamahata> rajivk_: you're on stage. 15:41:42 <rajivk_> yamahata, have you gone through patch? 15:41:59 <yamahata> do you mean 520339? Not yet. 15:42:03 <yamahata> Today I'll review it 15:42:16 <yamahata> #action yamahata review https://review.openstack.org/#/c/520339/ 15:42:18 <rajivk_> mkolesni and mpeterson, wants it to be separated to make into smaller patches to get it merged 15:42:41 <mkolesni> generally it has become too big to keep track of 15:42:49 <yamahata> splitting the patch into smaller ones makes sense. 15:42:56 <yamahata> It make review easier. 15:43:00 <rajivk_> And move common functionality into a base class. 15:43:16 <yamahata> Yes. 15:43:20 <mkolesni> yes it would be the most forward solution 15:43:33 <mkolesni> straightforward 15:43:40 <rajivk_> and all drivers inherit from the base class. get_resources and get_resource for full_sync and recovery 15:44:00 <mkolesni> yes and can override default behavior if necessary 15:44:10 <rajivk_> I want to move full sync and recovery inside a class as well. 15:44:25 <mkolesni> later we might find more usefullness for base class such as it can keep the journal 15:44:45 <mkolesni> rajivk_, i dont understand why, theyre already in modules with very simple logic 15:44:45 <rajivk_> reason is, i want to limit the scope to a class for full sync and recovery 15:45:21 <mkolesni> limit the scope of what? the base class? 15:45:44 <rajivk_> no, full sync and recovery methods 15:45:51 <rajivk_> variables and method 15:46:06 <rajivk_> I find, code more manageable that way 15:46:20 <mkolesni> i dont think they need to move only the logic to fetch the resources from the plugin 15:46:32 <mkolesni> this is decoupling and gives better code 15:46:45 <mkolesni> but why splinter the logic of full sync and recovery? 15:46:52 <mkolesni> it will become harder to maintain 15:47:30 <rajivk_> mkolesni, what do you mean by splinter of full sync and recovery? 15:47:35 <mkolesni> if you can send an example patch with the change youre proposing maybe it sill be easier to understand, doesnt have to compile or anything just to get the idea of what you want to move where 15:47:50 <rajivk_> sure 15:47:52 <mkolesni> even can be done in https://review.openstack.org/#/c/520339/ 15:48:03 <mkolesni> just do the split youre proposing so that we can see 15:48:28 <rajivk_> okay 15:48:34 <mkolesni> ok cool 15:48:39 <mkolesni> sorry i have to go 15:48:46 <yamahata> okay. 15:48:50 <mkolesni> have a nice week! 15:48:50 <yamahata> mkolesni: anything urgent? 15:48:55 <mkolesni> none from me 15:49:05 <manjeets_> https://review.openstack.org/#/c/519001/ 15:49:05 <yamahata> cool. 15:49:58 <manjeets_> this need review as https://review.openstack.org/#/c/519759/ is dependent on it 15:50:43 <yamahata> #action everyone review https://review.openstack.org/#/c/519001/ 15:50:55 <yamahata> manjeets_: do you have any arguable points? 15:51:02 <manjeets_> none 15:51:19 <yamahata> Okay. any ther patches/bugs? 15:51:31 <manjeets_> just a minor one i have 15:51:43 <manjeets_> i updated the refrence doc to use v2 driver 15:51:47 <rajivk_> manjeets, regarding https://review.openstack.org/#/c/504297/ 15:52:01 <rajivk_> Can you review this one? 15:52:16 <manjeets_> sure rajivk_ 15:52:17 <rajivk_> yamahata, manjeets, for information 15:52:31 <rajivk_> grenade does not support latest lib/neutron 15:52:39 <rajivk_> it still follow neutron-legacy 15:52:56 <yamahata> Oh oh. 15:53:02 <rajivk_> so for now, for grenade jobs, i am planning to keep lib/neutron-legacy 15:53:24 <yamahata> It makese sense. 15:53:27 <manjeets_> rajivk_, I am going to ask openstack qa if they have any plans on that 15:53:50 <rajivk_> manjeets_, i think, every project needs to register for upgrade 15:54:00 <rajivk_> and there is not direct way to specify order 15:54:25 <manjeets_> rajivk_, we can force it like we did for networking-odl to upgrade first 15:54:37 <rajivk_> so if i can plug neutron from networking-odl for upgrade, first we have to change order, like manjeets is doing 15:54:52 <rajivk_> but as far as i see problem with it is 15:55:14 <rajivk_> grenade looks in the same directory, from which project was registered 15:55:33 <rajivk_> so even if i register neutron from networking-odl neutron will not be upgraded 15:56:14 <rajivk_> that's all regarding lib/neutron 15:56:48 <manjeets_> rajivk_, can't we leave grenade on neutron-legacy 15:57:07 <rajivk_> yes, that's what current patch does :) 15:57:14 <manjeets_> i'll ask qa folks if they have any plans on grenade update 15:57:22 <rajivk_> manjeets_, thanks 15:57:39 <yamahata> Cool. 15:57:46 <yamahata> we have 3 mins left 15:57:49 <yamahata> any thing else? 15:57:51 <manjeets_> #action manjeets to talk to qa folks about neutron-legacy in grenade 15:57:55 <rajivk_> yamahata, wait 15:58:17 <yamahata> rajivk_: sure. 15:58:34 <rajivk_> I suspect deletion of completed row directly can cause some problem 15:58:46 <rajivk_> i am not sure, just suspecting it. 15:59:03 <yamahata> can you elaborate on it? 15:59:15 <yamahata> what kind of problem? 15:59:16 <rajivk_> so, when we have an entry depends on other resource and that resource is synced and deleted from db. 15:59:32 <rajivk_> do you think, while adding this entry, it will fail. 16:00:17 <rajivk_> because of foreign key 16:00:28 <rajivk_> constraint? 16:00:38 <yamahata> It will result in db exception and it will be retried. 16:00:54 <yamahata> Let me check what db exception will be retried. 16:01:43 <rajivk_> yes, so it will happens almost for all the resources? 16:01:47 <yamahata> At worst in networking-odl, we can raise neutron db retry request. 16:02:06 <rajivk_> I checked it, there are lot of failures and retry 16:02:49 <yamahata> I see. So is the correctness ok? 16:02:51 <rajivk_> We need log statement here https://github.com/openstack/networking-odl/blob/master/networking_odl/journal/journal.py#L121, to identify these kind of issues 16:03:16 <yamahata> It's a matter of performance with too much retry or too much logs? 16:03:26 <rajivk_> performance 16:04:03 <rajivk_> should we lower down retries at this statement 16:04:07 <yamahata> I see. at least it makes sense to add log there. 16:04:34 <rajivk_> however i don't know, how much it impact performance? 16:04:38 <yamahata> Regarding to performance, benchmarking is needed. 16:04:59 <yamahata> Are you observing so much retry? 16:05:18 <rajivk_> i saw many on the patches i tried to observe 16:05:24 <yamahata> Oh I see. 16:05:49 <yamahata> Originally direct deletion was added by mkolensni for performance issue. 16:05:59 <rajivk_> hmm, https://review.openstack.org/#/c/521130/ 16:06:31 <rajivk_> i did not see this much failure. mkolesni told that there were lot of entries in db during their performance testing 16:07:00 <rajivk_> i am not saying 5 minutes is a good time, it is just to verify whether it is the issue or not 16:07:21 <rajivk_> check this one http://logs.openstack.org/40/518540/3/check/networking-odl-rally-carbon/652432c/logs/screen-q-svc.txt.gz 16:08:37 <yamahata> the log is too large. I'm downloading it. 16:08:56 <yamahata> Now we don't have mkolensni. Can we continue next week? 16:09:11 <rajivk_> sure 16:09:22 <yamahata> without any benchmark, it's difficult to tell it. 16:09:28 <yamahata> anything else urgent? 16:09:30 <rajivk_> yes, agree 16:09:34 <rajivk_> no 16:09:59 <yamahata> okay thank you everyone. 16:10:02 <rajivk_> If it is possible to, check ERROR logs in logs and report at the end of jobs, it will be great 16:10:17 <yamahata> sure. 16:10:40 <yamahata> #topic cookies 16:10:46 <yamahata> #endmeeting