)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"d3f6e4682bbe5846711d8a9b5b8b290108c724ad","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"c0b0dd57_60a49812","updated":"2022-05-17 17:48:25.000000000","message":"recheck","commit_id":"c99d4e2da5fee4b18e8f537df3526de1d3058ae7"},{"author":{"_account_id":27224,"name":"Duc Truong","email":"duc.openstack@gmail.com","username":"dtruong2"},"change_message_id":"0736b92af1238c448ea0c082079e76ba928b61b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"0e97dc4a_0cb61b32","updated":"2022-05-21 05:09:39.000000000","message":"lgtm","commit_id":"7bad1628960d4548c0d31bbc1a1fbd0e7dacadce"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"35fd37b17431fdb0a2ce0a109e731605ed0312d7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"47a513cf_bb1c806d","updated":"2022-05-21 23:48:28.000000000","message":"recheck","commit_id":"e3264a9a7525b4cd13633506c6c2bb99a68ef4be"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"7a834db0f989d2faf21bef4e28b7cda2c7587dc0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":21,"id":"303fdf85_3971da7c","updated":"2022-05-22 03:43:31.000000000","message":"recheck flavor error","commit_id":"e3264a9a7525b4cd13633506c6c2bb99a68ef4be"},{"author":{"_account_id":27224,"name":"Duc Truong","email":"duc.openstack@gmail.com","username":"dtruong2"},"change_message_id":"2e0987ba5e5aa46c04448d3b05eb2a0744ccd4eb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":24,"id":"9927e753_d9e72d91","updated":"2022-05-24 18:32:15.000000000","message":"lgtm","commit_id":"ee2959af3c1fa63ce7068047105ec66ebcf4f5aa"}],"senlin/common/service.py":[{"author":{"_account_id":27224,"name":"Duc Truong","email":"duc.openstack@gmail.com","username":"dtruong2"},"change_message_id":"486c0a1647b00d97347042d36c5bb9f6b8ebfff2","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        # running on the main process."},{"line_number":47,"context_line":"        self.start_service_cleanup()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def start_service_cleanup(self):"},{"line_number":50,"context_line":"        if not self.tg:"},{"line_number":51,"context_line":"            return"},{"line_number":52,"context_line":"        self.cleanup_timer \u003d self.tg.add_timer("}],"source_content_type":"text/x-python","patch_set":15,"id":"3e3b8ed5_216c692b","line":49,"updated":"2022-05-18 06:57:34.000000000","message":"Do we need this as a separate method or can it just be part of __init__?  I think we don\u0027t want anybody to call this method and inadvertently create more cleanup timers.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"45c7d72442bb4c511b659feb90d73f242da24056","unresolved":true,"context_lines":[{"line_number":46,"context_line":"        # running on the main process."},{"line_number":47,"context_line":"        self.start_service_cleanup()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def start_service_cleanup(self):"},{"line_number":50,"context_line":"        if not self.tg:"},{"line_number":51,"context_line":"            return"},{"line_number":52,"context_line":"        self.cleanup_timer \u003d self.tg.add_timer("}],"source_content_type":"text/x-python","patch_set":15,"id":"d7b6f110_9948c514","line":49,"in_reply_to":"3e3b8ed5_216c692b","updated":"2022-05-18 09:21:40.000000000","message":"We could, but I believe I added it to make it easier mock this out. I should at least make it an private method, but can try to move it back to init too.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"5df8a4c405a277e5874a1f617bd01f8765121d5e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"        # running on the main process."},{"line_number":47,"context_line":"        self.start_service_cleanup()"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def start_service_cleanup(self):"},{"line_number":50,"context_line":"        if not self.tg:"},{"line_number":51,"context_line":"            return"},{"line_number":52,"context_line":"        self.cleanup_timer \u003d self.tg.add_timer("}],"source_content_type":"text/x-python","patch_set":15,"id":"2a419608_f553ab28","line":49,"in_reply_to":"d7b6f110_9948c514","updated":"2022-05-18 17:29:57.000000000","message":"Done","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":27224,"name":"Duc Truong","email":"duc.openstack@gmail.com","username":"dtruong2"},"change_message_id":"486c0a1647b00d97347042d36c5bb9f6b8ebfff2","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"b02a1689_51d0fe2e","line":129,"updated":"2022-05-18 06:57:34.000000000","message":"Why is the timer stopped after 5 cleanups?","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"467a0f800dfa368348cc2017bffb44cd19290947","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"7d28abf5_2c125bc7","line":129,"in_reply_to":"1f1408fe_1ea0015e","updated":"2022-05-19 06:47:49.000000000","message":"I added an comment. I think we keep it like this for now, and after a bit of testing of this new code we can probably just introduce a variable to allow user to turn it on to run indefinitely. I don\u0027t want to change too much in case I broke something else.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":27224,"name":"Duc Truong","email":"duc.openstack@gmail.com","username":"dtruong2"},"change_message_id":"95f6c4c2363cd765a79d7176ed11136ee902bb46","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"1f1408fe_1ea0015e","line":129,"in_reply_to":"42399131_b39d4ed9","updated":"2022-05-19 06:07:43.000000000","message":"Yea, I don\u0027t quite understand the logic behind this from the old code either. Like you said, probably was done so that the multiple workers don\u0027t run too many cleanup timers.  With the timer moved into the service base, I think it is safe to run the timer without ever stopping it.  Or least add some comment why the timer needs to be stopped after 5 iterations.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":27224,"name":"Duc Truong","email":"duc.openstack@gmail.com","username":"dtruong2"},"change_message_id":"0736b92af1238c448ea0c082079e76ba928b61b0","unresolved":false,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"b2232b92_c681d570","line":129,"in_reply_to":"7d28abf5_2c125bc7","updated":"2022-05-21 05:09:39.000000000","message":"Yea, that sounds good.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"79b54d42d53959fca7aa71d60f021add46276cef","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"42399131_b39d4ed9","line":129,"in_reply_to":"985e1d28_e7c1b4d3","updated":"2022-05-18 21:20:13.000000000","message":"For reference this is how it looked before\nhttps://github.com/openstack/senlin/blob/master/senlin/conductor/service.py#L165","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"45c7d72442bb4c511b659feb90d73f242da24056","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"ef838118_df1f7606","line":129,"in_reply_to":"b02a1689_51d0fe2e","updated":"2022-05-18 09:21:40.000000000","message":"This is how it worked before. I believe the idea is that this runs on startup, so that it cleans up after something crashes or gets restarted. The problem was that it only happened on one service before and not all of them. So a hm being restarted or crashing wouldn’t be able to fix itself.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"},{"author":{"_account_id":22623,"name":"Erik Olof Gunnar Andersson","email":"eandersson@blizzard.com","username":"eoandersson"},"change_message_id":"5df8a4c405a277e5874a1f617bd01f8765121d5e","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                }"},{"line_number":127,"context_line":"            )"},{"line_number":128,"context_line":""},{"line_number":129,"context_line":"        if self.cleanup_count \u003e\u003d 5:"},{"line_number":130,"context_line":"            self.cleanup_timer.stop()"},{"line_number":131,"context_line":"            self.cleanup_timer \u003d None"},{"line_number":132,"context_line":"            LOG.info(\u0027Finished cleaning up dead services.\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"985e1d28_e7c1b4d3","line":129,"in_reply_to":"ef838118_df1f7606","updated":"2022-05-18 17:29:57.000000000","message":"We could consider changing this, especially with this new design as it is a lot more efficient than it was before.","commit_id":"0e436778f4f5a1b371fe092f19722518baf1dff4"}]}