)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0e467c28eee3bbe986a414db29a2eb4d08ef4c1a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e51f3a8e_da2396b1","updated":"2026-05-06 15:41:56.000000000","message":"Testing this approach, the final implementation may differ.","commit_id":"36a4563d8e334caf4c3d1d3fff860fd6cc0f97f6"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8f434183786cee529112effe37eda71b2986f455","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"042ae3a9_5a60f831","updated":"2026-05-13 15:54:36.000000000","message":"I think this is a good improvement on model handling. I just have a couple of minor doubts but I\u0027m mostly fine with it.","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"7a7ebb2876beb1cf3e2532276bbe2b3e636b9b6d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"f6bbf009_a9641c8a","updated":"2026-05-14 08:22:29.000000000","message":"The third party job has failed the tempest tests:\n\nhttps://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/logs//9ab/rdoproject.org/9ab5833dee2647c4bb0f6d9625b0acf0/controller/ci-framework-data/tests/test_operator/tempest-tests-tempest/tempest_results.html\n\nLooking at one of the failing tests in logs:\n\n2026-05-13 21:50:15.706 11 DEBUG watcher.decision_engine.audit.base [None req-aec0cd48-655d-4038-bc24-d2b19569c915 - - - - - -] Trigger audit e60ca556-13f3-4769-823b-96b12495919c \n\n\u003cModelRoot\u003e\n  \u003cComputeNode hostname\u003d\"compute-1.ctlplane.example.com\" status\u003d\"disabled\" disabled_reason\u003d\"Watcher node resource consolidation strategy\" state\u003d\"up\" memory\u003d\"7679\" memory_mb_reserved\u003d\"512\" disk\u003d\"79\" disk_gb_reserved\u003d\"1\" vcpus\u003d\"8\" vcpu_reserved\u003d\"0\" memory_ratio\u003d\"1.0\" vcpu_ratio\u003d\"4.0\" disk_ratio\u003d\"0.9\" uuid\u003d\"6ba33264-3244-42be-9319-384a6740b57d\"\u003e\n    \u003cInstance watcher_exclude\u003d\"False\" name\u003d\"tempest-TestExecuteNodeResourceConsolidationStrategy-server-230113169\" state\u003d\"active\" memory\u003d\"128\" disk\u003d\"1\" vcpus\u003d\"1\" metadata\u003d\"{}\" project_id\u003d\"037192cf662f4059aa2ba83f1ee6e8f9\" locked\u003d\"False\" pinned_az\u003d\"\" flavor_extra_specs\u003d\"{}\" uuid\u003d\"286a8b2d-7e76-4b96-9264-b6ce321c9550\"/\u003e\n  \u003c/ComputeNode\u003e\n  \u003cComputeNode hostname\u003d\"compute-0.ctlplane.example.com\" status\u003d\"enabled\" disabled_reason\u003d\"None\" state\u003d\"up\" memory\u003d\"7679\" memory_mb_reserved\u003d\"512\" disk\u003d\"79\" disk_gb_reserved\u003d\"1\" vcpus\u003d\"8\" vcpu_reserved\u003d\"0\" memory_ratio\u003d\"1.0\" vcpu_ratio\u003d\"4.0\" disk_ratio\u003d\"0.9\" uuid\u003d\"f1bc88a4-0e25-4eda-9a29-25f344ce9085\"\u003e\n    \u003cInstance watcher_exclude\u003d\"False\" name\u003d\"tempest-TestExecuteNodeResourceConsolidationStrategy-server-139045779\" state\u003d\"active\" memory\u003d\"128\" disk\u003d\"1\" vcpus\u003d\"1\" metadata\u003d\"{}\" project_id\u003d\"037192cf662f4059aa2ba83f1ee6e8f9\" locked\u003d\"False\" pinned_az\u003d\"\" flavor_extra_specs\u003d\"{}\" uuid\u003d\"eb1467ed-e391-4c81-a288-0f3ff3c7a83b\"/\u003e\n  \u003c/ComputeNode\u003e\n\u003c/ModelRoot\u003e\n _pre_execute /usr/lib/python3.12/site-packages/watcher/decision_engine/strategy/strategies/base.py:272\n2026-05-13 21:50:15.756 11 INFO watcher.decision_engine.strategy.strategies.node_resource_consolidation [None req-ee9a8dbf-ecaf-4446-ae27-378ab34ed79a - - - - - -] nodes: [] migration failed\n2026-05-13 21:50:15.756 11 INFO watcher.decision_engine.strategy.strategies.node_resource_consolidation [None req-ee9a8dbf-ecaf-4446-ae27-378ab34ed79a - - - - - -] node compute-1.ctlplane.example.com is down\n2026-05-13 21:50:15.756 11 INFO watcher.decision_engine.strategy.strategies.node_resource_consolidation [None req-ee9a8dbf-ecaf-4446-ae27-378ab34ed79a - - - - - -] node compute-1.ctlplane.example.com is down\n2026-05-13 21:50:15.757 11 INFO watcher.decision_engine.strategy.strategies.node_resource_consolidation [None req-ee9a8dbf-ecaf-4446-ae27-378ab34ed79a - - - - - -] Free nodes: []\n2026-05-13 21:50:15.757 11 INFO watcher.decision_engine.strategy.strategies.node_resource_consolidation [None req-ee9a8dbf-ecaf-4446-ae27-378ab34ed79a - - - - - -] Source nodes: []\n\nNote it reports the compute node compute-1.ctlplane.example.com reports status\u003d\"disabled\" disabled_reason\u003d\"Watcher node resource consolidation strategy\" \n\nThat status comes from a previous test for node_resource_consolidation which disabled it.\n\nHowever, before triggering the failed audit there is a service.update notification saying that the service was re-enabled\n\n2026-05-13 21:49:14.703 11 INFO watcher.decision_engine.model.notification.nova [None req-a720e114-529a-48d3-882c-4d48668d8ff6 e3be9db3df7e4ffda67d1311f55180f6 53210df8b0ab4833a067216c5c11546e - - - -] Event \u0027service.update\u0027 received from nova-compute:compute-1.ctlplane.example.com with metadata {\u0027message_id\u0027: \u0027622b80c5-e5fe-4abd-aea5-3849776f8c50\u0027, \u0027timestamp\u0027: \u00272026-05-13 21:49:14.696504\u0027}\n2026-05-13 21:49:14.703 11 DEBUG watcher.decision_engine.model.notification.nova [None req-a720e114-529a-48d3-882c-4d48668d8ff6 e3be9db3df7e4ffda67d1311f55180f6 53210df8b0ab4833a067216c5c11546e - - - -] {\u0027nova_object.name\u0027: \u0027ServiceStatusPayload\u0027, \u0027nova_object.namespace\u0027: \u0027nova\u0027, \u0027nova_object.version\u0027: \u00271.1\u0027, \u0027nova_object.data\u0027: {\u0027host\u0027: \u0027compute-1.ctlplane.example.com\u0027, \u0027binary\u0027: \u0027nova-compute\u0027, \u0027topic\u0027: \u0027compute\u0027, \u0027report_count\u0027: 148, \u0027disabled\u0027: False, \u0027disabled_reason\u0027: None, \u0027availability_zone\u0027: None, \u0027last_seen_up\u0027: \u00272026-05-13T21:49:14Z\u0027, \u0027forced_down\u0027: False, \u0027version\u0027: 70, \u0027uuid\u0027: \u0027cc48015d-cf94-4690-98cb-1ea2b132c577\u0027}} info /usr/lib/python3.12/site-packages/watcher/decision_engine/model/notification/nova.py:402\n\nBut apparently that was not reflected in the model.\n\nI\u0027d say that the notification arrived while it was doing a collection\n\n2026-05-13 21:49:13.367 11 DEBUG watcher.decision_engine.model.collector.nova [None req-7ad4845c-eab3-40be-b270-4a9a39f226e3 - - - - - -] Building latest Nova cluster data model execute /usr/lib/python3.12/site-packages/watcher/decision_engine/model/collector/nova.py:163\n\nAnd the collection had not finished yet, as there are calls to nova still after the notification arrives:\n\n2026-05-13 21:49:14.858 11 DEBUG urllib3.connectionpool [-] https://nova-internal.openstack.svc:8774 \"GET /v2.1/servers/detail?limit\u003d2\u0026host\u003dcompute-0.ctlplane.example.com\u0026all_tenants\u003dTrue HTTP/1.1\" 200 5314 _make_request /usr/lib/python3.12/site-packages/urllib3/connectionpool.py:474\n\n2026-05-13 21:49:15.096 11 DEBUG urllib3.connectionpool [-] https://nova-internal.openstack.svc:8774 \"GET /v2.1/servers/detail?host\u003dcompute-0.ctlplane.example.com\u0026all_tenants\u003dTrue\u0026limit\u003d2\u0026marker\u003dc801387b-5c9d-44f6-801b-40fb2718a35f HTTP/1.1\" 200 15 _make_request /usr/lib/python3.12/site-packages/urllib3/connectionpool.py:474\n\nSo this seems to be a race condition before collector and notifications. I\u0027m not entirely sure that this is related to the change in the lock in the patch, but it\u0027s suspictius that we hit this issue right in this patch and i don\u0027t remember having it before.... Good to double check.","commit_id":"7554e6b49ec56e3a55bd87f912941ad9e21d4b39"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"e0304ac82adf2b07c1b2977b6099d36bae367a5c","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"687a6820_402474e1","in_reply_to":"4d415546_f4ad3fd5","updated":"2026-05-19 07:47:20.000000000","message":"My suspiction is that the shared per-method lock (add_node, map_instance, etc...) was kind of mitigating the lack of a global lock between synchronize and notifications update before this patch.","commit_id":"7554e6b49ec56e3a55bd87f912941ad9e21d4b39"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"4330a1c5089534f6da2e0b7aabbfe2ef353575f8","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4d415546_f4ad3fd5","in_reply_to":"74370cde_b1a9b3b0","updated":"2026-05-14 17:10:20.000000000","message":"As far I understood from code, the issue is expected to happen in current and previous implementation of the lock. I filed a bug to investigate this issue: https://bugs.launchpad.net/watcher/+bug/2152645","commit_id":"7554e6b49ec56e3a55bd87f912941ad9e21d4b39"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"cf1758b0ac669fcdb99a8ce5b8a0d51baf206eda","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"74370cde_b1a9b3b0","in_reply_to":"b43e8e86_80ab40de","updated":"2026-05-14 08:50:29.000000000","message":"So, I think we have a race between the new model being generated by the collector and the notification is applied. I guess the problem is that the periodic collection is creating a new model with a new lock and the reference to the new model in the \"main\" model in the collector only changes at the end of the collection. In the meanwhile, if a notification arrives before the new is finished, there is no lock between both, so the notification is applied in the initial model which is overriden when the periodic collection finishes.\n\nI think we need a shared lock between the execute() or synchronize() and the updates from notifications?","commit_id":"7554e6b49ec56e3a55bd87f912941ad9e21d4b39"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"c133cc45ef2f92b4e2cb8116b03f693ba8acc727","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"b43e8e86_80ab40de","in_reply_to":"f6bbf009_a9641c8a","updated":"2026-05-14 08:23:47.000000000","message":"decision-engine log is\n\nhttps://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/logs//9ab/rdoproject.org/9ab5833dee2647c4bb0f6d9625b0acf0/controller/ci-framework-data/logs/openstack-must-gather/quay-io-openstack-k8s-operators-openstack-must-gather-sha256-70ce4ab587da14dddf23f3816879343650007e91038f935686ab5355e6f2a7a2/namespaces/openstack/pods/watcher-decision-engine-0/logs/watcher-decision-engine.log.gz\n\nAnd tempest execution log\n\nhttps://gateway-cloud-softwarefactory.apps.ocp.cloud.ci.centos.org/logs//9ab/rdoproject.org/9ab5833dee2647c4bb0f6d9625b0acf0/controller/ci-framework-data/tests/test_operator/tempest-tests-tempest/tempest_results.log.gz","commit_id":"7554e6b49ec56e3a55bd87f912941ad9e21d4b39"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"693b8dbc76e99886b5e1322d7adb082ab019b35d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"49c60fc4_88cc9ff9","updated":"2026-05-27 09:32:54.000000000","message":"I don\u0027t find any issue that this may introduced other than the reported in bug #2152645 which is actually pre-existing and is being managed in the same patch series. I think per-instance lock should be what we need for these locks. The only case of locking between diferent instances is required is the race between notifications and synchronize which is being handled properly in separate patch.","commit_id":"2477f8fd4c6f158d562c44811011bc1b568dc2fb"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"9937b5095fec65890e223c603c34ebb6239d354d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"36fd37c3_472eaa76","updated":"2026-05-20 19:20:57.000000000","message":"recheck, py311 error doesn\u0027t reproduce in local env","commit_id":"2477f8fd4c6f158d562c44811011bc1b568dc2fb"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"a4f7d5a6d772c4d2e3f7849864fbd01e3788fb4b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"4aa42c37_5b48748a","updated":"2026-06-11 19:29:20.000000000","message":"recheck\n\nprometheus job timed out","commit_id":"51a27ae3b5aabf989d14f5acc3a1094db5df8d3a"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"80b5b237bad677b25b1d0d9fc149333ea2d26b7d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"cdeb942b_006d9e2e","updated":"2026-06-11 15:54:45.000000000","message":"thanks Doug, lgtm","commit_id":"51a27ae3b5aabf989d14f5acc3a1094db5df8d3a"}],"watcher/decision_engine/model/collector/base.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8f434183786cee529112effe37eda71b2986f455","unresolved":true,"context_lines":[{"line_number":183,"context_line":""},{"line_number":184,"context_line":"    def get_latest_cluster_data_model(self):"},{"line_number":185,"context_line":"        LOG.debug(\"Creating copy\")"},{"line_number":186,"context_line":"        with self.lock:"},{"line_number":187,"context_line":"            model \u003d self.cluster_data_model"},{"line_number":188,"context_line":"            LOG.debug(model.to_xml())"},{"line_number":189,"context_line":"            return copy.deepcopy(model)"}],"source_content_type":"text/x-python","patch_set":3,"id":"99ac3188_271e92ca","line":186,"range":{"start_line":186,"start_character":0,"end_line":186,"end_character":2},"updated":"2026-05-13 15:54:36.000000000","message":"+1 good to protect this with a lock.","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"}],"watcher/decision_engine/model/model_root.py":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8f434183786cee529112effe37eda71b2986f455","unresolved":true,"context_lines":[{"line_number":47,"context_line":"    return wrapper"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def with_instance_lock(cls):"},{"line_number":51,"context_line":"    \"\"\"Class decorator that adds a per-instance RLock and safe __deepcopy__."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    Wraps __init__ to initialise self._lock \u003d threading.RLock() after the"}],"source_content_type":"text/x-python","patch_set":3,"id":"efa8e37f_dc35b661","line":50,"range":{"start_line":50,"start_character":0,"end_line":50,"end_character":2},"updated":"2026-05-13 15:54:36.000000000","message":"I think we may use a mixin class here where ModelRoot and in StorageModelRoot would inherit from, right? I would find that more clear that using this decorator, but it may be only a personal preference.","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cf196a49d02ef8b38fa3ff67089bc2d2dfec3e58","unresolved":true,"context_lines":[{"line_number":47,"context_line":"    return wrapper"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def with_instance_lock(cls):"},{"line_number":51,"context_line":"    \"\"\"Class decorator that adds a per-instance RLock and safe __deepcopy__."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    Wraps __init__ to initialise self._lock \u003d threading.RLock() after the"}],"source_content_type":"text/x-python","patch_set":3,"id":"ffec1c70_e795a960","line":50,"range":{"start_line":50,"start_character":0,"end_line":50,"end_character":2},"in_reply_to":"efa8e37f_dc35b661","updated":"2026-05-13 19:51:28.000000000","message":"Mixin classes usually provide functionalities through additional methods, which is not what we are doing here. It would be more like a normal inheritance instead, with the additional lock attribute. I will think more about a different approach, but so far this seems the best one so far...","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"693b8dbc76e99886b5e1322d7adb082ab019b35d","unresolved":false,"context_lines":[{"line_number":47,"context_line":"    return wrapper"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def with_instance_lock(cls):"},{"line_number":51,"context_line":"    \"\"\"Class decorator that adds a per-instance RLock and safe __deepcopy__."},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    Wraps __init__ to initialise self._lock \u003d threading.RLock() after the"}],"source_content_type":"text/x-python","patch_set":3,"id":"021c1c61_fd814efa","line":50,"range":{"start_line":50,"start_character":0,"end_line":50,"end_character":2},"in_reply_to":"ffec1c70_e795a960","updated":"2026-05-27 09:32:54.000000000","message":"Acknowledged","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"8f434183786cee529112effe37eda71b2986f455","unresolved":true,"context_lines":[{"line_number":299,"context_line":"        return self.to_xml()"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def to_xml(self):"},{"line_number":302,"context_line":"        with self._lock:"},{"line_number":303,"context_line":"            root \u003d etree.Element(\"ModelRoot\")"},{"line_number":304,"context_line":"            # Build compute node tree"},{"line_number":305,"context_line":"            for cn in sorted("}],"source_content_type":"text/x-python","patch_set":3,"id":"7fc1ce45_78c4d39a","line":302,"range":{"start_line":302,"start_character":0,"end_line":302,"end_character":2},"updated":"2026-05-13 15:54:36.000000000","message":"I understand we are locking here and in to_list to make sure there are no modifications to the model while executing it, right?\nAny reason to use the context manager explicitely here instead of using the instance_lock decorator?","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"cf196a49d02ef8b38fa3ff67089bc2d2dfec3e58","unresolved":true,"context_lines":[{"line_number":299,"context_line":"        return self.to_xml()"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def to_xml(self):"},{"line_number":302,"context_line":"        with self._lock:"},{"line_number":303,"context_line":"            root \u003d etree.Element(\"ModelRoot\")"},{"line_number":304,"context_line":"            # Build compute node tree"},{"line_number":305,"context_line":"            for cn in sorted("}],"source_content_type":"text/x-python","patch_set":3,"id":"8ad7c2cc_dcbaca8b","line":302,"range":{"start_line":302,"start_character":0,"end_line":302,"end_character":2},"in_reply_to":"7fc1ce45_78c4d39a","updated":"2026-05-13 19:51:28.000000000","message":"oh yeah, my bad, i will update that","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"693b8dbc76e99886b5e1322d7adb082ab019b35d","unresolved":false,"context_lines":[{"line_number":299,"context_line":"        return self.to_xml()"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"    def to_xml(self):"},{"line_number":302,"context_line":"        with self._lock:"},{"line_number":303,"context_line":"            root \u003d etree.Element(\"ModelRoot\")"},{"line_number":304,"context_line":"            # Build compute node tree"},{"line_number":305,"context_line":"            for cn in sorted("}],"source_content_type":"text/x-python","patch_set":3,"id":"1b853e9f_73c8dc32","line":302,"range":{"start_line":302,"start_character":0,"end_line":302,"end_character":2},"in_reply_to":"8ad7c2cc_dcbaca8b","updated":"2026-05-27 09:32:54.000000000","message":"Done","commit_id":"bef15026f51cd20d292da62a9a52ef37c1058ffa"}]}