)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"48d8832a6f2cfab836564868391fd93932b6717e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5969b0e2_1163d749","updated":"2022-10-05 05:56:03.000000000","message":"LGTM","commit_id":"858c92bfa843ff8dbff57835cc7c667deab63c05"}],"neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"00c1d4393a8d65ff90a698a6a49bb79528d54df5","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                                worker.MaintenanceWorker,"},{"line_number":340,"context_line":"                                service.RpcWorker)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    @lockutils.synchronized(\u0027hash_ring_probe_lock\u0027, external\u003dTrue)"},{"line_number":343,"context_line":"    def _start_hash_ring_probe(self):"},{"line_number":344,"context_line":"        if not self._hash_ring_probe_event.is_set():"},{"line_number":345,"context_line":"            self._hash_ring_thread \u003d maintenance.MaintenanceThread()"}],"source_content_type":"text/x-python","patch_set":2,"id":"d8363188_db128dbf","line":342,"range":{"start_line":342,"start_character":4,"end_line":342,"end_character":66},"updated":"2022-10-04 13:49:15.000000000","message":"I was going to suggest removing the synchronized decorator and use this:\n\n        if not self._hash_ring_probe_event.is_set():\n            self._hash_ring_probe_event.set()\n            \nBut I\u0027m not completely sure this guarantees only one process access to the \"if\" branch.","commit_id":"858c92bfa843ff8dbff57835cc7c667deab63c05"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"f0db18085cf5283ed2dd8d6afcae62e43eac2cba","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                                worker.MaintenanceWorker,"},{"line_number":340,"context_line":"                                service.RpcWorker)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    @lockutils.synchronized(\u0027hash_ring_probe_lock\u0027, external\u003dTrue)"},{"line_number":343,"context_line":"    def _start_hash_ring_probe(self):"},{"line_number":344,"context_line":"        if not self._hash_ring_probe_event.is_set():"},{"line_number":345,"context_line":"            self._hash_ring_thread \u003d maintenance.MaintenanceThread()"}],"source_content_type":"text/x-python","patch_set":2,"id":"28403f5c_a49a7cfd","line":342,"updated":"2022-10-04 17:17:41.000000000","message":"The external lock is filesystem based, right? So with multiple controllers, we\u0027ll have 4 different locks.","commit_id":"858c92bfa843ff8dbff57835cc7c667deab63c05"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"333a6540c19b7368426b4c9b100791e537b13569","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                                worker.MaintenanceWorker,"},{"line_number":340,"context_line":"                                service.RpcWorker)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    @lockutils.synchronized(\u0027hash_ring_probe_lock\u0027, external\u003dTrue)"},{"line_number":343,"context_line":"    def _start_hash_ring_probe(self):"},{"line_number":344,"context_line":"        if not self._hash_ring_probe_event.is_set():"},{"line_number":345,"context_line":"            self._hash_ring_thread \u003d maintenance.MaintenanceThread()"}],"source_content_type":"text/x-python","patch_set":2,"id":"a57d3668_affb4dd8","line":342,"in_reply_to":"28403f5c_a49a7cfd","updated":"2022-10-04 17:20:48.000000000","message":"I think we had a periodic thread per node before too, didn\u0027t we? Then please disregard my comment :)","commit_id":"858c92bfa843ff8dbff57835cc7c667deab63c05"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"08fd2bae2b1e02b2f0bee0e49be4f1ea06e59db3","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                                worker.MaintenanceWorker,"},{"line_number":340,"context_line":"                                service.RpcWorker)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    @lockutils.synchronized(\u0027hash_ring_probe_lock\u0027, external\u003dTrue)"},{"line_number":343,"context_line":"    def _start_hash_ring_probe(self):"},{"line_number":344,"context_line":"        if not self._hash_ring_probe_event.is_set():"},{"line_number":345,"context_line":"            self._hash_ring_thread \u003d maintenance.MaintenanceThread()"}],"source_content_type":"text/x-python","patch_set":2,"id":"c3d3bd45_014881cd","line":342,"in_reply_to":"a57d3668_affb4dd8","updated":"2022-10-04 17:42:50.000000000","message":"That\u0027s right, the external\u003dTrue means this lock works across multiple processes.\n\nYes we only had one per node before (the maintenance task). The hash ring probe does probe for all workers in the same host, so only one is needed (it\u0027s an optimization to minimize DB writes)","commit_id":"858c92bfa843ff8dbff57835cc7c667deab63c05"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"37a2602c37a2876e2cd47dfcc0977a9622f64a59","unresolved":true,"context_lines":[{"line_number":339,"context_line":"                                worker.MaintenanceWorker,"},{"line_number":340,"context_line":"                                service.RpcWorker)"},{"line_number":341,"context_line":""},{"line_number":342,"context_line":"    @lockutils.synchronized(\u0027hash_ring_probe_lock\u0027, external\u003dTrue)"},{"line_number":343,"context_line":"    def _start_hash_ring_probe(self):"},{"line_number":344,"context_line":"        if not self._hash_ring_probe_event.is_set():"},{"line_number":345,"context_line":"            self._hash_ring_thread \u003d maintenance.MaintenanceThread()"}],"source_content_type":"text/x-python","patch_set":2,"id":"657bc4aa_c6f23b51","line":342,"range":{"start_line":342,"start_character":4,"end_line":342,"end_character":66},"in_reply_to":"d8363188_db128dbf","updated":"2022-10-04 14:13:21.000000000","message":"That would minimize the racing window but, wouldn\u0027t guarantee that only one process would enter that if block. I would keep the lock to be 100% sure.","commit_id":"858c92bfa843ff8dbff57835cc7c667deab63c05"}]}