)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"365e34041e13b9a0670057ca8521b42935cd8d2f","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"update port may takes an excessive number of seconds"},{"line_number":10,"context_line":"to complete if dvr routers are running on more than 100"},{"line_number":11,"context_line":"compute nodes. This patch tries to save some time by removing"},{"line_number":12,"context_line":"unnecessary calls inside looping through hosts."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Ide740e0c5c43c2d2b842460a37c8ce125da12b28"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"bfb3d3c7_a01c35c1","line":11,"range":{"start_line":11,"start_character":53,"end_line":11,"end_character":61},"updated":"2019-05-27 11:12:05.000000000","message":"Could you give us some test results on how much performance improvement has been achieved please?","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     shenjiatong \u003cyshxxsjt715@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-05-28 09:02:42 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"improve dvr port update under large scale deployment"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"update port may takes an excessive number of seconds"},{"line_number":10,"context_line":"to complete if dvr routers are running on more than 100"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"bfb3d3c7_006baf1d","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":11},"updated":"2019-05-28 14:59:16.000000000","message":"s/Improve DVR","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"improve dvr port update under large scale deployment"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"update port may takes an excessive number of seconds"},{"line_number":10,"context_line":"to complete if dvr routers are running on more than 100"},{"line_number":11,"context_line":"compute nodes. This patch tries to save some time by removing"},{"line_number":12,"context_line":"unnecessary calls inside looping through hosts."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"bfb3d3c7_405e8776","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":6},"updated":"2019-05-28 14:59:16.000000000","message":"s/Update","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"74a60c82bdee5ce02a2fd59682da5bb5b2b2097e","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     申嘉童 \u003cyshxxsjt715@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2019-06-04 19:38:35 +0800"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"improve dvr port update under large scale deployment"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"update port may takes an excessive number of seconds"},{"line_number":10,"context_line":"to complete if dvr routers are running on more than 100"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_d51e360b","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":11},"updated":"2019-06-04 14:30:36.000000000","message":"nit: Improve DVR","commit_id":"00eb6f26f6165a00d647d2bf35fb7996534cfc09"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"74a60c82bdee5ce02a2fd59682da5bb5b2b2097e","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"improve dvr port update under large scale deployment"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"update port may takes an excessive number of seconds"},{"line_number":10,"context_line":"to complete if dvr routers are running on more than 100"},{"line_number":11,"context_line":"compute nodes. This patch tries to save some time by removing"},{"line_number":12,"context_line":"unnecessary calls inside looping through hosts."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":13,"id":"9fb8cfa7_35d3123b","line":9,"range":{"start_line":9,"start_character":0,"end_line":9,"end_character":6},"updated":"2019-06-04 14:30:36.000000000","message":"nit: Update","commit_id":"00eb6f26f6165a00d647d2bf35fb7996534cfc09"}],"neutron/db/l3_dvrscheduler_db.py":[{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":148,"context_line":"            if port_host:"},{"line_number":149,"context_line":"                host_router_dict[port_host] \u003d set(router_ids)"},{"line_number":150,"context_line":"            for router_id in router_ids:"},{"line_number":151,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"},{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_36916b48","line":149,"range":{"start_line":149,"start_character":46,"end_line":149,"end_character":61},"updated":"2019-05-27 13:53:05.000000000","message":"can use collections.defaultdict(set)","commit_id":"85a64ae72375a8db9d69fb781761f0536c744755"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":148,"context_line":"            if port_host:"},{"line_number":149,"context_line":"                host_router_dict[port_host] \u003d set(router_ids)"},{"line_number":150,"context_line":"            for router_id in router_ids:"},{"line_number":151,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"},{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"bfb3d3c7_56965f41","line":149,"range":{"start_line":149,"start_character":16,"end_line":149,"end_character":32},"updated":"2019-05-27 13:53:05.000000000","message":"consider \"host_routers\"","commit_id":"85a64ae72375a8db9d69fb781761f0536c744755"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"365e34041e13b9a0670057ca8521b42935cd8d2f","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":148,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":149,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":150,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":151,"context_line":"                            context.elevated(), router_id, host):"},{"line_number":152,"context_line":"                        updated_routers.add(router_id)"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_e026ad15","side":"PARENT","line":150,"range":{"start_line":150,"start_character":28,"end_line":150,"end_character":60},"updated":"2019-05-27 11:12:05.000000000","message":"Why this check is not needed anymore?","commit_id":"3f837836f679e13a93607c1ad7f9714fa1405b9b"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":148,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":149,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":150,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":151,"context_line":"                            context.elevated(), router_id, host):"},{"line_number":152,"context_line":"                        updated_routers.add(router_id)"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_f6af537a","side":"PARENT","line":150,"range":{"start_line":150,"start_character":28,"end_line":150,"end_character":60},"in_reply_to":"bfb3d3c7_7b4d8ec2","updated":"2019-05-27 13:53:05.000000000","message":"Right, if we already executed get_hosts_to_notify for each router, then  _check_for_rtr_serviceable_ports is redundant.","commit_id":"3f837836f679e13a93607c1ad7f9714fa1405b9b"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"78f5ffbf983f29c5e55bcee8692fb3377a932003","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":148,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":149,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":150,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":151,"context_line":"                            context.elevated(), router_id, host):"},{"line_number":152,"context_line":"                        updated_routers.add(router_id)"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_7b4d8ec2","side":"PARENT","line":150,"range":{"start_line":150,"start_character":28,"end_line":150,"end_character":60},"in_reply_to":"bfb3d3c7_bb40e6a6","updated":"2019-05-27 12:28:27.000000000","message":"(N-1)*t is under assumption that there is only one router. If there is more than one, then formula should be (N-1)*M*t, where M is number of routers","commit_id":"3f837836f679e13a93607c1ad7f9714fa1405b9b"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"eeb61f38db183387333f51120f0e33681a2c2d18","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":148,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":149,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":150,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":151,"context_line":"                            context.elevated(), router_id, host):"},{"line_number":152,"context_line":"                        updated_routers.add(router_id)"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_9bee42b2","side":"PARENT","line":150,"range":{"start_line":150,"start_character":28,"end_line":150,"end_character":60},"in_reply_to":"bfb3d3c7_e026ad15","updated":"2019-05-27 12:20:15.000000000","message":"host list is get by get_hosts_to_notify and this implies host must have l3-agent hosting the router.","commit_id":"3f837836f679e13a93607c1ad7f9714fa1405b9b"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"eeb61f38db183387333f51120f0e33681a2c2d18","unresolved":false,"context_lines":[{"line_number":147,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":148,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":149,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":150,"context_line":"                    if self._check_for_rtr_serviceable_ports("},{"line_number":151,"context_line":"                            context.elevated(), router_id, host):"},{"line_number":152,"context_line":"                        updated_routers.add(router_id)"},{"line_number":153,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_bb40e6a6","side":"PARENT","line":150,"range":{"start_line":150,"start_character":28,"end_line":150,"end_character":60},"in_reply_to":"bfb3d3c7_e026ad15","updated":"2019-05-27 12:20:15.000000000","message":"unfortunately I do not get opportunity to test change on our product environment. But if the logic makes sense, then at least I believe this can save around (N-1)*t seconds, where N is number of hosts hosting router_id and t is time used to filter out router ports by router_id.","commit_id":"3f837836f679e13a93607c1ad7f9714fa1405b9b"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"157fe7c26bc8e88d3a524668fbb8f2d48473815c","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            # (ushen) save host - router relationship in a dict"},{"line_number":141,"context_line":"            # to avoid performance issue when looping through a large number of hosts"},{"line_number":142,"context_line":"            # the performance issue is get_ports method does cost at least half of second to complete"},{"line_number":143,"context_line":"            # so program should avoid calling this method inside a loop"},{"line_number":144,"context_line":"            host_router_dict \u003d dict()"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_1d3a43cb","line":143,"updated":"2019-05-27 19:35:33.000000000","message":"This part of the code was introduced to support the Interconnected routers.\nSo the first problem that I see here is this code path is being called for every router, even if the router is not interconnected.\nSo first we should eliminate this code.\nAs I mentioned in the bug report, please check for the interconnected routers, if there are more then one router, then go execute this code path ( is with the proposed change not to call the get_port within the loop).\nOtherwise directly call the notify method with the host and router-id that we have.","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"7cf998f497daf10c63bb687ad2ed19f19c1496c6","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            # (ushen) save host - router relationship in a dict"},{"line_number":141,"context_line":"            # to avoid performance issue when looping through a large number of hosts"},{"line_number":142,"context_line":"            # the performance issue is get_ports method does cost at least half of second to complete"},{"line_number":143,"context_line":"            # so program should avoid calling this method inside a loop"},{"line_number":144,"context_line":"            host_router_dict \u003d dict()"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_0ed1ab23","line":143,"in_reply_to":"bfb3d3c7_1d3a43cb","updated":"2019-05-28 04:33:58.000000000","message":"Wasn\u0027t check for interconnected routers done at line #120 in get_dvr_routers_by_subnet_ids()?","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            # to avoid performance issue when looping through a large number of hosts"},{"line_number":142,"context_line":"            # the performance issue is get_ports method does cost at least half of second to complete"},{"line_number":143,"context_line":"            # so program should avoid calling this method inside a loop"},{"line_number":144,"context_line":"            host_router_dict \u003d dict()"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_d6ca4f4c","line":144,"range":{"start_line":144,"start_character":12,"end_line":144,"end_character":28},"updated":"2019-05-27 13:53:05.000000000","message":"Consider host_routers","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            # to avoid performance issue when looping through a large number of hosts"},{"line_number":142,"context_line":"            # the performance issue is get_ports method does cost at least half of second to complete"},{"line_number":143,"context_line":"            # so program should avoid calling this method inside a loop"},{"line_number":144,"context_line":"            host_router_dict \u003d dict()"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_76aa8371","line":144,"range":{"start_line":144,"start_character":31,"end_line":144,"end_character":37},"updated":"2019-05-27 13:53:05.000000000","message":"consider collections.defaultdict(set)","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"68181aed90ac9677f1a3b21167856bc0f63bfa77","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            # to avoid performance issue when looping through a large number of hosts"},{"line_number":142,"context_line":"            # the performance issue is get_ports method does cost at least half of second to complete"},{"line_number":143,"context_line":"            # so program should avoid calling this method inside a loop"},{"line_number":144,"context_line":"            host_router_dict \u003d dict()"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_63d3a0d6","line":144,"range":{"start_line":144,"start_character":31,"end_line":144,"end_character":37},"in_reply_to":"bfb3d3c7_76aa8371","updated":"2019-05-28 02:31:58.000000000","message":"Done","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"68181aed90ac9677f1a3b21167856bc0f63bfa77","unresolved":false,"context_lines":[{"line_number":141,"context_line":"            # to avoid performance issue when looping through a large number of hosts"},{"line_number":142,"context_line":"            # the performance issue is get_ports method does cost at least half of second to complete"},{"line_number":143,"context_line":"            # so program should avoid calling this method inside a loop"},{"line_number":144,"context_line":"            host_router_dict \u003d dict()"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_03dc24e2","line":144,"range":{"start_line":144,"start_character":12,"end_line":144,"end_character":28},"in_reply_to":"bfb3d3c7_d6ca4f4c","updated":"2019-05-28 02:31:58.000000000","message":"Done","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"52dfa6d2ae8de75cc96743fc51a8a4b6a35397d1","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":148,"context_line":"            if port_host:"},{"line_number":149,"context_line":"                host_router_dict[port_host] \u003d set(router_ids)"},{"line_number":150,"context_line":"            for router_id in router_ids:"},{"line_number":151,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"},{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_e0d9cdf2","line":150,"updated":"2019-05-27 11:20:20.000000000","message":"So basically what you are doing here is to loop in routers and then hosts, instead of hosts and then routers. I don\u0027t see the advantage here.","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":148,"context_line":"            if port_host:"},{"line_number":149,"context_line":"                host_router_dict[port_host] \u003d set(router_ids)"},{"line_number":150,"context_line":"            for router_id in router_ids:"},{"line_number":151,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"},{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_16b547cd","line":150,"in_reply_to":"bfb3d3c7_9bc40261","updated":"2019-05-27 13:53:05.000000000","message":"the main idea is to get rid of _check_for_rtr_serviceable_ports","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"c90e0794adfc39144aa55e8b182b91de8073f3e4","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":148,"context_line":"            if port_host:"},{"line_number":149,"context_line":"                host_router_dict[port_host] \u003d set(router_ids)"},{"line_number":150,"context_line":"            for router_id in router_ids:"},{"line_number":151,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"},{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_9bc40261","line":150,"in_reply_to":"bfb3d3c7_e0d9cdf2","updated":"2019-05-27 12:08:02.000000000","message":"take a look at this https://github.com/openstack/neutron/blob/3f837836f679e13a93607c1ad7f9714fa1405b9b/neutron/db/l3_dvrscheduler_db.py#L268. The problem is get_ports method is not very fast. In our environment, I observe it takes 0.5s or more to finish. And in normal cases, exchange looping order and save subnet as a local variable will avoid invoking get_ports too many times.","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"52dfa6d2ae8de75cc96743fc51a8a4b6a35397d1","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":154,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":155,"context_line":"                    if host_router_dict[host] is None:"},{"line_number":156,"context_line":"                        host_router_dict[host] \u003d {router_id}"},{"line_number":157,"context_line":"                    else:"},{"line_number":158,"context_line":"                        host_router_dict[host] |\u003d {router_id}"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_40c9b923","line":155,"updated":"2019-05-27 11:20:20.000000000","message":"What you are missing here is what we had in PSbase, to check is this router has serviceable ports (_check_for_rtr_serviceable_ports).","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":154,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":155,"context_line":"                    if host_router_dict[host] is None:"},{"line_number":156,"context_line":"                        host_router_dict[host] \u003d {router_id}"},{"line_number":157,"context_line":"                    else:"},{"line_number":158,"context_line":"                        host_router_dict[host] |\u003d {router_id}"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_56e41fe1","line":155,"in_reply_to":"bfb3d3c7_3b8cf634","updated":"2019-05-27 13:53:05.000000000","message":"+1 to norman shen","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"07bfe01bfed72a671cf3c88635581c6e7487aeff","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":154,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":155,"context_line":"                    if host_router_dict[host] is None:"},{"line_number":156,"context_line":"                        host_router_dict[host] \u003d {router_id}"},{"line_number":157,"context_line":"                    else:"},{"line_number":158,"context_line":"                        host_router_dict[host] |\u003d {router_id}"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_6c8705b7","line":155,"in_reply_to":"bfb3d3c7_3b8cf634","updated":"2019-05-28 17:11:01.000000000","message":"Yes, but this function is also filtering by portbinding profile:\n\nhttps://github.com/openstack/neutron/blob/master/neutron/db/l3_dvrscheduler_db.py#L486\n\nThat\u0027s why, I suppose, in the func tests [1] you have more calls to \"routers_updated_on_host\". For example, when port1 is updated with the portbindings, port2 doesn\u0027t have yet this information.","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"c90e0794adfc39144aa55e8b182b91de8073f3e4","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":154,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":155,"context_line":"                    if host_router_dict[host] is None:"},{"line_number":156,"context_line":"                        host_router_dict[host] \u003d {router_id}"},{"line_number":157,"context_line":"                    else:"},{"line_number":158,"context_line":"                        host_router_dict[host] |\u003d {router_id}"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_3b8cf634","line":155,"in_reply_to":"bfb3d3c7_40c9b923","updated":"2019-05-27 12:08:02.000000000","message":"I do not understand why you need `_check_for_rtr_serviceable_ports` because basically host is retrieved from get_hosts_to_notify and this already implies router is locating on the host","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"da2629ed4bfb6c85c9efa3e4a500e851723c2559","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":154,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":155,"context_line":"                    if host_router_dict[host] is None:"},{"line_number":156,"context_line":"                        host_router_dict[host] \u003d {router_id}"},{"line_number":157,"context_line":"                    else:"},{"line_number":158,"context_line":"                        host_router_dict[host] |\u003d {router_id}"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"            for host, routers in host_router_dict.items():"},{"line_number":161,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_560dff9e","line":158,"range":{"start_line":155,"start_character":20,"end_line":158,"end_character":61},"updated":"2019-05-27 13:53:05.000000000","message":"Just host_router_dict[host].add(router_id) is you use collections.defaultdict(set) as I suggested above","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"68181aed90ac9677f1a3b21167856bc0f63bfa77","unresolved":false,"context_lines":[{"line_number":152,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":153,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":154,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":155,"context_line":"                    if host_router_dict[host] is None:"},{"line_number":156,"context_line":"                        host_router_dict[host] \u003d {router_id}"},{"line_number":157,"context_line":"                    else:"},{"line_number":158,"context_line":"                        host_router_dict[host] |\u003d {router_id}"},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"            for host, routers in host_router_dict.items():"},{"line_number":161,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_e349d0b6","line":158,"range":{"start_line":155,"start_character":20,"end_line":158,"end_character":61},"in_reply_to":"bfb3d3c7_560dff9e","updated":"2019-05-28 02:31:58.000000000","message":"Done","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"52dfa6d2ae8de75cc96743fc51a8a4b6a35397d1","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"            for host, routers in host_router_dict.items():"},{"line_number":161,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":162,"context_line":"                    context, list(routers), host"},{"line_number":163,"context_line":"                )"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def get_dvr_snat_agent_list(self, context):"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_80e2519f","line":162,"updated":"2019-05-27 11:20:20.000000000","message":"this can be a set too, this conversion is not needed.","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"68181aed90ac9677f1a3b21167856bc0f63bfa77","unresolved":false,"context_lines":[{"line_number":159,"context_line":""},{"line_number":160,"context_line":"            for host, routers in host_router_dict.items():"},{"line_number":161,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":162,"context_line":"                    context, list(routers), host"},{"line_number":163,"context_line":"                )"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def get_dvr_snat_agent_list(self, context):"}],"source_content_type":"text/x-python","patch_set":4,"id":"bfb3d3c7_c34cccc9","line":162,"in_reply_to":"bfb3d3c7_80e2519f","updated":"2019-05-28 02:31:58.000000000","message":"Done","commit_id":"1d7f8282fc85452fe5b315c1660c97f4ad963e79"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import collections"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"from neutron_lib.api.definitions import portbindings"},{"line_number":19,"context_line":"from neutron_lib.api import extensions"},{"line_number":20,"context_line":"from neutron_lib.callbacks import events"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_e0dbfbe9","line":17,"updated":"2019-05-28 14:59:16.000000000","message":"don\u0027t think you need the blank line","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from neutron.objects import l3agent as rb_obj"},{"line_number":38,"context_line":"from neutron.plugins.ml2 import db as ml2_db"},{"line_number":39,"context_line":"from neutron.plugins.ml2 import models as ml2_models"},{"line_number":40,"context_line":""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_a04c23c8","line":40,"updated":"2019-05-28 14:59:16.000000000","message":"spurious change","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":126,"context_line":"        agent_port_host_match \u003d False"},{"line_number":127,"context_line":"        if unbound_migrate:"},{"line_number":128,"context_line":"            # This might be a case were it is migrating from"},{"line_number":129,"context_line":"            # unbound to a bound port."},{"line_number":130,"context_line":"            # In that case please forward the notification to the"},{"line_number":131,"context_line":"            # snat_nodes hosting the routers."},{"line_number":132,"context_line":"            # Make a call here to notify the snat nodes."}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_a0e1839d","line":129,"updated":"2019-05-28 14:59:16.000000000","message":"please don\u0027t change lines unneccesarily, it can lead to problems with backports","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                if agent.host \u003d\u003d port_host:"},{"line_number":141,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":142,"context_line":"        if not agent_port_host_match:"},{"line_number":143,"context_line":"            # (ushen) save host - router relationship in a dict"},{"line_number":144,"context_line":"            # to avoid performance issue when looping through a"},{"line_number":145,"context_line":"            # large number of hosts, which is caused by"},{"line_number":146,"context_line":"            # get_ports method does cost at least half of second"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_40ec478e","line":143,"updated":"2019-05-28 14:59:16.000000000","message":"nit: you don\u0027t need to add your ($name) here, the git log will show who to blame :)","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"669ba673fd81b6207fb13c018401c994500fec3c","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            # large number of hosts, which is caused by"},{"line_number":146,"context_line":"            # get_ports method does cost at least half of second"},{"line_number":147,"context_line":"            # to complete so program should avoid calling this"},{"line_number":148,"context_line":"            # method inside a loop"},{"line_number":149,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_4f8a33bf","line":148,"updated":"2019-05-28 17:38:03.000000000","message":"Hi normal shen, as I mentioned earlier, we need to add a check here to see if the router has additional connected routers, ( connected_dvr_router_ids \u003d set(\n                    self._get_other_dvr_router_ids_connected_router(\n                        context, router_id)))\nif so then proceed with the code path below, else you need to just call the l3_rpc_notifier to updat the router.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"a028ecf76ec81eca8dbf51782e9b16d2dce83d69","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            # large number of hosts, which is caused by"},{"line_number":146,"context_line":"            # get_ports method does cost at least half of second"},{"line_number":147,"context_line":"            # to complete so program should avoid calling this"},{"line_number":148,"context_line":"            # method inside a loop"},{"line_number":149,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_d0379bfe","line":148,"in_reply_to":"bfb3d3c7_4f8a33bf","updated":"2019-05-29 00:50:52.000000000","message":"I do not understand. First why just call l3_rpc_notifier is fine if router is not connected to others. Second, can you give me an example where connected dvr routers are not locating on the same host? Thanks","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"77a190e122fe62f29be13f7161516dcad1b77a49","unresolved":false,"context_lines":[{"line_number":145,"context_line":"            # large number of hosts, which is caused by"},{"line_number":146,"context_line":"            # get_ports method does cost at least half of second"},{"line_number":147,"context_line":"            # to complete so program should avoid calling this"},{"line_number":148,"context_line":"            # method inside a loop"},{"line_number":149,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_e16a137f","line":148,"in_reply_to":"bfb3d3c7_d0379bfe","updated":"2019-05-29 05:13:07.000000000","message":"Swami, I believe we still need to send notification to all agents hosting routers connected to port\u0027s subnets - this is what below code is doing. So I\u0027m not sure what could be skipped here.\nConnected routers will be handled in get_hosts_to_notify().","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"669ba673fd81b6207fb13c018401c994500fec3c","unresolved":false,"context_lines":[{"line_number":148,"context_line":"            # method inside a loop"},{"line_number":149,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_efc447f7","line":151,"range":{"start_line":151,"start_character":22,"end_line":151,"end_character":31},"updated":"2019-05-28 17:38:03.000000000","message":"dest_host does not have any effect in this code path. It denotes the VM that is migrating from old host to a new host (dest_host)","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"77a190e122fe62f29be13f7161516dcad1b77a49","unresolved":false,"context_lines":[{"line_number":148,"context_line":"            # method inside a loop"},{"line_number":149,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_610ce3eb","line":151,"range":{"start_line":151,"start_character":22,"end_line":151,"end_character":31},"in_reply_to":"bfb3d3c7_efc447f7","updated":"2019-05-29 05:13:07.000000000","message":"this code comment is unclear to me either.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"fa6f5f5935650f271280d0e13842f36bab4440c9","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_6edf073a","line":153,"range":{"start_line":153,"start_character":15,"end_line":153,"end_character":24},"updated":"2019-05-28 04:32:17.000000000","message":"Can this be None?","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_8069dff9","line":153,"range":{"start_line":153,"start_character":15,"end_line":153,"end_character":24},"in_reply_to":"bfb3d3c7_0999a5c2","updated":"2019-05-28 14:59:16.000000000","message":"Right, I don\u0027t think port_host can be None.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"3824565ee7939eeb3ecc71ed4f304e2d98352161","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_0999a5c2","line":153,"range":{"start_line":153,"start_character":15,"end_line":153,"end_character":24},"in_reply_to":"bfb3d3c7_6edf073a","updated":"2019-05-28 06:18:25.000000000","message":"Hi, I believe port_host cannot be none because of https://github.com/openstack/neutron/blob/23e3213a07eb0b0fcdd2a1da36a847dde9beba57/neutron/db/l3_dvrscheduler_db.py#L99. I definitely need to fix this one.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"f72b33de283d537714ae2a26219b7584627dc0ed","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_2e234fbf","line":153,"range":{"start_line":153,"start_character":15,"end_line":153,"end_character":24},"in_reply_to":"bfb3d3c7_6edf073a","updated":"2019-05-28 05:59:08.000000000","message":"Honestly, I\u0027m not sure. But it seems to me this value might be None if port is created and updated without specifying a binding:host_id. Please correct me if I miss something.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"669ba673fd81b6207fb13c018401c994500fec3c","unresolved":false,"context_lines":[{"line_number":150,"context_line":""},{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_4fb31399","line":153,"range":{"start_line":153,"start_character":15,"end_line":153,"end_character":24},"in_reply_to":"bfb3d3c7_8069dff9","updated":"2019-05-28 17:38:03.000000000","message":"port_host will not be None.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"0a1fc9c81986f6850c84d2d7d9a96bac9e559c8a","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_0e968b4d","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"updated":"2019-05-28 04:38:27.000000000","message":"Do all routers from routers_id list belong to host port_host? Anyway seems this step is redundant - the loop at #156-161 should ensure to add all routers for each host, shouldn\u0027t it?","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"cefaea9e3b985eabc089310db148a877ceb63eff","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_b18c7bd7","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_0923e557","updated":"2019-05-28 10:23:21.000000000","message":"I think this code snippet is way complicated than I thought. Please look at this neutron.tests.unit.scheduler.test_l3_agent_scheduler.L3DvrSchedulerTestCase#test_dvr_handle_new_service_port test case. Maybe I\u0027d better keep this untouched..","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"8b65031f48c393d5de7633e5c58d7dafb69d368d","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_514dff97","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_0923e557","updated":"2019-05-28 10:28:34.000000000","message":"more specifically in test case, get_hosts_to_notify() will only return other_host instead of both other_host and host1. So I think there must be something I am missing.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"f72b33de283d537714ae2a26219b7584627dc0ed","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_8e1b5b7f","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_0e968b4d","updated":"2019-05-28 05:59:08.000000000","message":"Correct me if i\u0027m wrong. Is that possible that dest_port and port[portbindings.HOST_ID] are different. For example,at here https://github.com/openstack/neutron/blob/23e3213a07eb0b0fcdd2a1da36a847dde9beba57/neutron/db/l3_dvrscheduler_db.py#L568, it looks during migration, dest_port will possibly differ from port[portbindings.HOST_ID]. But this piece of code could definitely be redundant","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"e57bbc61cfbff1d647266f63454ec7bae80d568c","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_05f8a178","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_4543f913","updated":"2019-05-28 13:57:41.000000000","message":"Thanks. I still need to figure why some functional tests failed.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"151b0d46384edc5bb592877f77eb50bb7168c50e","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_4543f913","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_514dff97","updated":"2019-05-28 13:39:09.000000000","message":"I think the that test case might not be quite correct: if port is bound, get_hosts_to_notify should return it\u0027s host. But I\u0027ll let Slawek and Swami (authors of the code being changed here) to comment on this.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"08463f325664376201ff96434d0be879d7fbb047","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_a90bb9d3","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_8e1b5b7f","updated":"2019-05-28 06:38:24.000000000","message":"If this situation happens, then dest_port should not send all router ids. So think this two lines are indeed redundant.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"745933952ee38f14f8f8c7c50dc8b28d4e372ab9","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_49579dcb","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_8e1b5b7f","updated":"2019-05-28 06:37:22.000000000","message":"ah, yes, you\u0027re right, I missed the dest_host case. So please keep this","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"9904afd2d8dcd74ef5609babda4cabeb7886c816","unresolved":false,"context_lines":[{"line_number":151,"context_line":"            # (ushen) dest_host should handle all router_ids"},{"line_number":152,"context_line":"            # which are on the current port[portbindings.HOST_ID]"},{"line_number":153,"context_line":"            if port_host:"},{"line_number":154,"context_line":"                host_routers[port_host] \u003d set(router_ids)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"            for router_id in router_ids:"},{"line_number":157,"context_line":"                for host in self.get_hosts_to_notify(context, router_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_0923e557","line":154,"range":{"start_line":154,"start_character":16,"end_line":154,"end_character":57},"in_reply_to":"bfb3d3c7_a90bb9d3","updated":"2019-05-28 06:42:20.000000000","message":"I believe we better keep it as get_hosts_to_notify() later will not account for dest_host in case port migration","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"ba79d07ebdd6086c900b07c528593dbcc0c7f61c","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":159,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":160,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":161,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"            for host, routers in host_routers.items():"},{"line_number":164,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_86a258fb","line":161,"updated":"2019-05-28 14:59:16.000000000","message":"I don\u0027t immediately see how this is equivalent to the old code using _check_for_rtr_serviceable_ports()","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6f1d2b7515ed58c3f40d16eb73fe5c31bf67ad13","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":159,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":160,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":161,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"            for host, routers in host_routers.items():"},{"line_number":164,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_01a727a9","line":161,"in_reply_to":"bfb3d3c7_6f75b7ca","updated":"2019-05-29 05:21:32.000000000","message":"If look at _check_for_rtr_serviceable_ports() and _get_dvr_hosts_for_router() can see that they are doing almost the same, except that the latter just returns all hosts with dvr serviceable ports for a router. get_hosts_to_notify() does include call to _get_dvr_hosts_for_router() but also returns snat hosts and hosts for interconnected routers.\n\nSo original code first calls get_hosts_to_notify() for each router and then filters only hosts with dvr serviceable ports again for each router. This is kind of not optimal. We can just call _get_dvr_hosts_for_router() once for each port\u0027s router, which I suggest this patch should do.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"669ba673fd81b6207fb13c018401c994500fec3c","unresolved":false,"context_lines":[{"line_number":158,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":159,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":160,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":161,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"            for host, routers in host_routers.items():"},{"line_number":164,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_6f75b7ca","line":161,"in_reply_to":"bfb3d3c7_86a258fb","updated":"2019-05-28 17:38:03.000000000","message":"_check_for_rtr_serviceable_ports is required here, since we are dealing with connected routers, and only if there is a valid service port on the next compute where the other end of the router is, we need to update the router otherwise, we can wait until the service port is created on the other end.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"929042d5470a73bcd6d9c74be20874c4ea39f405","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        \"\"\"Returns all hosts to send notification about router update\"\"\""},{"line_number":311,"context_line":"        hosts \u003d super(L3_DVRsch_db_mixin, self).get_hosts_to_notify("},{"line_number":312,"context_line":"            context, router_id)"},{"line_number":313,"context_line":"        router \u003d self.get_router(context.elevated(), router_id)"},{"line_number":314,"context_line":"        if router.get(\u0027distributed\u0027, False):"},{"line_number":315,"context_line":"            dvr_hosts \u003d self._get_dvr_hosts_for_router(context, router_id)"},{"line_number":316,"context_line":"            dvr_hosts \u003d set(dvr_hosts) - set(hosts)"},{"line_number":317,"context_line":"            dvr_hosts |\u003d self._get_other_dvr_hosts(context, router_id)"},{"line_number":318,"context_line":"            state \u003d agentschedulers_db.get_admin_state_up_filter()"},{"line_number":319,"context_line":"            agents \u003d self.get_l3_agents(context, active\u003dstate,"},{"line_number":320,"context_line":"                                        filters\u003d{\u0027host\u0027: dvr_hosts})"},{"line_number":321,"context_line":"            hosts +\u003d [a.host for a in agents]"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        return hosts"},{"line_number":324,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_207b537b","line":321,"range":{"start_line":313,"start_character":8,"end_line":321,"end_character":45},"updated":"2019-05-28 14:14:58.000000000","message":"consider refactoring this part into separate get_dvr_hosts_to_notify() and use it at line #157, thus original behavior should be preserved and functional tests should pass.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"6f1d2b7515ed58c3f40d16eb73fe5c31bf67ad13","unresolved":false,"context_lines":[{"line_number":310,"context_line":"        \"\"\"Returns all hosts to send notification about router update\"\"\""},{"line_number":311,"context_line":"        hosts \u003d super(L3_DVRsch_db_mixin, self).get_hosts_to_notify("},{"line_number":312,"context_line":"            context, router_id)"},{"line_number":313,"context_line":"        router \u003d self.get_router(context.elevated(), router_id)"},{"line_number":314,"context_line":"        if router.get(\u0027distributed\u0027, False):"},{"line_number":315,"context_line":"            dvr_hosts \u003d self._get_dvr_hosts_for_router(context, router_id)"},{"line_number":316,"context_line":"            dvr_hosts \u003d set(dvr_hosts) - set(hosts)"},{"line_number":317,"context_line":"            dvr_hosts |\u003d self._get_other_dvr_hosts(context, router_id)"},{"line_number":318,"context_line":"            state \u003d agentschedulers_db.get_admin_state_up_filter()"},{"line_number":319,"context_line":"            agents \u003d self.get_l3_agents(context, active\u003dstate,"},{"line_number":320,"context_line":"                                        filters\u003d{\u0027host\u0027: dvr_hosts})"},{"line_number":321,"context_line":"            hosts +\u003d [a.host for a in agents]"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"        return hosts"},{"line_number":324,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"bfb3d3c7_41caff4d","line":321,"range":{"start_line":313,"start_character":8,"end_line":321,"end_character":45},"in_reply_to":"bfb3d3c7_207b537b","updated":"2019-05-29 05:21:32.000000000","message":"please disregard this, just need to use _get_dvr_hosts_for_router() at line #157 to preserve original behavior.","commit_id":"caeffe2d726fe4c491d599c8040d9ded326b550f"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"208cae160821d1d91f00a431c354ba06e691eef9","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            for router_id in router_ids:"},{"line_number":149,"context_line":"                # get_subnet_ids_on_router is slow because it"},{"line_number":150,"context_line":"                # triggers get_ports internally, extract it out"},{"line_number":151,"context_line":"                # of host loop will save some time, especially"},{"line_number":152,"context_line":"                # not a log of routers are used"},{"line_number":153,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":154,"context_line":"                    context.elevated(), router_id"},{"line_number":155,"context_line":"                )"},{"line_number":156,"context_line":"                for host in hosts:"},{"line_number":157,"context_line":"                    # from my debug this method returns quickly"},{"line_number":158,"context_line":"                    # although there might be a way to completely"},{"line_number":159,"context_line":"                    # remove this call"},{"line_number":160,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":161,"context_line":"                        context.elevated(), host, subnet_ids"},{"line_number":162,"context_line":"                    ):"},{"line_number":163,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"            for host, routers in host_routers.items():"},{"line_number":166,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":167,"context_line":"                    context, routers, host"},{"line_number":168,"context_line":"                )"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":171,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_01d5e7e4","line":168,"range":{"start_line":143,"start_character":12,"end_line":168,"end_character":17},"updated":"2019-05-29 05:24:33.000000000","message":"please check my comments on PS5: need to use _get_dvr_hosts_for_router() instead of get_hosts_to_notify and exclude _check_dvr_serviceable_ports_on_host()","commit_id":"6bc1f431ba0e8466dd0fe06406e03a65300789a4"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"a81ee83accdb2f5271e56d6d2aaa92b3acf4b57c","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            for router_id in router_ids:"},{"line_number":149,"context_line":"                # get_subnet_ids_on_router is slow because it"},{"line_number":150,"context_line":"                # triggers get_ports internally, extract it out"},{"line_number":151,"context_line":"                # of host loop will save some time, especially"},{"line_number":152,"context_line":"                # not a log of routers are used"},{"line_number":153,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":154,"context_line":"                    context.elevated(), router_id"},{"line_number":155,"context_line":"                )"},{"line_number":156,"context_line":"                for host in hosts:"},{"line_number":157,"context_line":"                    # from my debug this method returns quickly"},{"line_number":158,"context_line":"                    # although there might be a way to completely"},{"line_number":159,"context_line":"                    # remove this call"},{"line_number":160,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":161,"context_line":"                        context.elevated(), host, subnet_ids"},{"line_number":162,"context_line":"                    ):"},{"line_number":163,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"            for host, routers in host_routers.items():"},{"line_number":166,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":167,"context_line":"                    context, routers, host"},{"line_number":168,"context_line":"                )"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":171,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_52808bbf","line":168,"range":{"start_line":143,"start_character":12,"end_line":168,"end_character":17},"in_reply_to":"bfb3d3c7_01d5e7e4","updated":"2019-05-29 09:04:37.000000000","message":"Done","commit_id":"6bc1f431ba0e8466dd0fe06406e03a65300789a4"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"374f12529b9b0697939011442e3e10965eb0e9a0","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            for router_id in router_ids:"},{"line_number":149,"context_line":"                # get_subnet_ids_on_router is slow because it"},{"line_number":150,"context_line":"                # triggers get_ports internally, extract it out"},{"line_number":151,"context_line":"                # of host loop will save some time, especially"},{"line_number":152,"context_line":"                # not a log of routers are used"},{"line_number":153,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":154,"context_line":"                    context.elevated(), router_id"},{"line_number":155,"context_line":"                )"},{"line_number":156,"context_line":"                for host in hosts:"},{"line_number":157,"context_line":"                    # from my debug this method returns quickly"},{"line_number":158,"context_line":"                    # although there might be a way to completely"},{"line_number":159,"context_line":"                    # remove this call"},{"line_number":160,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":161,"context_line":"                        context.elevated(), host, subnet_ids"},{"line_number":162,"context_line":"                    ):"},{"line_number":163,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"            for host, routers in host_routers.items():"},{"line_number":166,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":167,"context_line":"                    context, routers, host"},{"line_number":168,"context_line":"                )"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":171,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_413d1f29","line":168,"range":{"start_line":143,"start_character":12,"end_line":168,"end_character":17},"in_reply_to":"bfb3d3c7_01d5e7e4","updated":"2019-05-29 05:31:50.000000000","message":"I am not wandering what if port has a `migration_to` host in profile which is not among hosts returned by _get_dvr_hosts_for_router()? Is that possible? Cause, in _check_dvr_serviceable_ports_on_host, I see it also allows host appearing in binding profile","commit_id":"6bc1f431ba0e8466dd0fe06406e03a65300789a4"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"23bfe67b467b55e8994f9db7a14632b6499ac17a","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            for router_id in router_ids:"},{"line_number":149,"context_line":"                # get_subnet_ids_on_router is slow because it"},{"line_number":150,"context_line":"                # triggers get_ports internally, extract it out"},{"line_number":151,"context_line":"                # of host loop will save some time, especially"},{"line_number":152,"context_line":"                # not a log of routers are used"},{"line_number":153,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":154,"context_line":"                    context.elevated(), router_id"},{"line_number":155,"context_line":"                )"},{"line_number":156,"context_line":"                for host in hosts:"},{"line_number":157,"context_line":"                    # from my debug this method returns quickly"},{"line_number":158,"context_line":"                    # although there might be a way to completely"},{"line_number":159,"context_line":"                    # remove this call"},{"line_number":160,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":161,"context_line":"                        context.elevated(), host, subnet_ids"},{"line_number":162,"context_line":"                    ):"},{"line_number":163,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"            for host, routers in host_routers.items():"},{"line_number":166,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":167,"context_line":"                    context, routers, host"},{"line_number":168,"context_line":"                )"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":171,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_a132db0e","line":168,"range":{"start_line":143,"start_character":12,"end_line":168,"end_character":17},"in_reply_to":"bfb3d3c7_01df27d6","updated":"2019-05-29 06:14:07.000000000","message":"For each migrating port dvr_handle_new_service_port() will be called","commit_id":"6bc1f431ba0e8466dd0fe06406e03a65300789a4"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"30b88ef2f1c999533bbaf66253b9ddeda2f4281a","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            for router_id in router_ids:"},{"line_number":149,"context_line":"                # get_subnet_ids_on_router is slow because it"},{"line_number":150,"context_line":"                # triggers get_ports internally, extract it out"},{"line_number":151,"context_line":"                # of host loop will save some time, especially"},{"line_number":152,"context_line":"                # not a log of routers are used"},{"line_number":153,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":154,"context_line":"                    context.elevated(), router_id"},{"line_number":155,"context_line":"                )"},{"line_number":156,"context_line":"                for host in hosts:"},{"line_number":157,"context_line":"                    # from my debug this method returns quickly"},{"line_number":158,"context_line":"                    # although there might be a way to completely"},{"line_number":159,"context_line":"                    # remove this call"},{"line_number":160,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":161,"context_line":"                        context.elevated(), host, subnet_ids"},{"line_number":162,"context_line":"                    ):"},{"line_number":163,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"            for host, routers in host_routers.items():"},{"line_number":166,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":167,"context_line":"                    context, routers, host"},{"line_number":168,"context_line":"                )"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":171,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_618063c0","line":168,"range":{"start_line":143,"start_character":12,"end_line":168,"end_character":17},"in_reply_to":"bfb3d3c7_413d1f29","updated":"2019-05-29 05:34:38.000000000","message":"Right, dest_host should be added explicitly, just like in you patch set 5, except that we don\u0027t need check for None.","commit_id":"6bc1f431ba0e8466dd0fe06406e03a65300789a4"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"e7656c01f941a7f558c883614d0bec609acbdd16","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"            for router_id in router_ids:"},{"line_number":149,"context_line":"                # get_subnet_ids_on_router is slow because it"},{"line_number":150,"context_line":"                # triggers get_ports internally, extract it out"},{"line_number":151,"context_line":"                # of host loop will save some time, especially"},{"line_number":152,"context_line":"                # not a log of routers are used"},{"line_number":153,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":154,"context_line":"                    context.elevated(), router_id"},{"line_number":155,"context_line":"                )"},{"line_number":156,"context_line":"                for host in hosts:"},{"line_number":157,"context_line":"                    # from my debug this method returns quickly"},{"line_number":158,"context_line":"                    # although there might be a way to completely"},{"line_number":159,"context_line":"                    # remove this call"},{"line_number":160,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":161,"context_line":"                        context.elevated(), host, subnet_ids"},{"line_number":162,"context_line":"                    ):"},{"line_number":163,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"            for host, routers in host_routers.items():"},{"line_number":166,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":167,"context_line":"                    context, routers, host"},{"line_number":168,"context_line":"                )"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":171,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":6,"id":"bfb3d3c7_01df27d6","line":168,"range":{"start_line":143,"start_character":12,"end_line":168,"end_character":17},"in_reply_to":"bfb3d3c7_618063c0","updated":"2019-05-29 05:48:20.000000000","message":"But what about those ports in subnet which does not finish migrating? In that case, hosts in those ports might also be valid candidate to send notifications.","commit_id":"6bc1f431ba0e8466dd0fe06406e03a65300789a4"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"c1fde14bc80e0cd4aa76a5f50781549afd879106","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_c37b5025","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"updated":"2019-05-29 10:46:58.000000000","message":"LGTM, now the logic should preserve original behavior.\n\nThe only thing I\u0027d like to clarify with Swami is why at all should we send notification to all agents hosting routers? In particular what action should agents take after receiving the notification? Asking Swami as this logic was added after comment here: https://review.opendev.org/#/c/597567/6/neutron/db/l3_dvrscheduler_db.py","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"cd33a459dc9291ddd79bea46cf91a13d4ef703a2","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_2037b74f","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_0937494f","updated":"2019-05-29 18:09:06.000000000","message":"Might be we did not handle the dest_host case for the connected router case.","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"358c8aece882bc658b62405383c834e13c092dd4","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_c39a4912","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_224ec637","updated":"2019-05-31 05:38:41.000000000","message":"Could you please give me one example except migration that port profile could be filled with a `host` value? From this https://github.com/openstack/neutron/blob/36428d60a643ee6c8a6a7a7c0cb649a4969047df/neutron/db/l3_dvrscheduler_db.py#L487, it seems that the only possible situation for sending to connected router host except those already available ones is port got a profile containing `host` which only hosting connected router. If port profile is only updated during migration, then this implies this new host will eventually be updated by another call, except that you want to send notifications to connected routers every time port is updated?","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"d6599232379c22f1745c1f15f6ae3372467b6e99","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_c2eb2a10","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_b5e520de","updated":"2019-05-30 18:03:20.000000000","message":"The reason we check for the serviceable port on the router is because, only if there is a dvr serviceable port associated with the router, then we go ahead and create the connected routers, otherwise, it is of no use, just to have the connected routers namespace in place.","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"8484846a8a94418210cfcdbe0a26ac6c2290fb7a","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_b5e520de","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_c0fc9b71","updated":"2019-05-29 23:53:52.000000000","message":"Hi swami, I am wandering if we really need to send notifications to `all` connected routers host, then I believe you do not need to check serviceable router. Right now I believe you only send notification if connected routers\u0027 host is in port profile. Is it correct?","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"68bf1840b9b200c2f8cb10a4d661fc300c77db05","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_f6d885ef","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_c0fc9b71","updated":"2019-05-29 19:57:44.000000000","message":"Ok, just trying to understand, is there a case that new namespace should be created anywhere except new port\u0027s host? In other words, shouldn\u0027t we just send notification with all connected routers to only new port\u0027s host?","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2d240a5a4478ad71361f12de22c0b5c3a5edd28c","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_0937494f","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_c37b5025","updated":"2019-05-29 12:56:09.000000000","message":"Moreover the code on the left side might not send notification to dest_host in case of port migration - because _check_for_rtr_serviceable_ports might return False for this host. This was broken in https://review.opendev.org/#/c/597567/41/neutron/db/l3_dvrscheduler_db.py","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"cd33a459dc9291ddd79bea46cf91a13d4ef703a2","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_c0fc9b71","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_c37b5025","updated":"2019-05-29 18:09:06.000000000","message":"Oleg, if there are connected routers in place, then we should send notification to all connected routers host, so that they receive this message and create the necessary namespaces.","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"077a0922b51423975ed62de8dd1ae1dc2eb23df1","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_224ec637","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_e2898e74","updated":"2019-05-30 18:09:13.000000000","message":"Just check this two pastebin logs for what I did for testing it. ( The second link is the continuation of the first one)\nhttp://paste.openstack.org/show/733078/ and\nhttp://paste.openstack.org/show/733079/","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"d6599232379c22f1745c1f15f6ae3372467b6e99","unresolved":false,"context_lines":[{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"},{"line_number":151,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":152,"context_line":"                    context, routers, host"},{"line_number":153,"context_line":"                )"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":156,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_e2898e74","line":153,"range":{"start_line":145,"start_character":12,"end_line":153,"end_character":17},"in_reply_to":"bfb3d3c7_f6d885ef","updated":"2019-05-30 18:03:20.000000000","message":"I don\u0027t exactly remember it. I have re-evaluate it.","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"7299a519e579786068605db17fd840d1084328a1","unresolved":false,"context_lines":[{"line_number":355,"context_line":"            Port.device_owner.in_("},{"line_number":356,"context_line":"                n_utils.get_other_dvr_serviced_device_owners()))"},{"line_number":357,"context_line":"        query \u003d query.filter(owner_filter)"},{"line_number":358,"context_line":"        hosts \u003d [item[0] for item in query]"},{"line_number":359,"context_line":"        return hosts"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def _get_dvr_subnet_ids_on_host_query(self, context, host):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_0f3d09dc","line":358,"range":{"start_line":358,"start_character":8,"end_line":358,"end_character":43},"updated":"2019-05-29 14:31:59.000000000","message":"Please change this to \"hosts \u003d [item[0] for item in query if item[0] !\u003d \u0027\u0027]\". Functional test log has \"Hosts for router 4aaf4071-3de4-4376-bc05-78cebb192867: [\u0027host1\u0027, \u0027\u0027]\" - so empty host is being added by mistake.","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"39973d554a7f36452caf3b8478524d08bd9d4806","unresolved":false,"context_lines":[{"line_number":355,"context_line":"            Port.device_owner.in_("},{"line_number":356,"context_line":"                n_utils.get_other_dvr_serviced_device_owners()))"},{"line_number":357,"context_line":"        query \u003d query.filter(owner_filter)"},{"line_number":358,"context_line":"        hosts \u003d [item[0] for item in query]"},{"line_number":359,"context_line":"        return hosts"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"    def _get_dvr_subnet_ids_on_host_query(self, context, host):"}],"source_content_type":"text/x-python","patch_set":7,"id":"bfb3d3c7_d50f7485","line":358,"range":{"start_line":358,"start_character":8,"end_line":358,"end_character":43},"in_reply_to":"bfb3d3c7_0f3d09dc","updated":"2019-05-29 23:54:26.000000000","message":"Done","commit_id":"fc715a711992c0ff94755145045cb700dc9e818d"},{"author":{"_account_id":7016,"name":"Swaminathan Vasudevan","email":"swvasude@cisco.com","username":"souminathan"},"change_message_id":"1ecc83acf13dda772b7d751635dc7f99819a50df","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                if agent.host \u003d\u003d port_host:"},{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_4580a4f5","line":142,"updated":"2019-05-30 18:41:49.000000000","message":"I still believe that you need to check if there are any connected routers, before you go through the code snippet in #L143 to L153.\n\nWhat I suggest is:\n\nif len(\n               self._get_other_dvr_router_ids_connected_router(\n                        context, router_id)) \u003e 1:\n  then proceed with #L143 to L153\nelse:\n      LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027\n                      \u0027router ids %(router_ids)s\u0027,\n                {\u0027host\u0027: port_host, \u0027router_ids\u0027: router_ids})\n            self.l3_rpc_notifier.routers_updated_on_host(\n                context, router_ids, port_host)","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"1e078b354d56005cd51a0ed373c1af28bfcf9de1","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                if agent.host \u003d\u003d port_host:"},{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_8364424b","line":142,"in_reply_to":"bfb3d3c7_28277ccc","updated":"2019-05-31 16:11:35.000000000","message":"Maybe you can take look at this bug, it is the case 5. you described upper:\nhttps://bugs.launchpad.net/neutron/+bug/1786272\nAnd the commit is here which changed the code to current behavior left side:\nhttps://review.opendev.org/#/c/597567/41/neutron/db/l3_dvrscheduler_db.py\n\nAnd there are some test scripts for the bug 1786272, maybe you can confirm this change does not break that. One script:\nhttp://paste.openstack.org/show/732964/","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"32095463b6cdd31648a5b8a9b6f04e623faa8743","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                if agent.host \u003d\u003d port_host:"},{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_28277ccc","line":142,"in_reply_to":"bfb3d3c7_4580a4f5","updated":"2019-05-31 09:26:52.000000000","message":"Besides, is it possible to have partial connected router? For example, R1 and R2 are connected, is it possible that only R1 or R2 exist on some compute node? If no, then I am still confusing why we need #L143 to #L153?","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"358c8aece882bc658b62405383c834e13c092dd4","unresolved":false,"context_lines":[{"line_number":139,"context_line":"                if agent.host \u003d\u003d port_host:"},{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_2327a5cc","line":142,"in_reply_to":"bfb3d3c7_4580a4f5","updated":"2019-05-31 05:38:41.000000000","message":"this confuses me even more.\n1. why do not we need to send port update notification to all hosts if router is not connected?\n2. do we need to send notifications to host where connected routers already co-exist? cause, there is chance that they already live together.\n3. let\u0027s see user manually update port profile and add a `host1` there, but vm is not migrating at all, and there is a standalone connected router on `host1` as well. Then do we need to send notification to `host1` as well?\n4. If we only need to send notification to `host1` if \n  i. there is a standalone connected router\n  ii. vm is migrating there\n  iii. it hasn\u0027t be updated before\nthen I believe sending notification to `dest_port` to update `router_ids` are enough.\n5. what about situations where routers are not directed but eventually connected with each other? For example, Ra -- subnet 1 -- Rb -- subnet2 -- Rc, do we need to take care of Rc and its hosts as well?","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":7125,"name":"Alexander Ignatov","email":"aignatov@mirantis.com","username":"aignatov"},"change_message_id":"1ef97495ce5dfc426c1731191a6fd7a2871bb911","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_52d94f1f","line":147,"range":{"start_line":146,"start_character":12,"end_line":147,"end_character":79},"updated":"2019-05-30 13:29:19.000000000","message":"I guess debug log should be present here still for new update.","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"12b682a6d67be5e6ae002458978d173506b401be","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_887b01f3","line":147,"range":{"start_line":146,"start_character":12,"end_line":147,"end_character":79},"in_reply_to":"bfb3d3c7_52d94f1f","updated":"2019-05-31 14:36:19.000000000","message":"Done","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"358c8aece882bc658b62405383c834e13c092dd4","unresolved":false,"context_lines":[{"line_number":143,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"            host_routers[port_host] \u003d set(router_ids)"},{"line_number":146,"context_line":"            for router_id in router_ids:"},{"line_number":147,"context_line":"                for host in self._get_dvr_hosts_for_router(context, router_id):"},{"line_number":148,"context_line":"                    host_routers[host].add(router_id)"},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"            for host, routers in host_routers.items():"}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_6d44b847","line":147,"range":{"start_line":146,"start_character":12,"end_line":147,"end_character":79},"in_reply_to":"bfb3d3c7_52d94f1f","updated":"2019-05-31 05:38:41.000000000","message":"Yes, that\u0027s right","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"ac0f1bc6bde6f08b6d8d7a3862ecf365f3c10bdf","unresolved":false,"context_lines":[{"line_number":234,"context_line":"            info \u003d {\u0027router_id\u0027: router_id, \u0027host\u0027: port_host,"},{"line_number":235,"context_line":"                    \u0027agent_id\u0027: str(agent.id)}"},{"line_number":236,"context_line":"            removed_router_info.append(info)"},{"line_number":237,"context_line":"        # Now collect the connected router info as well to remove"},{"line_number":238,"context_line":"        # it from the agent, only if there is not a serviceable port."},{"line_number":239,"context_line":"        if not host_has_serviceable_port:"},{"line_number":240,"context_line":"            related_router_ids \u003d set()"},{"line_number":241,"context_line":"            for router_id in router_ids:"},{"line_number":242,"context_line":"                connected_dvr_router_ids \u003d set("}],"source_content_type":"text/x-python","patch_set":8,"id":"bfb3d3c7_c804a017","line":239,"range":{"start_line":237,"start_character":8,"end_line":239,"end_character":41},"updated":"2019-05-31 14:10:03.000000000","message":"I guess \"if get_related_hosts_info\" should be here, currently this parameter is not used. However as I see it is always passed as False, so logic is definitely broken. This deserves a separate patch.","commit_id":"d1e1b5e22320ce28db21b95a5f214fd4c31afde7"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"0c692c623344fced32fed5d3e11d43e809e993a3","unresolved":false,"context_lines":[{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"},{"line_number":151,"context_line":"                    router_id"},{"line_number":152,"context_line":"                )"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_f0a13197","line":149,"range":{"start_line":149,"start_character":34,"end_line":149,"end_character":58},"updated":"2019-06-01 02:35:26.000000000","message":"extract subnet ids from previous inner loop to avoid excessively calling get_ports when hosts are more than 100","commit_id":"65623e860ef5054ed70f1c09ace094bc7771fefa"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"0c692c623344fced32fed5d3e11d43e809e993a3","unresolved":false,"context_lines":[{"line_number":154,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":155,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":156,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":157,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":158,"context_line":"                        context.elavated(),"},{"line_number":159,"context_line":"                        host,"},{"line_number":160,"context_line":"                        subnet_ids"}],"source_content_type":"text/x-python","patch_set":10,"id":"9fb8cfa7_10c88553","line":157,"updated":"2019-06-01 02:35:26.000000000","message":"honestly, I personally doubt if this branch is necessary, but since it looks like it involves a lot of things. So I suggest keep this check.","commit_id":"65623e860ef5054ed70f1c09ace094bc7771fefa"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ce786703c6218a0ed40879163601074ed890a31f","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_3d9ea363","line":147,"range":{"start_line":147,"start_character":12,"end_line":147,"end_character":40},"updated":"2019-06-04 06:41:27.000000000","message":"This for loop body can merge to under line 143?","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"30a366a345b8881d6f6c322a255ce03c9b675771","unresolved":false,"context_lines":[{"line_number":140,"context_line":"                    agent_port_host_match \u003d True"},{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_95d75eee","line":147,"range":{"start_line":143,"start_character":0,"end_line":147,"end_character":40},"updated":"2019-06-04 14:37:55.000000000","message":"Why the following can not work?\n            host_routers \u003d collections.defaultdict(set)\n            for router_id in router_ids:\n                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))\n                # avoid calling get_ports in host loop\n                subnet_ids \u003d self.get_subnet_ids_on_router(\n                    context.elevated(), router_id)\n                for host in hosts:\n                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027\n                              \u0027router ids %(router_id)s\u0027,\n                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})\n                    if self._check_dvr_serviceable_ports_on_host(\n                            context.elevated(), host, subnet_ids):\n                        host_routers[host].add(router_id)","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"5b25f314aa64caf3627f5129066f02f6a4778a70","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_02ab3b1d","line":147,"range":{"start_line":147,"start_character":12,"end_line":147,"end_character":40},"in_reply_to":"9fb8cfa7_3d9ea363","updated":"2019-06-04 10:05:38.000000000","message":"besides, #L153 needs all hosts generated by #L144. So I think they cannot be merged.","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"0aa7e3d30b1e3cde78b6285f32f9e8d93431faf5","unresolved":false,"context_lines":[{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_0c6db297","line":147,"range":{"start_line":147,"start_character":12,"end_line":147,"end_character":40},"in_reply_to":"9fb8cfa7_3d9ea363","updated":"2019-06-04 08:07:20.000000000","message":"then I think I have to create a list holding all subnets for all routers. I guess this may not be ideal?","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ce786703c6218a0ed40879163601074ed890a31f","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"},{"line_number":151,"context_line":"                    router_id"},{"line_number":152,"context_line":"                )"},{"line_number":153,"context_line":"                for host in hosts:"},{"line_number":154,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":155,"context_line":"                              \u0027router ids %(router_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_9df54fce","line":152,"range":{"start_line":150,"start_character":20,"end_line":152,"end_character":17},"updated":"2019-06-04 06:41:27.000000000","message":"Nit: these new lines is not necessary:\n                subnet_ids \u003d self.get_subnet_ids_on_router(\n                    context.elevated(), router_id)","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"52b4f550c949b153cd21002a238ca007349968f1","unresolved":false,"context_lines":[{"line_number":147,"context_line":"            for router_id in router_ids:"},{"line_number":148,"context_line":"                # avoid calling get_ports in host loop"},{"line_number":149,"context_line":"                subnet_ids \u003d self.get_subnet_ids_on_router("},{"line_number":150,"context_line":"                    context.elevated(),"},{"line_number":151,"context_line":"                    router_id"},{"line_number":152,"context_line":"                )"},{"line_number":153,"context_line":"                for host in hosts:"},{"line_number":154,"context_line":"                    LOG.debug(\u0027DVR: Handle new service port, host %(host)s, \u0027"},{"line_number":155,"context_line":"                              \u0027router ids %(router_id)s\u0027,"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_e0613f3b","line":152,"range":{"start_line":150,"start_character":20,"end_line":152,"end_character":17},"in_reply_to":"9fb8cfa7_9df54fce","updated":"2019-06-04 11:41:28.000000000","message":"Done","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ce786703c6218a0ed40879163601074ed890a31f","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":156,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":157,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":158,"context_line":"                        context.elevated(),"},{"line_number":159,"context_line":"                        host,"},{"line_number":160,"context_line":"                        subnet_ids"},{"line_number":161,"context_line":"                    ):"},{"line_number":162,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            for host, router_ids in host_routers.items():"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_3d0223b6","line":161,"range":{"start_line":158,"start_character":24,"end_line":161,"end_character":22},"updated":"2019-06-04 06:41:27.000000000","message":"ditto","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"52b4f550c949b153cd21002a238ca007349968f1","unresolved":false,"context_lines":[{"line_number":155,"context_line":"                              \u0027router ids %(router_id)s\u0027,"},{"line_number":156,"context_line":"                              {\u0027host\u0027: host, \u0027router_id\u0027: router_id})"},{"line_number":157,"context_line":"                    if self._check_dvr_serviceable_ports_on_host("},{"line_number":158,"context_line":"                        context.elevated(),"},{"line_number":159,"context_line":"                        host,"},{"line_number":160,"context_line":"                        subnet_ids"},{"line_number":161,"context_line":"                    ):"},{"line_number":162,"context_line":"                        host_routers[host].add(router_id)"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            for host, router_ids in host_routers.items():"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_0067b34f","line":161,"range":{"start_line":158,"start_character":24,"end_line":161,"end_character":22},"in_reply_to":"9fb8cfa7_3d0223b6","updated":"2019-06-04 11:41:28.000000000","message":"Done","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"ce786703c6218a0ed40879163601074ed890a31f","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            for host, router_ids in host_routers.items():"},{"line_number":165,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":166,"context_line":"                    context,"},{"line_number":167,"context_line":"                    router_ids,"},{"line_number":168,"context_line":"                    host"},{"line_number":169,"context_line":"                )"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":172,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_5dffd7a9","line":169,"range":{"start_line":166,"start_character":20,"end_line":169,"end_character":17},"updated":"2019-06-04 06:41:27.000000000","message":"ditto","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"52b4f550c949b153cd21002a238ca007349968f1","unresolved":false,"context_lines":[{"line_number":163,"context_line":""},{"line_number":164,"context_line":"            for host, router_ids in host_routers.items():"},{"line_number":165,"context_line":"                self.l3_rpc_notifier.routers_updated_on_host("},{"line_number":166,"context_line":"                    context,"},{"line_number":167,"context_line":"                    router_ids,"},{"line_number":168,"context_line":"                    host"},{"line_number":169,"context_line":"                )"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    def get_dvr_snat_agent_list(self, context):"},{"line_number":172,"context_line":"        agent_filters \u003d {\u0027agent_modes\u0027: [n_const.L3_AGENT_MODE_DVR_SNAT]}"}],"source_content_type":"text/x-python","patch_set":12,"id":"9fb8cfa7_a05b4785","line":169,"range":{"start_line":166,"start_character":20,"end_line":169,"end_character":17},"in_reply_to":"9fb8cfa7_5dffd7a9","updated":"2019-06-04 11:41:28.000000000","message":"Done","commit_id":"3417fc32934849bc4a316a13c2ed17cf1ed86135"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"74a60c82bdee5ce02a2fd59682da5bb5b2b2097e","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_152dae42","line":144,"updated":"2019-06-04 14:30:36.000000000","message":"Why didn\u0027t we combine the below loop right here?  To not go through router_ids twice.  We would just have to move the host_routers above this.","commit_id":"00eb6f26f6165a00d647d2bf35fb7996534cfc09"},{"author":{"_account_id":29071,"name":"norman shen","email":"yshxxsjt715@gmail.com","username":"ushen"},"change_message_id":"541f332faadc76348ed8d417c0c1925e2849f5f6","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_5519e6d4","line":144,"in_reply_to":"9fb8cfa7_152dae42","updated":"2019-06-04 14:40:23.000000000","message":"Of course we could merge these two for loops. But it\u0027s not going to save code because you still need to check if host has been visited or not. And you may waste time on executing _check_dvr_serviceable_ports_on_host on the host which does not hold serviceable ports. So I prefer keep these loops separate.","commit_id":"00eb6f26f6165a00d647d2bf35fb7996534cfc09"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"6a72d0f80a35d1e3da6095d76fda68e07881354f","unresolved":false,"context_lines":[{"line_number":141,"context_line":"        if not agent_port_host_match:"},{"line_number":142,"context_line":"            hosts \u003d set([port_host])"},{"line_number":143,"context_line":"            for router_id in router_ids:"},{"line_number":144,"context_line":"                hosts |\u003d set(self.get_hosts_to_notify(context, router_id))"},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"            host_routers \u003d collections.defaultdict(set)"},{"line_number":147,"context_line":"            for router_id in router_ids:"}],"source_content_type":"text/x-python","patch_set":13,"id":"9fb8cfa7_f5e9fa7f","line":144,"in_reply_to":"9fb8cfa7_5519e6d4","updated":"2019-06-04 14:46:56.000000000","message":"Ah, maybe I see why.\n\nhost_routers \u003d collections.defaultdict(set)\nfor router_id in router_ids:\n    hosts |\u003d set(self.get_hosts_to_notify(context, router_id))\n    # avoid...\n    ...\n    for host in hosts:\n\nWe want all the hosts here, and if we move it we don\u0027t have it.\n\nNot enough coffee :)","commit_id":"00eb6f26f6165a00d647d2bf35fb7996534cfc09"}]}