)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"bc777aef5bc8f5c7f875a8bd80a9161e3355d635","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c0534df6_0538e15b","updated":"2025-02-27 11:12:42.000000000","message":"CI is healthy again","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"bebef3b5cf1f4eeece36777b64d102d328ab0544","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"aeb10017_1ebd2095","updated":"2025-02-25 15:59:24.000000000","message":"Holding the W+1, gate is still broken[1]\n\n[1] https://launchpad.net/bugs/2099955","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"51245c853cf30be241ba5a30c3341834cc9640d0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5949b0c6_2b89949a","updated":"2025-02-25 14:42:07.000000000","message":"Thanks James","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4629905779a4d44a923cf5494b784497c176d10e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"4af55c01_74386a31","updated":"2025-02-15 18:51:02.000000000","message":"This is kind of a half measure but it may be a good interim solution.\n\nwe discussed the session and connection handling issues in \nhttps://github.com/eventlet/eventlet/issues/1020\nin relation to https://bugs.launchpad.net/watcher/+bug/2086710\n\nthe correct long term solution would be to properly refactor the code to \nuse the enginefacade transaction decorators.\n\nhttps://specs.openstack.org/openstack/nova-specs/specs/mitaka/implemented/oslo_db-enginefacade.html\n\nThat is more invasive than what you are proposing but it much less error prone.\n\nwe were going to discuss this topic in the next ptg as part fo tech debt removal.\nim not against this current change as an intermediate stepping stone but its quite error prone so i would liek to move away from manually using context managers for sessions and connection. the real issue with that is having causticity to actually do that refactor so it was likely going to be a 2025.2 or 2026.1 activity. \n\n\ni also would not have been comfortable backporting the lareger refactor.\n\nill need to look at this properly durign the week but this seam more reasonable to consider backporting.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c7ad3a90846fa1114bc82c229fde415e211da1a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c9b537e9_25c48c8f","updated":"2025-02-25 16:04:40.000000000","message":"ack on waiting for ci to be unblocked.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f777d66f7a58e41a6287a366f6d4d175385e8422","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ce7458f0_dd802cb0","updated":"2025-02-24 17:21:30.000000000","message":"while i think this can be improved in the long term\ni think this is important enough to make progress on in 2025.1\n\nas i have also noted elsewhere the more robust approch is more invasive and less backportable so for now i think this is the right way to adress this for master and stable/*\n\nnext cycle we can see if we have time for a larger refactor vs the other techdebt we have.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"}],"watcher/db/sqlalchemy/api.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"13456cabea3408b90581622df27eff6efdd2714e","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"# NOTE(tylerchristie) Please add @oslo_db_api.retry_on_deadlock decorator to"},{"line_number":56,"context_line":"# any new methods using _session_for_write (as deadlocks happen on write), so"},{"line_number":57,"context_line":"# that oslo_db is able to retry in case of deadlocks."},{"line_number":58,"context_line":"def _session_for_write():"},{"line_number":59,"context_line":"    return enginefacade.writer.using(_CONTEXT)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1875a1e0_e3954ad4","line":57,"updated":"2025-02-25 15:23:53.000000000","message":"so this comment is overstating what we actually support\nif your using a supported database topology with only one replica accepting writes then you shoudl not get deadlocks on write ever as all writes will be in an independent write transaction and the database will linearize the requests.\n\nwhile we do not support active active unless the guarantee read consistency after every write and do nto test them we generally accept changes that make that topology less problematic.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"bebef3b5cf1f4eeece36777b64d102d328ab0544","unresolved":true,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"# NOTE(tylerchristie) Please add @oslo_db_api.retry_on_deadlock decorator to"},{"line_number":56,"context_line":"# any new methods using _session_for_write (as deadlocks happen on write), so"},{"line_number":57,"context_line":"# that oslo_db is able to retry in case of deadlocks."},{"line_number":58,"context_line":"def _session_for_write():"},{"line_number":59,"context_line":"    return enginefacade.writer.using(_CONTEXT)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"aa7a0c2e_749f67bf","line":57,"in_reply_to":"1875a1e0_e3954ad4","updated":"2025-02-25 15:59:24.000000000","message":"ack, don\u0027t see any problem with leaving this here, and the decorators already added to the code.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"c7ad3a90846fa1114bc82c229fde415e211da1a8","unresolved":false,"context_lines":[{"line_number":54,"context_line":""},{"line_number":55,"context_line":"# NOTE(tylerchristie) Please add @oslo_db_api.retry_on_deadlock decorator to"},{"line_number":56,"context_line":"# any new methods using _session_for_write (as deadlocks happen on write), so"},{"line_number":57,"context_line":"# that oslo_db is able to retry in case of deadlocks."},{"line_number":58,"context_line":"def _session_for_write():"},{"line_number":59,"context_line":"    return enginefacade.writer.using(_CONTEXT)"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f595bf43_eccb0ac0","line":57,"in_reply_to":"aa7a0c2e_749f67bf","updated":"2025-02-25 16:04:40.000000000","message":"ya, i think it is ok we just cant commit to supproting that use case.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"aee4b52a32d4bbd2b9c948774b6f73a40eb488f6","unresolved":true,"context_lines":[{"line_number":588,"context_line":"                                    self._add_audit_templates_filters,"},{"line_number":589,"context_line":"                                    *args, **kwargs)"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"    def create_audit_template(self, values):"},{"line_number":592,"context_line":"        # ensure defaults are present for new audit_templates"},{"line_number":593,"context_line":"        if not values.get(\u0027uuid\u0027):"},{"line_number":594,"context_line":"            values[\u0027uuid\u0027] \u003d utils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":1,"id":"a24bb94c_99d3c1ee","line":591,"range":{"start_line":591,"start_character":8,"end_line":591,"end_character":29},"updated":"2025-02-24 20:51:39.000000000","message":"Shouldn\u0027t we add oslo_db_api.retry_on_deadlock decorator here and in the other methods below, that are using _session_for_write?","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"51245c853cf30be241ba5a30c3341834cc9640d0","unresolved":true,"context_lines":[{"line_number":588,"context_line":"                                    self._add_audit_templates_filters,"},{"line_number":589,"context_line":"                                    *args, **kwargs)"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"    def create_audit_template(self, values):"},{"line_number":592,"context_line":"        # ensure defaults are present for new audit_templates"},{"line_number":593,"context_line":"        if not values.get(\u0027uuid\u0027):"},{"line_number":594,"context_line":"            values[\u0027uuid\u0027] \u003d utils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":1,"id":"1815f474_d0b2af59","line":591,"range":{"start_line":591,"start_character":8,"end_line":591,"end_character":29},"in_reply_to":"0d8b6d86_7ea7b665","updated":"2025-02-25 14:42:07.000000000","message":"ack for this one, but there are other query.delete() below, for other methods.\nbut would require more refactoring than this. So we might be ok with the current patch for now.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"398cb3449f17266602f5c9a0001ecc7251ef843c","unresolved":true,"context_lines":[{"line_number":588,"context_line":"                                    self._add_audit_templates_filters,"},{"line_number":589,"context_line":"                                    *args, **kwargs)"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"    def create_audit_template(self, values):"},{"line_number":592,"context_line":"        # ensure defaults are present for new audit_templates"},{"line_number":593,"context_line":"        if not values.get(\u0027uuid\u0027):"},{"line_number":594,"context_line":"            values[\u0027uuid\u0027] \u003d utils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":1,"id":"0407d1d8_9620746f","line":591,"range":{"start_line":591,"start_character":8,"end_line":591,"end_character":29},"in_reply_to":"1815f474_d0b2af59","updated":"2025-02-25 15:18:37.000000000","message":"I do not believe we can get a db deadlock form query.delete()\n\nso I don\u0027t think retry_on_deadlock is required\n\n_destroy is not used in the same way that _create is.\nand there are some cases where an update is done directly.\ndb deadlocks generally indicate that you have dpeloy the service in an unsupported topology.\n\nhowever, that is a good point to consider with regard to what database requriement we have. we require strict acid compliance and galara does not fullfile that in active active mode\n\nto be explcit we do not support Active/Active galara because it does not guarrente that reads form a difent replcia will always see the most recent write.\nthat brakes the C requirement in ACID, reads in active active galarea are not consitnet across replicas.\n\nso at most there can be 1 active db process accpating writes\nsince there is only one accepting write a write transaction should be enough to prevent db deadlocks with other requests.\n\n\nso strictly speaking we should never need @oslo_db_api.retry_on_deadlock on any of these calls because we should never have a db deadlock. if we do we have a bug.\n\nin practice some operator od try to run with active active galarea even though its not supproted by Watcher, Nova, placement neutron or basically any upstream service \nand over the year @oslo_db_api.retry_on_deadlock and similar decorators have been added to adie in that unsupported usecase.\n\nto me adding @oslo_db_api.retry_on_deadlock is strictly speaking an unrelated change we did not rety these queies before and this coud have been done without proting the use of that decorator.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"a75bff43d29448eaab7ae2497d2dee5f68bafa14","unresolved":true,"context_lines":[{"line_number":588,"context_line":"                                    self._add_audit_templates_filters,"},{"line_number":589,"context_line":"                                    *args, **kwargs)"},{"line_number":590,"context_line":""},{"line_number":591,"context_line":"    def create_audit_template(self, values):"},{"line_number":592,"context_line":"        # ensure defaults are present for new audit_templates"},{"line_number":593,"context_line":"        if not values.get(\u0027uuid\u0027):"},{"line_number":594,"context_line":"            values[\u0027uuid\u0027] \u003d utils.generate_uuid()"}],"source_content_type":"text/x-python","patch_set":1,"id":"0d8b6d86_7ea7b665","line":591,"range":{"start_line":591,"start_character":8,"end_line":591,"end_character":29},"in_reply_to":"a24bb94c_99d3c1ee","updated":"2025-02-24 22:00:45.000000000","message":"its on _create\n\ncalle don line 606 so we will never get the deadlock here.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"}],"watcher/tests/db/base.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f777d66f7a58e41a6287a366f6d4d175385e8422","unresolved":true,"context_lines":[{"line_number":40,"context_line":"        self.sql_connection \u003d sql_connection"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        self.engine \u003d engine"},{"line_number":43,"context_line":"        self.engine.dispose()"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"        with self.engine.connect() as conn:"},{"line_number":46,"context_line":"            self.setup_sqlite(db_migrate)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7cd4ccc5_8c66bd06","line":43,"updated":"2025-02-24 17:21:30.000000000","message":"we may want to revisit why this explicit dispose is required\nand either remove it or document it in a followup.\n\nit was there before but no reason is given why we are disposing of the engine and then reconnecting again.","commit_id":"753c44b0c465f018d7c61f5ffce24b603e2f12c0"}]}