)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"2815dbdbf93f6e160a076ec2ab95ffca0f2dcf10","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d80efb0a_ee75d9e8","updated":"2026-07-28 12:08:57.000000000","message":"recheck","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"}],"watcher/decision_engine/model/model_root.py":[{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"460f798fa3ccbae3673d6bbe449b54e34684d718","unresolved":true,"context_lines":[{"line_number":112,"context_line":"        self._extended_attributes_enabled \u003d value"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _add_to_resource_cache(self, node_uuid, instance):"},{"line_number":115,"context_line":"        if node_uuid in self._node_resource_cache:"},{"line_number":116,"context_line":"            cached \u003d self._node_resource_cache[node_uuid]"},{"line_number":117,"context_line":"            cached[\u0027vcpu\u0027] +\u003d instance.vcpus"},{"line_number":118,"context_line":"            cached[\u0027memory\u0027] +\u003d instance.memory"}],"source_content_type":"text/x-python","patch_set":1,"id":"2598284c_af1259ed","line":115,"updated":"2026-07-28 11:14:27.000000000","message":"won\u0027t this miss adding the first instance created on the node? e.g we have an empty compute node when we first build the model so the node will be created but have no entry in the cache. After some time, the first instance is created and this is called but the instance resources are not added since the node is not in the cache, if I\u0027m correct we could easily fix this by adding an else or using a default dict","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"d983eddbb980b166ca5d43c8d3d53a9b1184a666","unresolved":true,"context_lines":[{"line_number":112,"context_line":"        self._extended_attributes_enabled \u003d value"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _add_to_resource_cache(self, node_uuid, instance):"},{"line_number":115,"context_line":"        if node_uuid in self._node_resource_cache:"},{"line_number":116,"context_line":"            cached \u003d self._node_resource_cache[node_uuid]"},{"line_number":117,"context_line":"            cached[\u0027vcpu\u0027] +\u003d instance.vcpus"},{"line_number":118,"context_line":"            cached[\u0027memory\u0027] +\u003d instance.memory"}],"source_content_type":"text/x-python","patch_set":1,"id":"90a34c66_fbcfff98","line":115,"in_reply_to":"2598284c_af1259ed","updated":"2026-07-28 12:08:06.000000000","message":"I\u0027m not sure I\u0027m following you. The idea is that the cache is not initialized on each node creation but until the first time that the `get_node_used_resources` method is called for the node (line 335). Once the cache is created for the node, then we want to maintain it updated but while it\u0027s not used, we don\u0027t want to maintain it at all.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"859e4a048e7dbe2cf48a9dd6bce4f5e7d813a0e8","unresolved":false,"context_lines":[{"line_number":112,"context_line":"        self._extended_attributes_enabled \u003d value"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"    def _add_to_resource_cache(self, node_uuid, instance):"},{"line_number":115,"context_line":"        if node_uuid in self._node_resource_cache:"},{"line_number":116,"context_line":"            cached \u003d self._node_resource_cache[node_uuid]"},{"line_number":117,"context_line":"            cached[\u0027vcpu\u0027] +\u003d instance.vcpus"},{"line_number":118,"context_line":"            cached[\u0027memory\u0027] +\u003d instance.memory"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a1cac33_b63553dd","line":115,"in_reply_to":"90a34c66_fbcfff98","updated":"2026-07-28 13:55:04.000000000","message":"ok I see it now, we only care about this cache once we\u0027ve called get_node_used_resources, for other strategies it\u0027s irrelevant so the scenario I mentioned it\u0027s not a problem","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"06b451774b3c8fac19018a3394e05065d0343a35","unresolved":false,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        return node_instances"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def get_node_used_resources(self, node):"},{"line_number":323,"context_line":"        if node.uuid in self._node_resource_cache:"},{"line_number":324,"context_line":"            return dict(self._node_resource_cache[node.uuid])"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"2d59dac5_a5268ec5","line":322,"updated":"2026-07-28 08:59:20.000000000","message":"get_node_used_resources, get_node_free_resources, and the invalidate_* methods read/write _node_resource_cache without @instance_lock. All topology methods that also modify the cache ARE locked. Notification handlers run on a separate thread, so a strategy calling get_node_used_resources can inte...\n\n**Severity**: WARNING | **Confidence**: 0.8\n\n**Impact**: If a nova notification arrives while a strategy calls get_node_used_resources, the cache can be populated with stale values overwriting a concurrent invalidation, or read a partially-updated entry. Strategies like basic_consolidation and node_resource_consolidation rely on these values for placem...\n\n**Suggestion**:\nAdd @instance_lock to get_node_used_resources and get_node_free_resources. ModelRoot uses threading.RLock (reentrant), so no deadlock when get_node_used_resources calls the already-locked get_node_instances. Also consider adding @instance_lock to invalidate_node_resource_cache and invalidate_resource_cache for consistency.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"15f88b6c1ba5ade14e46fe6910ec015716f11340","unresolved":true,"context_lines":[{"line_number":319,"context_line":""},{"line_number":320,"context_line":"        return node_instances"},{"line_number":321,"context_line":""},{"line_number":322,"context_line":"    def get_node_used_resources(self, node):"},{"line_number":323,"context_line":"        if node.uuid in self._node_resource_cache:"},{"line_number":324,"context_line":"            return dict(self._node_resource_cache[node.uuid])"},{"line_number":325,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"0e7904d0_84491a47","line":322,"in_reply_to":"2d59dac5_a5268ec5","updated":"2026-07-28 13:59:11.000000000","message":"I\u0027m not sure if this is correct. At first glance it seems like we\u0027d need to add it to `invalidate_node_resource_cache`, but considering each strategy would have a private copy of the cache I don\u0027t think the scenario proposed would actually be a problem","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"}],"watcher/tests/unit/decision_engine/model/test_model.py":[{"author":{"_account_id":28006,"name":"teim-ci","display_name":"teim-ci","email":"ci@seanmooney.info","username":"ci-sean-mooney","status":"this is a third-party ci account run by sean-k-mooney on irc\nhosted at zuul.teim.app"},"tag":"autogenerated:zuul:automatic-ci","change_message_id":"9ea67463ef4bf4a9d2930a9b1a0a95c0a8aa49d6","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        mock_get_node_instances \u003d mock.patch.object("},{"line_number":404,"context_line":"            model, \u0027get_node_instances\u0027"},{"line_number":405,"context_line":"        )"},{"line_number":406,"context_line":"        mock_get_node_instances.return_value \u003d []"},{"line_number":407,"context_line":"        with mock_get_node_instances:"},{"line_number":408,"context_line":"            used_b \u003d model.get_node_used_resources(node_b)"},{"line_number":409,"context_line":"            self.assertEqual(8, used_b[\u0027vcpu\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"cc85a1b7_43e0f17a","line":406,"updated":"2026-07-28 12:20:12.000000000","message":"In test_resource_cache_map_instance, mock_get_node_instances.return_value \u003d [] sets return_value on the _patch object before entering the context manager. This does not configure the mock\u0027s return value. When the mock is activated, it returns a default MagicMock, not an empty list. The test still...\n\n**Severity**: SUGGESTION | **Confidence**: 0.9\n\n**Benefit**: The test validates the correct behavior (cache hit skips get_node_instances, cache miss calls it), but the return_value assignment is a no-op. A future maintainer may believe the mock returns [] and be confused when modifying the test. If the test is later changed to check return values explicitl...\n\n**Recommendation**:\nUse the conventional context manager pattern: \u0027with mock.patch.object(model, \u0027get_node_instances\u0027) as mock_get_instances: mock_get_instances.return_value \u003d []\u0027 and indent the test body accordingly. Alternatively, remove the dead return_value line since MagicMock iteration already yields nothing.","commit_id":"593bd642b3307c8d6e578b8dcd8a9caf77c69e3d"}]}