14:02:23 <ihrachys> #startmeeting neutron_upgrades
14:02:26 <openstack> Meeting started Thu Feb  8 14:02:23 2018 UTC and is due to finish in 60 minutes.  The chair is ihrachys. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:02:27 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:02:29 <openstack> The meeting name has been set to 'neutron_upgrades'
14:02:42 <lujinluo> o/
14:02:43 <ihrachys> o/
14:02:47 <TuanVu> Hi Ihar
14:02:50 <TuanVu> Hi Luo
14:03:07 <hungpv> Hi guys
14:03:22 <slaweq> hi
14:03:30 <ihrachys> PSA: this week is RC1 (some time Friday) which should afaiu create stable/queens for neutron
14:03:36 <TuanVu> Hi Slawek
14:04:08 <ihrachys> after which we should be able to cautiously start merging Pike patches
14:04:28 <ihrachys> sorry, I mean Rocky patches
14:04:53 <ihrachys> of course, while final release is not shipped, the focus will be diverged to Queens matters
14:05:54 <ihrachys> as some of you know, we have an OVO related issue in bgpvpn that is release blocker for them
14:06:03 <ihrachys> https://launchpad.net/bugs/1746996
14:06:04 <openstack> Launchpad bug 1746996 in neutron "bagpipe: IPAllocation DB query failing, engine facade mismatch" [High,Confirmed]
14:07:13 <ihrachys> it's a rather twisted issue. but basically, it boils down to bgpvpn using OVO objects in callback handlers that are issued from new engine facade subtransactions, so it fails rather spectacularly because it doesn't see models created by the callback caller in its session
14:07:32 <ihrachys> and there is no clear cut way out of the issue.
14:08:04 <ihrachys> one idea we are trying right now is switching bgpvpn objects to new facade, while leaving other objects on the old one for Queens
14:08:19 <ihrachys> there is patch for bagpipe here: https://review.openstack.org/#/c/541513/
14:08:37 <ihrachys> but it would require neutron change: https://review.openstack.org/#/c/541512/
14:08:50 <ihrachys> and the latter is not ready for prime time, lots of unit tests were failing
14:09:00 <ihrachys> I see lujinluo picked it up, thanks for that
14:09:14 <lujinluo> ihrachys: it is still not in shape yet
14:10:37 <ihrachys> yeah I know
14:10:48 <ihrachys> so some questions about your changes
14:11:00 <ihrachys> first, what's about subnetpool change to new_facade=True? https://review.openstack.org/#/c/541512/4..5/neutron/objects/subnetpool.py
14:12:08 <lujinluo> one minute, finding the code snippets
14:13:25 <ihrachys> there is also https://review.openstack.org/#/c/541512/4..5/neutron/db/db_base_plugin_v2.py that deals with subnetpool
14:14:41 <lujinluo> sorry, the new_facade=True might be garbage code that i forgot to delete
14:15:16 <lujinluo> as for the latter, it is because of this change https://review.openstack.org/#/c/541512/5/neutron/objects/base.py@319
14:15:40 <lujinluo> we need to refresh(db_obj) in case synthetic field changed
14:16:10 <lujinluo> but if the session is not active, we ignored it then we miss the synthetic field change in db
14:16:55 <ihrachys> shouldn't session be active at this point?
14:17:22 <lujinluo> somehow no
14:18:21 <lujinluo> neutron.tests.unit.db.test_db_base_plugin_v2.TestSubnetPoolsV2.test_allocate_any_subnet_with_prefixlen is the failed UT
14:18:33 <lujinluo> http://logs.openstack.org/12/541512/4/check/openstack-tox-py27/15c63a0/testr_results.html.gz first one here
14:20:28 <lujinluo> We missed to refresh the synthetic field (prefixes) of subnetpool
14:20:51 <ihrachys> ok I guess prefixes are not reloaded in time. maybe we should put refresh in subnetpool object code, I will have a look
14:21:23 <lujinluo> ok. that is basically the only failed UT I managed to mitigate, LOL
14:21:54 <ihrachys> actually, maybe it's the decorator that should open subtransaction so that it always spans throughout func() AND refresh()
14:22:21 <ihrachys> well that's still something. there are other valuable changes there where I missed model to obj_cls conversion
14:22:27 <ihrachys> thanks for looking into it
14:22:38 <ihrachys> I will take it back over today
14:22:54 <ihrachys> I thought tmorin will help with that but he probably was busy
14:23:22 <lujinluo> yes, although i have not fixed any other UTs, but during the debugging it seems that most of the UTs are about timing to commit changes
14:23:22 <ihrachys> now to other, non-blocking patches
14:23:38 <ihrachys> lujinluo, timing to commit?
14:23:53 <lujinluo> i mean when to open and close transactions
14:25:04 <lujinluo> also, i want to address that in the patch we are changing some autonested sessions to context.session, and i am not sure if that is the right thing to do
14:25:37 <ihrachys> lujinluo, do you have an example of the latter? it's easier to follow with an example.
14:26:13 <lujinluo> https://review.openstack.org/#/c/541512/5/neutron/objects/base.py@599
14:26:17 <lujinluo> here you go
14:26:59 <ihrachys> right. what's the problem with that?
14:27:06 <lujinluo> i think when we use autonested we are addressing the transaction inside transaction issue, but with this patch we lost that
14:27:41 <ihrachys> you mean like independent rollback?
14:27:49 <lujinluo> yes
14:28:10 <ihrachys> I don't think we use it except in some places where it's explicit in the code (I believe ipam layer had rollback per ip allocation)
14:28:36 <ihrachys> autonested just helps us to not care about whether caller opened a subtransaction already
14:29:15 <ihrachys> begin(subtransactions=True) is afaik the same as autonested_transaction
14:29:16 <lujinluo> ok, i see. then maybe we do not need to care about it
14:30:16 <ihrachys> ok, moving to other patches
14:30:21 <ihrachys> https://review.openstack.org/#/q/status:open+project:openstack/neutron+branch:master+topic:bp/adopt-oslo-versioned-objects-for-db
14:30:33 <ihrachys> https://review.openstack.org/#/c/507772/ "Use Network OVO in db_base_plugin"
14:30:49 <TuanVu> For this patch, the good news is: there’s just only 1 remaining failed unit test at this moment, which is: “test_get_objects_queries_constant”
14:30:52 <ihrachys> there are 3 failures right now, all because of number of queries not scaling well
14:31:03 <TuanVu> yes
14:31:08 <TuanVu> I’m trying to find exactly where the number of queries is increased
14:32:29 <ihrachys> based on diff of queries in logs, it seems that's because of externalnetworks table fetch
14:33:13 <TuanVu> thank you, Ihar
14:33:28 <ihrachys> also networksecuritybindings is fetched twice
14:33:37 <ihrachys> I guess once per network object
14:33:46 <TuanVu> yes, I agree, but I still haven't found where it is increased
14:34:24 <ihrachys> afaiu ExternalNetwork is a type of one of synthetic fields of Network
14:34:52 <TuanVu> correct
14:34:58 <ihrachys> in theory, it should be eagerly fetched with network model because its relationship is lazy='joined'
14:35:34 <TuanVu> yes
14:36:27 <ihrachys> hm, I am looking at Network object from_db_object: https://review.openstack.org/#/c/507772/36/neutron/objects/network.py@326
14:36:40 <ihrachys> and I don't see where we would extract values for external from db model
14:37:44 <ihrachys> hm, though maybe it's base class that is driving synthetic fields extraction, I am already not sure, it was so long time ago I wrote it :)
14:38:29 <TuanVu> thanks for your suggestion, Ihar :)
14:38:33 <ihrachys> this may actually be related to the same thing why you needed to catch InvalidRequestError here: https://review.openstack.org/#/c/507772/36/neutron/objects/base.py@320
14:38:39 <TuanVu> we'll check it deeper
14:39:38 <ihrachys> if you don't refresh the relationship, maybe it refetches it somewhere else once per object returned. I think ultimately catching the error is not correct and we should make sure that the session is there for the time when refresh is issued
14:39:53 <ihrachys> that boils down to what we discussed for that other patch adding support for new_engine
14:40:42 <ihrachys> actually, that's a good patch to try my approach with subtransaction opened in decorator. if it works, it will probably fix the failure.
14:40:48 <ihrachys> I will try it on your patch later.
14:40:50 <TuanVu> thanks, we'll try to figure out way to handle it more properly
14:41:03 <ihrachys> ok
14:41:14 <ihrachys> https://review.openstack.org/#/c/521797/ "Use Router OVO in external_net_db"
14:42:04 <ihrachys> this has pep8 issue, let me fix it quickly
14:42:44 <ihrachys> done
14:42:53 <ihrachys> apart from that, it seems fine
14:43:02 <hungpv> Thanks
14:43:06 <ihrachys> https://review.openstack.org/#/c/537320/ "Use Port OVO in neutron/db/external_net_db.py"
14:43:48 <ihrachys> this patch is largely a variation on that other patch adding support for new_engine
14:44:13 <ihrachys> so we'll need to rebase it on top of the patch at some point to be able to utilize the new engine feature
14:44:35 <ihrachys> it also has this InvalidRequestError exception handler that we'll probably try to avoid
14:45:02 <ihrachys> seems like this patch will need to wait a bit for when those underlying issues are handled
14:45:19 <lujinluo> yes, i will rebase this one on top of https://review.openstack.org/#/c/541512/ later when it is ready
14:45:34 <ihrachys> right. it's not ready yet so no need to rebase right away
14:45:43 <ihrachys> ok next is https://review.openstack.org/#/c/537325/ "Use Meter Label OVO in neutron/db/metering/metering_db.py"
14:46:01 <ihrachys> I had a question there https://review.openstack.org/#/c/537325/4/neutron/db/l3_dvr_db.py@1077
14:46:09 <ihrachys> not sure you had a chance to think about it
14:46:26 <lujinluo> no... i will check tmr
14:47:13 <ihrachys> I personally hate those functions that try to support polymorphic argument types. seems convoluted. I wonder if it's too hard to switch all callers of is_distributed_router to OVO
14:48:35 <ihrachys> ok let's move on
14:49:20 <ihrachys> https://review.openstack.org/#/c/530182/ "Use Router OVO in l3_db"
14:49:26 <ihrachys> the author is not responsive
14:49:40 <ihrachys> if people feel like taking that over, I wouldn't mind :)
14:51:41 <ihrachys> there are no more patches in the queue that would be worth looking at at this point
14:53:13 <ihrachys> anyone has a topic to discuss?
14:53:21 <lujinluo> i have one thing
14:53:36 <ihrachys> lujinluo, shoot
14:53:36 <lujinluo> i will not be able to join the weekly meeting next week
14:53:45 <lujinluo> as it is chinese new year eve
14:53:55 <lujinluo> i won't have the mood to work, LOL
14:53:59 <ihrachys> ok no problem. happy new year :)
14:54:05 <lujinluo> thanks!
14:54:07 <ihrachys> you are lucky, you have two of them!
14:54:20 <lujinluo> LOL, yeah
14:54:57 <lujinluo> i think this also applies to TuanVu ? right?
14:54:59 <ihrachys> is it the same / similar for others? or is it just in your part of Asia?
14:55:09 <lujinluo> you guys have public holiday on that day, no?
14:55:11 <TuanVu> yes, I think so
14:55:28 <ihrachys> I see. well then we will probably cancel the next meeting, and meet in 2 weeks
14:55:37 <lujinluo> yeah, Vietnamese also celebrate lunar new year
14:55:48 <ihrachys> enjoy the holiday!
14:55:54 <lujinluo> thank you!
14:56:03 <TuanVu> correct, the same with An-san and Hung-san
14:56:10 <lujinluo> happy chinese new year of dog to you too ihrachys and TuanVu
14:56:30 <TuanVu> thank you, Ihar and Luo :)
14:56:34 <ihrachys> yay
14:56:34 <ihrachys> *°*”˜˜”*°•.¸☆ ★ ☆¸.•°*”˜˜”*°•.¸☆
14:56:34 <ihrachys> ╔╗╔╦══╦═╦═╦╗╔╗ ★ ★ ★
14:56:35 <ihrachys> ║╚╝║══║═║═║╚╝║ ☆¸.•°*”˜˜”*°•.¸☆
14:56:35 <ihrachys> ║╔╗║╔╗║╔╣╔╩╗╔╝ ★ NEW YEAR ☆
14:56:35 <ihrachys> ╚╝╚╩╝╚╩╝╚╝═╚╝ ♥￥☆★☆★☆￥♥ ★☆❤♫❤♫❤
14:56:36 <ihrachys> .•*¨`*•..¸☼ ¸.•*¨`*•.♫❤♫❤♫❤
14:56:41 <TuanVu> happy chinese new year to you, too
14:56:42 <TuanVu> :)
14:56:44 <ihrachys> ok, thanks everyone for joining :)
14:56:47 <ihrachys> #endmeeting