)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9531,"name":"liuyulong","display_name":"LIU Yulong","email":"i@liuyulong.me","username":"LIU-Yulong"},"change_message_id":"cc9f661efee94582d955cb652d357fca11f6257d","unresolved":false,"context_lines":[{"line_number":13,"context_line":""},{"line_number":14,"context_line":"In the event that an RPC fails, put the port IDs in the"},{"line_number":15,"context_line":"self.dhcp_ready_ports set for the worker thread, which"},{"line_number":16,"context_line":"will now throttle the number it sends to 64 at a time."},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Partially based on https://review.opendev.org/#/c/659274/"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fb8cfa7_283ee3b7","line":16,"range":{"start_line":16,"start_character":41,"end_line":16,"end_character":43},"updated":"2019-06-26 13:50:30.000000000","message":"That \u0027_dhcp_ready_ports_loop\u0027 is running in one thread only, not?","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"}],"neutron/agent/dhcp/agent.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"bdea4647560fc84e96d4996c9afc71a0a7e7aa9d","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                        network.id in only_nets):  # specific network to sync"},{"line_number":226,"context_line":"                    pool.spawn(self.safe_configure_dhcp_for_network, network)"},{"line_number":227,"context_line":"            pool.waitall()"},{"line_number":228,"context_line":"            # we notify all ports in case some were created while the agent"},{"line_number":229,"context_line":"            # was down"},{"line_number":230,"context_line":"            # why? we will get a network_update_end message if admin_state_up"},{"line_number":231,"context_line":"            # changes, which will add all the ports for this network to the"},{"line_number":232,"context_line":"            # cache and send status to the server for them"},{"line_number":233,"context_line":"            # self.dhcp_ready_ports |\u003d set(self.cache.get_port_ids(only_nets))"},{"line_number":234,"context_line":"            LOG.info(\u0027Synchronizing state complete\u0027)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_367aca76","line":233,"range":{"start_line":228,"start_character":0,"end_line":233,"end_character":78},"updated":"2019-06-26 08:37:04.000000000","message":"This seems unrelated to the bugfix. Maybe move it into its own change? Also I think deleting unnecessary code is better than commenting it out.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e3b3b8ba4ac826d6f5bd0a5e5a45d0af5456ffb","unresolved":false,"context_lines":[{"line_number":225,"context_line":"                        network.id in only_nets):  # specific network to sync"},{"line_number":226,"context_line":"                    pool.spawn(self.safe_configure_dhcp_for_network, network)"},{"line_number":227,"context_line":"            pool.waitall()"},{"line_number":228,"context_line":"            # we notify all ports in case some were created while the agent"},{"line_number":229,"context_line":"            # was down"},{"line_number":230,"context_line":"            # why? we will get a network_update_end message if admin_state_up"},{"line_number":231,"context_line":"            # changes, which will add all the ports for this network to the"},{"line_number":232,"context_line":"            # cache and send status to the server for them"},{"line_number":233,"context_line":"            # self.dhcp_ready_ports |\u003d set(self.cache.get_port_ids(only_nets))"},{"line_number":234,"context_line":"            LOG.info(\u0027Synchronizing state complete\u0027)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        except Exception as e:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_42fc5871","line":233,"range":{"start_line":228,"start_character":0,"end_line":233,"end_character":78},"in_reply_to":"9fb8cfa7_367aca76","updated":"2019-06-26 13:38:39.000000000","message":"Yes, I left it more as a comment to myself, since there might be a small window where a network is admin_state_up False and we somehow miss sending the port ID over.  The whole thing seems very inefficient since we could be adding things to the list that have already been sent, being a set() only helps if the item is still in it.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"6845e6f0e911d95f8ba2ff636a3df207893af4e9","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        \"\"\"Notifies the server of any ports that had reservations setup.\"\"\""},{"line_number":246,"context_line":"        try:"},{"line_number":247,"context_line":"            self.plugin_rpc.dhcp_ready_on_ports(ports_to_send)"},{"line_number":248,"context_line":"            return True"},{"line_number":249,"context_line":"        except oslo_messaging.MessagingTimeout:"},{"line_number":250,"context_line":"            LOG.error(\"Timeout notifying server of ports ready. \""},{"line_number":251,"context_line":"                      \"Retrying...\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_530af01f","line":248,"updated":"2019-06-26 07:02:44.000000000","message":"You can add here some debug log message about what ports were send already. It would be useful as in our last issue :)","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"bdea4647560fc84e96d4996c9afc71a0a7e7aa9d","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        \"\"\"Notifies the server of any ports that had reservations setup.\"\"\""},{"line_number":246,"context_line":"        try:"},{"line_number":247,"context_line":"            self.plugin_rpc.dhcp_ready_on_ports(ports_to_send)"},{"line_number":248,"context_line":"            return True"},{"line_number":249,"context_line":"        except oslo_messaging.MessagingTimeout:"},{"line_number":250,"context_line":"            LOG.error(\"Timeout notifying server of ports ready. \""},{"line_number":251,"context_line":"                      \"Retrying...\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_b6179aee","line":248,"in_reply_to":"9fb8cfa7_530af01f","updated":"2019-06-26 08:37:04.000000000","message":"I\u0027m not familiar with the last issue, so excuse me if this does not apply here, but putting the debug logs in the except cases would produce a better signal/noise ratio in the logs.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e3b3b8ba4ac826d6f5bd0a5e5a45d0af5456ffb","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        \"\"\"Notifies the server of any ports that had reservations setup.\"\"\""},{"line_number":246,"context_line":"        try:"},{"line_number":247,"context_line":"            self.plugin_rpc.dhcp_ready_on_ports(ports_to_send)"},{"line_number":248,"context_line":"            return True"},{"line_number":249,"context_line":"        except oslo_messaging.MessagingTimeout:"},{"line_number":250,"context_line":"            LOG.error(\"Timeout notifying server of ports ready. \""},{"line_number":251,"context_line":"                      \"Retrying...\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_2253646d","line":248,"in_reply_to":"9fb8cfa7_b6179aee","updated":"2019-06-26 13:38:39.000000000","message":"Bence - Slawek is referring to an issue we were debugging yesterday where a port was never transitioning to active but there were no errors being generated, we had to hack things to log the IDs.  It could be a lot of noise so I\u0027m not sure of the best solution.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"bdea4647560fc84e96d4996c9afc71a0a7e7aa9d","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            return True"},{"line_number":249,"context_line":"        except oslo_messaging.MessagingTimeout:"},{"line_number":250,"context_line":"            LOG.error(\"Timeout notifying server of ports ready. \""},{"line_number":251,"context_line":"                      \"Retrying...\")"},{"line_number":252,"context_line":"        except Exception:"},{"line_number":253,"context_line":"            LOG.exception(\"Failure notifying DHCP server of \""},{"line_number":254,"context_line":"                          \"ready DHCP ports. Will retry on next \""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_960a96cf","line":251,"updated":"2019-06-26 08:37:04.000000000","message":"A \u0027return False\u0027 (while technically superfluous) here would call the attention of the reader that the return value here  matters.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"bdea4647560fc84e96d4996c9afc71a0a7e7aa9d","unresolved":false,"context_lines":[{"line_number":252,"context_line":"        except Exception:"},{"line_number":253,"context_line":"            LOG.exception(\"Failure notifying DHCP server of \""},{"line_number":254,"context_line":"                          \"ready DHCP ports. Will retry on next \""},{"line_number":255,"context_line":"                          \"iteration.\")"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    def _dhcp_ready_ports_loop(self):"},{"line_number":258,"context_line":"        \"\"\"Notifies the server of any ports that had reservations setup.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_360baad5","line":255,"updated":"2019-06-26 08:37:04.000000000","message":"Here too.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e3b3b8ba4ac826d6f5bd0a5e5a45d0af5456ffb","unresolved":false,"context_lines":[{"line_number":252,"context_line":"        except Exception:"},{"line_number":253,"context_line":"            LOG.exception(\"Failure notifying DHCP server of \""},{"line_number":254,"context_line":"                          \"ready DHCP ports. Will retry on next \""},{"line_number":255,"context_line":"                          \"iteration.\")"},{"line_number":256,"context_line":""},{"line_number":257,"context_line":"    def _dhcp_ready_ports_loop(self):"},{"line_number":258,"context_line":"        \"\"\"Notifies the server of any ports that had reservations setup.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_a230b457","line":255,"in_reply_to":"9fb8cfa7_360baad5","updated":"2019-06-26 13:38:39.000000000","message":"I can put one here that would cover both cases.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":30314,"name":"Sebastian Lohff","email":"sebastian.lohff@sap.com","username":"seba"},"change_message_id":"75ec6d5161adb73d115613c84967c288ab0ea365","unresolved":false,"context_lines":[{"line_number":346,"context_line":"                    # If we fail to send all the ports, queue them to be"},{"line_number":347,"context_line":"                    # sent by the worker thread at it\u0027s next iteration."},{"line_number":348,"context_line":"                    ports_to_send \u003d {p.id for p in network.ports}"},{"line_number":349,"context_line":"                    if not self._send_dhcp_ready_ports(ports_to_send):"},{"line_number":350,"context_line":"                        self.dhcp_ready_ports |\u003d ports_to_send"},{"line_number":351,"context_line":"                break"},{"line_number":352,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_dca4ad4d","line":349,"range":{"start_line":349,"start_character":20,"end_line":349,"end_character":70},"updated":"2019-06-26 11:05:07.000000000","message":"Do I understand this correctly that this will do a blocking synchronous call to neutron-server when scheduling the network and setting up the network namespace on the dhcp-agent?\n\nOne big problem I have observed in our cloud setup is that the neutron-dhcp-agents take a long time to built up the network namespaces when we restart the dhcp-agent. On agent restart we need DHCP/DNS back as fast as possible, as it has customer impact if these services are missing. Therefore I\u0027d be against anything that slows down this process. I\u0027d rather have working DNS/DHCP as fast as possible and an extra thread concurrently taking care of getting the ports in ACTIVE mode.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e3b3b8ba4ac826d6f5bd0a5e5a45d0af5456ffb","unresolved":false,"context_lines":[{"line_number":346,"context_line":"                    # If we fail to send all the ports, queue them to be"},{"line_number":347,"context_line":"                    # sent by the worker thread at it\u0027s next iteration."},{"line_number":348,"context_line":"                    ports_to_send \u003d {p.id for p in network.ports}"},{"line_number":349,"context_line":"                    if not self._send_dhcp_ready_ports(ports_to_send):"},{"line_number":350,"context_line":"                        self.dhcp_ready_ports |\u003d ports_to_send"},{"line_number":351,"context_line":"                break"},{"line_number":352,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_22928458","line":349,"range":{"start_line":349,"start_character":20,"end_line":349,"end_character":70},"in_reply_to":"9fb8cfa7_dca4ad4d","updated":"2019-06-26 13:38:39.000000000","message":"Sebastian - yes, and I thought about this right after sending it out.  We have to find some balance between setting things up and notifying the server the ports are ready.  I don\u0027t know what that is - more worker threads sending RPC messages could be it, but then we\u0027d need synchronization between them.\n\nThanks everyone for the reviews though, maybe there is something here worth changing.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":26250,"name":"Johannes Kulik","email":"johannes.kulik@sap.com","username":"jkulik"},"change_message_id":"feab0c97e247b2fb8658fbccd3d8b4713ab81d62","unresolved":false,"context_lines":[{"line_number":347,"context_line":"                    # sent by the worker thread at it\u0027s next iteration."},{"line_number":348,"context_line":"                    ports_to_send \u003d {p.id for p in network.ports}"},{"line_number":349,"context_line":"                    if not self._send_dhcp_ready_ports(ports_to_send):"},{"line_number":350,"context_line":"                        self.dhcp_ready_ports |\u003d ports_to_send"},{"line_number":351,"context_line":"                break"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        self._resize_process_pool()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_7984b35e","line":350,"updated":"2019-06-26 09:10:32.000000000","message":"The problem I see here, is that a network can have more than `DHCP_READY_PORTS_SYNC_MAX` ports which would then hog the CPU on neutron-server unnecessarily. So maybe add a check for the size of `ports_to_send` to the `if`.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"2e3b3b8ba4ac826d6f5bd0a5e5a45d0af5456ffb","unresolved":false,"context_lines":[{"line_number":347,"context_line":"                    # sent by the worker thread at it\u0027s next iteration."},{"line_number":348,"context_line":"                    ports_to_send \u003d {p.id for p in network.ports}"},{"line_number":349,"context_line":"                    if not self._send_dhcp_ready_ports(ports_to_send):"},{"line_number":350,"context_line":"                        self.dhcp_ready_ports |\u003d ports_to_send"},{"line_number":351,"context_line":"                break"},{"line_number":352,"context_line":""},{"line_number":353,"context_line":"        self._resize_process_pool()"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fb8cfa7_e270ec42","line":350,"in_reply_to":"9fb8cfa7_7984b35e","updated":"2019-06-26 13:38:39.000000000","message":"Yes, I thought of doing that in _send_dhcp_ready_ports() so if this was a /16 it wouldn\u0027t run into the same problem, I\u0027ll send an update tweaking some things.","commit_id":"b37ab5b0d29ddf576ab6ba4e7be28fa3151111dd"}]}