)]}'
{"nova/virt/ironic/driver.py":[{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"92a911afd2fd88267babcd6969984f0a5fdab700","unresolved":true,"context_lines":[{"line_number":775,"context_line":"                  CONF.host.lower() in"},{"line_number":776,"context_line":"                  self.hash_ring.get_nodes(node.uuid.encode(\u0027utf-8\u0027))):"},{"line_number":777,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":778,"context_line":""},{"line_number":779,"context_line":"        self.node_cache \u003d node_cache"},{"line_number":780,"context_line":"        self.node_cache_time \u003d time.time()"},{"line_number":781,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1ce2392b_bfbfee7c","side":"PARENT","line":778,"updated":"2021-07-02 20:44:47.000000000","message":"So I was just looking at this and I can\u0027t help but wonder if this should also be changed, albeit unrelated, to remove an entry from the cache if present. Just to keep things tidy (Yes, this would be a separate patch), and if the super inefficient \"give me a number of nodes\" call is ever refactored to use the cache, then it would be hyper-important to keep the cache tidy since the hash ring can change based upon participants to the hash ring.","commit_id":"d03a600461db7d0fe0d6a9f522c30b890c691353"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"ade053620787aaf3899e1bae84e9ada8239b8515","unresolved":true,"context_lines":[{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        for node in nodes:"},{"line_number":778,"context_line":"            # NOTE(jroll): we always manage the nodes for instances we manage"},{"line_number":779,"context_line":"            if node.instance_uuid in instances:"},{"line_number":780,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"            # NOTE(jroll): check if the node matches us in the hash ring, and"}],"source_content_type":"text/x-python","patch_set":1,"id":"583dd0e7_6987d5a7","line":779,"range":{"start_line":779,"start_character":15,"end_line":779,"end_character":33},"updated":"2021-07-05 09:55:36.000000000","message":"Does it matter that this association may now be stale? The node may have had an instance_uuid set at the time we queried it, but no longer does, or vice versa.","commit_id":"f84d5917c6fb045f03645d9f80eafbc6e5f94bdd"},{"author":{"_account_id":12640,"name":"Arun S A G","email":"sagarun@gmail.com","username":"sagarun"},"change_message_id":"f99863eb73f7fc93ca04b9f210b7792ed6048c53","unresolved":true,"context_lines":[{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        for node in nodes:"},{"line_number":778,"context_line":"            # NOTE(jroll): we always manage the nodes for instances we manage"},{"line_number":779,"context_line":"            if node.instance_uuid in instances:"},{"line_number":780,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"            # NOTE(jroll): check if the node matches us in the hash ring, and"}],"source_content_type":"text/x-python","patch_set":1,"id":"ebe2bf94_0e44b874","line":779,"range":{"start_line":779,"start_character":15,"end_line":779,"end_character":33},"in_reply_to":"15ecc413_7becc2fc","updated":"2021-07-08 09:09:54.000000000","message":"node1 is managed by a compute named C1 on the hash ring before it was even scheduled. When it is scheduled node1 is deployed by  compute C1. Once this happens C1 always manages that instance for the life time of that instance.\n\n\u003e the nova instance list includes instance1, but because the node we got from the API has instance_uuid \u003d None, we do not add it to the cache (assuming it does not hash to this service)\n\nI thought we always use objects.InstanceList.get_by_host_and_node to figure out which instances are managed by this compute. If C1 manages node1 and instance_uuid is not set, it won\u0027t just abandon that node. objects.InstanceList.get_by_host_and_node should still return the instance uuid and show C1 that the node1 is still managed by it.","commit_id":"f84d5917c6fb045f03645d9f80eafbc6e5f94bdd"},{"author":{"_account_id":12640,"name":"Arun S A G","email":"sagarun@gmail.com","username":"sagarun"},"change_message_id":"a61e76fc4c943672c6c7c0ea855f517e60c85458","unresolved":true,"context_lines":[{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        for node in nodes:"},{"line_number":778,"context_line":"            # NOTE(jroll): we always manage the nodes for instances we manage"},{"line_number":779,"context_line":"            if node.instance_uuid in instances:"},{"line_number":780,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"            # NOTE(jroll): check if the node matches us in the hash ring, and"}],"source_content_type":"text/x-python","patch_set":1,"id":"7fa19144_6f539fd3","line":779,"range":{"start_line":779,"start_character":15,"end_line":779,"end_character":33},"in_reply_to":"583dd0e7_6987d5a7","updated":"2021-07-06 07:36:19.000000000","message":"We have not seen any problems when node.instance_uuid is unset but it is stale. node.instance_uuid will be unset only after deletion of the instance. It may take some time for that node to be managed by the \"right\" compute service. Since we do cleaning on deletion of that instance, there is normally enough time for all compute services to reconcile with where the newly available node will land.","commit_id":"f84d5917c6fb045f03645d9f80eafbc6e5f94bdd"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"5d4388ed4d2411ce3acd7772c2e018f8c635cdee","unresolved":true,"context_lines":[{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        for node in nodes:"},{"line_number":778,"context_line":"            # NOTE(jroll): we always manage the nodes for instances we manage"},{"line_number":779,"context_line":"            if node.instance_uuid in instances:"},{"line_number":780,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"            # NOTE(jroll): check if the node matches us in the hash ring, and"}],"source_content_type":"text/x-python","patch_set":1,"id":"fc4040df_80c9d746","line":779,"range":{"start_line":779,"start_character":15,"end_line":779,"end_character":33},"in_reply_to":"7fa19144_6f539fd3","updated":"2021-07-06 16:03:25.000000000","message":"In thinking about it, there is really no explicit requirement that the instance_uuid is owned by only one nova deployment, and this would just result in any internal cache entry to be dropped. At least, that is my perception. Line 788 would just result in the baremetal node being completely ignored until the next pass-through, which I suspect is fine and was realistically already the case in that usage pattern. In a more run of the mill pattern, as Arun indicates, it should just get picked up on the next pass which *should* actually be better. It is also worth noting that since the node instance_uuid data is from an API query, which may have been removed on a task but not yet committed to the database, instances may already occur where nodes are ignored/removed from the cache pending the next run. However, afaik, that has never been reported as an issue since the node is in cleaning and other processes ignore the node by provision_state elsewhere if memory serves.","commit_id":"f84d5917c6fb045f03645d9f80eafbc6e5f94bdd"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"4082a0d36b4f3bed55305d4a07fc3ccb98c6eb5a","unresolved":true,"context_lines":[{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        for node in nodes:"},{"line_number":778,"context_line":"            # NOTE(jroll): we always manage the nodes for instances we manage"},{"line_number":779,"context_line":"            if node.instance_uuid in instances:"},{"line_number":780,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"            # NOTE(jroll): check if the node matches us in the hash ring, and"}],"source_content_type":"text/x-python","patch_set":1,"id":"fd297a99_98c676e5","line":779,"range":{"start_line":779,"start_character":15,"end_line":779,"end_character":33},"in_reply_to":"ebe2bf94_0e44b874","updated":"2021-07-08 09:19:19.000000000","message":"You\u0027re right, thanks for explaining 😊","commit_id":"f84d5917c6fb045f03645d9f80eafbc6e5f94bdd"},{"author":{"_account_id":14826,"name":"Mark Goddard","email":"markgoddard86@gmail.com","username":"mgoddard"},"change_message_id":"0a46dda191df979f0149b26fe26e5575d3611b34","unresolved":true,"context_lines":[{"line_number":776,"context_line":""},{"line_number":777,"context_line":"        for node in nodes:"},{"line_number":778,"context_line":"            # NOTE(jroll): we always manage the nodes for instances we manage"},{"line_number":779,"context_line":"            if node.instance_uuid in instances:"},{"line_number":780,"context_line":"                node_cache[node.uuid] \u003d node"},{"line_number":781,"context_line":""},{"line_number":782,"context_line":"            # NOTE(jroll): check if the node matches us in the hash ring, and"}],"source_content_type":"text/x-python","patch_set":1,"id":"15ecc413_7becc2fc","line":779,"range":{"start_line":779,"start_character":15,"end_line":779,"end_character":33},"in_reply_to":"fc4040df_80c9d746","updated":"2021-07-08 08:43:09.000000000","message":"Here\u0027s the scenario I\u0027m imagining.\n\n* node list returns node1 with instance_uuid \u003d None\n* the API query takes a long time, during which instance1 is created and scheduled to node1\n* the nova instance list includes instance1, but because the node we got from the API has instance_uuid \u003d None, we do not add it to the cache (assuming it does not hash to this service)\n* other compute services will remove the node from their cache on subsequent refreshes, since they see instance_uuid set\n* the node is now not in any cache, which I believe is the bug we are trying to avoid\n\nUltimately, we\u0027ve got two async queries which are potentially made many seconds apart, and I\u0027m not sure how changing their order results in them being more synchronised.","commit_id":"f84d5917c6fb045f03645d9f80eafbc6e5f94bdd"}]}