)]}'
{"neutron_dynamic_routing/services/bgp/agent/bgp_dragent.py":[{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"cad015d8bfecd002a3df19752f17ebb1cae36af8","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"ef28fd1e_8c1690dc","line":118,"updated":"2023-11-07 12:24:58.000000000","message":"Doesn\u0027t this mean that the speaker is never removed (even if the message queue works) as long as `speaker_cache_timeout` is configured?","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"fcf6a4beff020be0330a8162a02fa08721ea0570","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"610d128c_804a0b2a","line":118,"in_reply_to":"134336ba_24077fb4","updated":"2023-11-07 16:02:22.000000000","message":"but isn\u0027t the whole point of this function to resynchronize against neutrons state in case we for some reason fell out of sync? In this case we need to remove the speaker if this has happened in the mean time and we lost the message (e.g. if the message has been sent by neutron to the queue, but expired before we picked it up).","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"77d6bac1abf171461ba90aca40eba5e7de445134","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"b09940a9_6ba3939b","line":118,"in_reply_to":"43a081a9_d798a4a6","updated":"2023-11-08 07:07:48.000000000","message":"I would propose to:\n1. Generally only block the deletion when the list of `hosted_bgp_speakers` is empty (allthough in this case i would rather expect an exception from get_bgp_speakers.\n2. Still drop the speakers after the cache timeout","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"212c312eb2b97f7dd4b763a1101ce6cca4625773","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"43a081a9_d798a4a6","line":118,"in_reply_to":"610d128c_804a0b2a","updated":"2023-11-07 16:55:32.000000000","message":"Good point! yes, this is exactly the point of this method. However,  we have no way of knowing whether the speaker list from RPC message is empty because some RMQ failed, lost the message, etc. or simply because it was removed during the RMQ outage.\n\nThe goal of this proposed change is exactly to keep the datapath (BGP advertised routes) out of problems with transient RMQ failures. I understand that messages can be lost, but in all the tests I\u0027ve done, when the RMQ comes back, the DRAgent receives a stop message from API (maybe I\u0027m missing something).\n\nWhat would be the suggestion to not remove the speaker? as this causes Network outage until the infrastructure returns to operation. Maybe start the timout timer to allow removing only after it expires for that case too?","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":29074,"name":"Felix Huettner","email":"felix.huettner@digits.schwarz","username":"felix.huettner"},"change_message_id":"c7312396c5de3d660912febc2a445eae5ea24bab","unresolved":true,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"8af02bd5_a858eab1","line":118,"in_reply_to":"879bd3b5_fe7240cf","updated":"2023-11-08 11:57:58.000000000","message":"I\u0027m not sure. If you just block the deletion on the empty list then you don\u0027t need the timeout at all. I think the timeout would mean that after this time we actually drop these entries? At least the docstring on the config suggest that","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"492459d52bf9d34e224bc61325f9406f67238af0","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"a2e4b4c4_db945539","line":118,"in_reply_to":"8af02bd5_a858eab1","updated":"2023-11-08 12:39:50.000000000","message":"It\u0027s not completely true as the peer can be removed by a full resync immediately after the RMQ comes back. That\u0027s why we need the timeout for the conditions proposed in the RFE. However, I\u0027ll propose applying the cache timeout timer for the BGP speaker deletion case too ;) thank you.","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"72c6da069ab354a7cc58bd9e7a6eae6da70c6c9c","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"879bd3b5_fe7240cf","line":118,"in_reply_to":"b09940a9_6ba3939b","updated":"2023-11-08 11:47:38.000000000","message":"Right, thanks for the feedback!\n\nIt makes sense to me, It\u0027s a great simple solution for this case. I agree about the exception but it doesn\u0027t occur in a very specific situation: when the RMQ comes back online and the get_bgp_speakers method was running waiting for response/timeout - it may be empty in this case due to transient issues in RMQ.\n\nJust to check if we\u0027re on the same page, even after cache timeout expires, the `hosted_bgp_speakers` empty list continues to be blocked for deletion, right? \n\nThanks","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"},{"author":{"_account_id":35432,"name":"Roberto Acosta","display_name":"rbartzen","email":"rbartzen@gmail.com","username":"rbartzen"},"change_message_id":"1a9eea94b6d0749880082a137c1a5ff9a8fbb822","unresolved":false,"context_lines":[{"line_number":115,"context_line":"                    # get_bgp_speakers may return an empty value when the"},{"line_number":116,"context_line":"                    # message queue service is offline. Wait for BGP speaker to"},{"line_number":117,"context_line":"                    # be removed via API requests from Neutron server."},{"line_number":118,"context_line":"                    if self.conf.speaker_cache_timeout \u003e 0:"},{"line_number":119,"context_line":"                        return"},{"line_number":120,"context_line":"                    self.remove_bgp_speaker_from_dragent(bgp_speaker_id)"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"134336ba_24077fb4","line":118,"in_reply_to":"ef28fd1e_8c1690dc","updated":"2023-11-07 13:24:52.000000000","message":"yeah, AFAIU the speaker should only be removed from DRAgent when an RPC message is received from Neutron API. Is there any reason this should happens automatically  (even if the message queue works) ?","commit_id":"171de9fc69f7f70aed814ec96097ed3ce3733f60"}]}