)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"e4afbfcf3edf623326c8ba80dbd182ec88576a09","unresolved":false,"context_lines":[{"line_number":32,"context_line":"do that by clearing the provider cache from the resource tracker if"},{"line_number":33,"context_line":"is_new_compute_node is detected."},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"WIP: Tests"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"Change-Id: Ieeaad9783e0ff93377fbc6c7932618d2fac8946a"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3f79a3b5_dbea670b","line":35,"updated":"2018-11-08 14:30:45.000000000","message":"Could probably track this against bug 1742467 which has been open for awhile now. Either here or somewhere else in the series, but it\u0027s definitely related.","commit_id":"e7417a68cf77c3b153b0e00fb89e51323270d6fb"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b3d63a10c14890a6d6def4d2901a7c9caa76367","unresolved":false,"context_lines":[{"line_number":22,"context_line":"/resource_providers?in_tree\u003d$compute_rp) on every periodic. Now we can"},{"line_number":23,"context_line":"optimize that out."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"This should reduce the number of calls to placement per RT periodic to"},{"line_number":26,"context_line":"zero in steady state when [compute]resource_provider_association_refresh"},{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3f79a3b5_e5b25295","line":27,"range":{"start_line":25,"start_character":0,"end_line":27,"end_character":8},"updated":"2018-11-22 10:49:30.000000000","message":"makes total sense after reviewing the patch. thanks.\n\njust a small doubt, wanted to confirm if this is the only part of the periodic update doing inventory refresh: https://github.com/openstack/nova/blob/1a1ea8e2aa66a2654e6cc141c735e47bbd8c4fef/nova/scheduler/client/report.py#L698.\n\nI mean it happens again a second time here too right ? - https://github.com/openstack/nova/blob/1a1ea8e2aa66a2654e6cc141c735e47bbd8c4fef/nova/scheduler/client/report.py#L1032 - after the whole ensure_resource_provider thing ? Or maybe I am seeing this wrong?","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f0b8146766cc2c308ca45aa0829e877ac0e27be4","unresolved":false,"context_lines":[{"line_number":22,"context_line":"/resource_providers?in_tree\u003d$compute_rp) on every periodic. Now we can"},{"line_number":23,"context_line":"optimize that out."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"This should reduce the number of calls to placement per RT periodic to"},{"line_number":26,"context_line":"zero in steady state when [compute]resource_provider_association_refresh"},{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3f79a3b5_ac1f306f","line":27,"range":{"start_line":25,"start_character":0,"end_line":27,"end_character":8},"in_reply_to":"3f79a3b5_9035dade","updated":"2018-11-26 23:23:01.000000000","message":"Being late to the party is a boon here, believe me. The reasons for the doubling are largely historic, and part of what we\u0027re trying to clean up with this series. I can\u0027t say exactly, but I believe it had something to do with ensure and/or refresh being called from multiple code paths. So we had to make sure the refreshing happened in cases where one, but not the other, was called.\n\nBut note that the association refresh timers were in place to make sure that, if we did call refresh twice, the second would be a no-op.","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"976d28e237c664f817cfc49eb5319f63f271c4ea","unresolved":false,"context_lines":[{"line_number":22,"context_line":"/resource_providers?in_tree\u003d$compute_rp) on every periodic. Now we can"},{"line_number":23,"context_line":"optimize that out."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"This should reduce the number of calls to placement per RT periodic to"},{"line_number":26,"context_line":"zero in steady state when [compute]resource_provider_association_refresh"},{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3f79a3b5_9035dade","line":27,"range":{"start_line":25,"start_character":0,"end_line":27,"end_character":8},"in_reply_to":"3f79a3b5_e5b25295","updated":"2018-11-22 10:53:45.000000000","message":"okay, never mind sorry didn\u0027t see that the patchset above this (https://review.openstack.org/#/c/615695/8) is doing exactly the removal of this second refresh. But out of curiosity why did we have this second refresh in the first place ? We were already doing this via the ensure_resource_provider right ? (sorry for being late to the party)","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b3d63a10c14890a6d6def4d2901a7c9caa76367","unresolved":false,"context_lines":[{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"},{"line_number":31,"context_line":"deleted the compute service and restarted it), we force that call to"},{"line_number":32,"context_line":"placement to guarantee that the compute node resource provider is"},{"line_number":33,"context_line":"created. So we do that by clearing the provider cache from the resource"},{"line_number":34,"context_line":"tracker if is_new_compute_node is detected."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"Closes-Bug: #1742467"},{"line_number":37,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"3f79a3b5_e5401261","line":34,"range":{"start_line":33,"start_character":9,"end_line":34,"end_character":42},"updated":"2018-11-22 10:49:30.000000000","message":"cool so that\u0027s what you do here: https://review.openstack.org/#/c/615677/8/nova/compute/resource_tracker.py@811, makes sense.","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd993d682d089d90befd67ac53f22cb144544a4","unresolved":false,"context_lines":[{"line_number":14,"context_line":"With this patch, we instead invalidate the whole cache and fail right"},{"line_number":15,"context_line":"away."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"In real life, since we don\u0027t yet have any implementations of sharing or"},{"line_number":18,"context_line":"nested, this would have been effectively a null change."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Except: this allows us to resolve a TODO whereby we would *always*"},{"line_number":21,"context_line":"_ensure_resource_provider (including a call to GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_0e74e688","line":18,"range":{"start_line":17,"start_character":0,"end_line":18,"end_character":55},"updated":"2018-11-28 23:00:04.000000000","message":"well, yeah, except for ironic, which could effectively have had thousands of entries in the provider tree...","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"aded707a5cb103357e28cb72c84123eb9640bc49","unresolved":false,"context_lines":[{"line_number":14,"context_line":"With this patch, we instead invalidate the whole cache and fail right"},{"line_number":15,"context_line":"away."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"In real life, since we don\u0027t yet have any implementations of sharing or"},{"line_number":18,"context_line":"nested, this would have been effectively a null change."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Except: this allows us to resolve a TODO whereby we would *always*"},{"line_number":21,"context_line":"_ensure_resource_provider (including a call to GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"3f79a3b5_6eed5a8b","line":18,"range":{"start_line":17,"start_character":0,"end_line":18,"end_character":55},"in_reply_to":"3f79a3b5_0e74e688","updated":"2018-11-28 23:15:38.000000000","message":"Oh, yeah, I see your point.\n\nThere was a time in the development of this series (though I must not have ever committed it) when I was only invalidating the tree of the failing provider. That turned out to be nontrivial and hacky to do, so I replaced it with this simplification. But I wasn\u0027t thinking about the ironic case - so I\u0027ll reinstate that.\n\nJust a warning, though - this is going to require some ugly code.","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd04d33d8c55df3b791adeab6a2448e440743972","unresolved":false,"context_lines":[{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"},{"line_number":31,"context_line":"deleted the compute service and restarted it), we force that call to"},{"line_number":32,"context_line":"placement to guarantee that the compute node resource provider is"},{"line_number":33,"context_line":"created. So we do that by clearing the node\u0027s provider cache from the"},{"line_number":34,"context_line":"resource tracker if is_new_compute_node is detected."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"bfdaf3ff_2d2bbfeb","line":31,"range":{"start_line":30,"start_character":21,"end_line":31,"end_character":44},"updated":"2019-01-14 13:52:23.000000000","message":"deleting the compute service shouldn\u0027t affect whether the resource provider record for that hypervisor_hostname exists in the placement service. If that was the case, deleting a nova-compute service that manages Ironic nodes would end up attempting to delete potentially thousands of resource provider records -- and it\u0027s not like those Ironic nodes actually were deleted (they still potentially have instances on them, therefore allocations in placement, therefore the provider records could not be deleted anyway).\n\nI think what you\u0027re saying here, though, is that restarting the compute service should re-fetch and populate the provider cache (the ProviderTree), which of course is correct.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"520742ccdf062431e852bac724f6ee68d6e78395","unresolved":false,"context_lines":[{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"},{"line_number":31,"context_line":"deleted the compute service and restarted it), we force that call to"},{"line_number":32,"context_line":"placement to guarantee that the compute node resource provider is"},{"line_number":33,"context_line":"created. So we do that by clearing the node\u0027s provider cache from the"},{"line_number":34,"context_line":"resource tracker if is_new_compute_node is detected."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"bfdaf3ff_4a07fe1d","line":31,"range":{"start_line":30,"start_character":21,"end_line":31,"end_character":44},"in_reply_to":"bfdaf3ff_2d2bbfeb","updated":"2019-01-14 21:41:40.000000000","message":"\u003e they still potentially have instances on them, therefore allocations in placement, therefore the provider records could not be deleted anyway\n\nRegarding deleting compute services, we do check that there aren\u0027t any instances on them (I think that was new in Rocky or Queens and backported):\n\nhttps://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/api/openstack/compute/services.py#L232","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8bb146a717bf6b67c949279a0c660840c79211b","unresolved":false,"context_lines":[{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"},{"line_number":31,"context_line":"deleted the compute service and restarted it), we force that call to"},{"line_number":32,"context_line":"placement to guarantee that the compute node resource provider is"},{"line_number":33,"context_line":"created. So we do that by clearing the node\u0027s provider cache from the"},{"line_number":34,"context_line":"resource tracker if is_new_compute_node is detected."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"bfdaf3ff_4a593e58","line":31,"range":{"start_line":30,"start_character":21,"end_line":31,"end_character":44},"in_reply_to":"bfdaf3ff_2d2bbfeb","updated":"2019-01-14 22:53:44.000000000","message":"I thought deleting the compute service did in fact delete the placement records for the compute node resource provider(s).\n\n[Later] Hm, [1] makes it look like we do delete the provider for non-ironic deployments. But for ironic deployments, because of [2], we would only delete the \"first\" node\u0027s provider. That seems broken to me.\n\nIn any case, at least for non-ironic nodes, the statement seems to be correct. Unless I\u0027m missing something?\n\n[More laterer] Talked this over with mriedem a bit [3]. We think the \"potentially thousands\" of providers do in fact get deleted and recreated - just not necessarily at the ideal / most sensible times. Wound up opening a bug [4].\n\n[1] https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/api/openstack/compute/services.py#L244-L247\n[2] https://github.com/openstack/nova/blob/da98f4ba4554139b3901103aa0d26876b11e1d9a/nova/objects/service.py#L308-L311\n[3] http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-01-14.log.html#t2019-01-14T21:36:18\n[4] https://bugs.launchpad.net/nova/+bug/1811726","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5c7c186776e379dfe80df6799e4a40200b59d370","unresolved":false,"context_lines":[{"line_number":27,"context_line":"is zero."},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"But we also need to make sure that, whenever we\u0027re bootstrapping the"},{"line_number":30,"context_line":"compute node record (either for the first time, or because the operator"},{"line_number":31,"context_line":"deleted the compute service and restarted it), we force that call to"},{"line_number":32,"context_line":"placement to guarantee that the compute node resource provider is"},{"line_number":33,"context_line":"created. So we do that by clearing the node\u0027s provider cache from the"},{"line_number":34,"context_line":"resource tracker if is_new_compute_node is detected."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":17,"id":"bfdaf3ff_88840f60","line":31,"range":{"start_line":30,"start_character":21,"end_line":31,"end_character":44},"in_reply_to":"bfdaf3ff_4a07fe1d","updated":"2019-01-14 21:51:40.000000000","message":"More IRC discussion related to this:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-01-14.log.html#t2019-01-14T21:35:45","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ddd4766db5194f584c11632d3923d953df5bf312","unresolved":false,"context_lines":[{"line_number":14,"context_line":"With this patch, we instead invalidate the tree around the failing"},{"line_number":15,"context_line":"provider and fail right away."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"In real life, since we don\u0027t yet have any implementations of nested,"},{"line_number":18,"context_line":"this would have been effectively a null change."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Except: this allows us to resolve a TODO whereby we would *always*"},{"line_number":21,"context_line":"_ensure_resource_provider (including a call to GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"9fdfeff1_6327cab8","line":18,"range":{"start_line":17,"start_character":0,"end_line":18,"end_character":47},"updated":"2019-01-18 21:40:06.000000000","message":"So, when we do support nested, like nested vgpu, what do we lose, or what might we need to add back? Because I\u0027m sure I\u0027ll for one forget all about this.","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e596206f63959058d1bf38191cbfa13f7fbd35ca","unresolved":false,"context_lines":[{"line_number":14,"context_line":"With this patch, we instead invalidate the tree around the failing"},{"line_number":15,"context_line":"provider and fail right away."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"In real life, since we don\u0027t yet have any implementations of nested,"},{"line_number":18,"context_line":"this would have been effectively a null change."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Except: this allows us to resolve a TODO whereby we would *always*"},{"line_number":21,"context_line":"_ensure_resource_provider (including a call to GET"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":20,"id":"9fdfeff1_1e7df16d","line":18,"range":{"start_line":17,"start_character":0,"end_line":18,"end_character":47},"in_reply_to":"9fdfeff1_6327cab8","updated":"2019-01-18 22:35:00.000000000","message":"The code was and is already written to accommodate nested structures. This is just clarifying that, when you have a tree of one, invalidating \"from here down\" is the same as invalidating \"the whole tree\".\n\nThe algorithm was originally written to be best-effort: to effect as many of the changes in as many of the providers as possible, even if an operation fails for some subset of the providers. Now we bail on first failure. Theoretically the best-effort version could have been more efficient, requiring fewer retries to flush everything through. But what was concluded in working this problem was that that theoretical gain wasn\u0027t worth the added complexity and the inability to exploit the cache fully.","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"}],"nova/compute/provider_tree.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"450befc03d0f891cb7a1a30432d1b0b4bfd2cfb8","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        self.lock \u003d lockutils.internal_lock(_LOCK_NAME)"},{"line_number":235,"context_line":"        self.roots \u003d []"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def get_provider_uuids(self, name_or_uuid\u003dNone, whole_tree\u003dFalse):"},{"line_number":238,"context_line":"        \"\"\"Return a list, in top-down traversable order, of the UUIDs of all"},{"line_number":239,"context_line":"        providers (in a (sub)tree)."},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_1bf08d93","line":237,"updated":"2018-11-29 23:04:26.000000000","message":"UT for this","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"543638850513ae2e4483fad61a96e78bc9c10eb4","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        self.lock \u003d lockutils.internal_lock(_LOCK_NAME)"},{"line_number":235,"context_line":"        self.roots \u003d []"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"    def get_provider_uuids(self, name_or_uuid\u003dNone, whole_tree\u003dFalse):"},{"line_number":238,"context_line":"        \"\"\"Return a list, in top-down traversable order, of the UUIDs of all"},{"line_number":239,"context_line":"        providers (in a (sub)tree)."},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_1f2ce892","line":237,"in_reply_to":"3f79a3b5_1bf08d93","updated":"2018-11-30 17:23:16.000000000","message":"Done","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"450befc03d0f891cb7a1a30432d1b0b4bfd2cfb8","unresolved":false,"context_lines":[{"line_number":391,"context_line":"            self.roots.append(p)"},{"line_number":392,"context_line":"            return p.uuid"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def _find_with_lock(self, name_or_uuid, return_root\u003dFalse):"},{"line_number":395,"context_line":"        for root in self.roots:"},{"line_number":396,"context_line":"            found \u003d root.find(name_or_uuid)"},{"line_number":397,"context_line":"            if found:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_9b21fd0c","line":394,"updated":"2018-11-29 23:04:26.000000000","message":"UT for this","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"543638850513ae2e4483fad61a96e78bc9c10eb4","unresolved":false,"context_lines":[{"line_number":391,"context_line":"            self.roots.append(p)"},{"line_number":392,"context_line":"            return p.uuid"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"    def _find_with_lock(self, name_or_uuid, return_root\u003dFalse):"},{"line_number":395,"context_line":"        for root in self.roots:"},{"line_number":396,"context_line":"            found \u003d root.find(name_or_uuid)"},{"line_number":397,"context_line":"            if found:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_3f29a482","line":394,"in_reply_to":"3f79a3b5_9b21fd0c","updated":"2018-11-30 17:23:16.000000000","message":"Covered handily by the UT for the above.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"06cb4640ab2c4dc42961d10d8448f2a41b30b6c6","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                           descendants of the provider specified by"},{"line_number":248,"context_line":"                           name_or_uuid; otherwise it returns UUIDs of all the"},{"line_number":249,"context_line":"                           providers in the associated tree (but not other"},{"line_number":250,"context_line":"                           trees in the ProviderTree object)."},{"line_number":251,"context_line":"        \"\"\""},{"line_number":252,"context_line":"        if name_or_uuid is not None:"},{"line_number":253,"context_line":"            with self.lock:"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_3a2bd730","line":250,"updated":"2019-01-15 15:03:44.000000000","message":"If you want to be super clear here, you could say \"all the providers in the whole tree of which the provider named in name_or_uuid is a member\" or something like that. It took me a moment to translate \"associated\" properly.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ec7ac860f0881450514bda2f8835eb35865650db","unresolved":false,"context_lines":[{"line_number":247,"context_line":"                           descendants of the provider specified by"},{"line_number":248,"context_line":"                           name_or_uuid; otherwise it returns UUIDs of all the"},{"line_number":249,"context_line":"                           providers in the associated tree (but not other"},{"line_number":250,"context_line":"                           trees in the ProviderTree object)."},{"line_number":251,"context_line":"        \"\"\""},{"line_number":252,"context_line":"        if name_or_uuid is not None:"},{"line_number":253,"context_line":"            with self.lock:"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_aef8e948","line":250,"in_reply_to":"bfdaf3ff_3a2bd730","updated":"2019-01-15 19:09:28.000000000","message":"Done (in the docstring of the new method)","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd04d33d8c55df3b791adeab6a2448e440743972","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        with self.lock:"},{"line_number":260,"context_line":"            for root in self.roots:"},{"line_number":261,"context_line":"                ret.extend(root.get_provider_uuids())"},{"line_number":262,"context_line":"        return ret"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def populate_from_iterable(self, provider_dicts):"},{"line_number":265,"context_line":"        \"\"\"Populates this ProviderTree from an iterable of provider dicts."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_2d317f0c","line":262,"updated":"2019-01-14 13:52:23.000000000","message":"Recommend having get_provider_uuids() do one thing only. Consider adding a get_provider_uuids_in_tree() method that does the tree-traversal stuff you need to call from the resource tracker in these special circumstances. Having boolean arguments that change the returned result value or behaviour is a code smell, IMHO.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8bb146a717bf6b67c949279a0c660840c79211b","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        with self.lock:"},{"line_number":260,"context_line":"            for root in self.roots:"},{"line_number":261,"context_line":"                ret.extend(root.get_provider_uuids())"},{"line_number":262,"context_line":"        return ret"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def populate_from_iterable(self, provider_dicts):"},{"line_number":265,"context_line":"        \"\"\"Populates this ProviderTree from an iterable of provider dicts."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_e8991b79","line":262,"in_reply_to":"bfdaf3ff_2d317f0c","updated":"2019-01-14 22:53:44.000000000","message":"I considered that, but felt the smell was already perpetrated by the name_or_uuid kwarg returning the *sub*tree. If I could turn back time (♪) I would have get_provider_uuids(name_or_uuid\u003dNone) always return whole trees, and a different get_subtree_uuids(name_or_uuid) return subtree. That would be a pretty big change, I think.\n\nI can change this if you feel strongly about it, but IMO it\u0027s ugly either way.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ec7ac860f0881450514bda2f8835eb35865650db","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        with self.lock:"},{"line_number":260,"context_line":"            for root in self.roots:"},{"line_number":261,"context_line":"                ret.extend(root.get_provider_uuids())"},{"line_number":262,"context_line":"        return ret"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def populate_from_iterable(self, provider_dicts):"},{"line_number":265,"context_line":"        \"\"\"Populates this ProviderTree from an iterable of provider dicts."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_0e9fb503","line":262,"in_reply_to":"bfdaf3ff_7aa4ffb9","updated":"2019-01-15 19:09:28.000000000","message":"Okay, done.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"06cb4640ab2c4dc42961d10d8448f2a41b30b6c6","unresolved":false,"context_lines":[{"line_number":259,"context_line":"        with self.lock:"},{"line_number":260,"context_line":"            for root in self.roots:"},{"line_number":261,"context_line":"                ret.extend(root.get_provider_uuids())"},{"line_number":262,"context_line":"        return ret"},{"line_number":263,"context_line":""},{"line_number":264,"context_line":"    def populate_from_iterable(self, provider_dicts):"},{"line_number":265,"context_line":"        \"\"\"Populates this ProviderTree from an iterable of provider dicts."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_7aa4ffb9","line":262,"in_reply_to":"bfdaf3ff_e8991b79","updated":"2019-01-15 15:03:44.000000000","message":"I\u0027m mildly with Jay on this. The initial functionality of the method was to do \"from here down\", and if name_or_uuid was not set, the top of the tree is \"here\". that makes some form of sense.\n\nwhole_tree is new functionality which suggests a new method, which calls this one with the right args.\n\nI don\u0027t think it\u0027s huge deal, but it _might_ be less confusing.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c69dea96380a56108691f8793253294f865c9c0","unresolved":false,"context_lines":[{"line_number":804,"context_line":"        # but it is. This should be changed in ComputeNode"},{"line_number":805,"context_line":"        cn.metrics \u003d jsonutils.dumps(metrics)"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        # We need to make sure to create the resource provider in placement, if"},{"line_number":808,"context_line":"        # it\u0027s not already there."},{"line_number":809,"context_line":"        if is_new_compute_node:"},{"line_number":810,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":811,"context_line":"        # update the compute_node"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_9ca97bdf","line":808,"range":{"start_line":807,"start_character":8,"end_line":808,"end_character":33},"updated":"2018-11-06 15:19:17.000000000","message":"And that happens somewhere else down in _update_to_placement I\u0027m assuming when we call reportclient.update_from_provider_tree? Since this is a bit tightly coupled to what happens down there, you should probably add that detail to the comment, otherwise pass the is_new_compute_node through that call path. It\u0027s not immediately obvious to me how clearing the cache here means we create (or ensure) the resource provider for the newly created compute node later.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"18ef270e76f92809b2d142aa0130ec2bccac839f","unresolved":false,"context_lines":[{"line_number":804,"context_line":"        # but it is. This should be changed in ComputeNode"},{"line_number":805,"context_line":"        cn.metrics \u003d jsonutils.dumps(metrics)"},{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        # We need to make sure to create the resource provider in placement, if"},{"line_number":808,"context_line":"        # it\u0027s not already there."},{"line_number":809,"context_line":"        if is_new_compute_node:"},{"line_number":810,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":811,"context_line":"        # update the compute_node"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_c5acf6a9","line":808,"range":{"start_line":807,"start_character":8,"end_line":808,"end_character":33},"in_reply_to":"3f79a3b5_9ca97bdf","updated":"2018-11-06 20:32:26.000000000","message":"Yeah, I had another version where I passed is_new_compute_node through [1], and was really on the fence about it, but I think this is cleaner. If adding more words to the comment brings you on board with this version, I\u0027ll do that.\n\nhttps://review.openstack.org/#/c/614886/2/nova/compute/resource_tracker.py","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"e5126bd51b91226403761eaa831445e6f27f46f7","unresolved":false,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        # We need to make sure to create the resource provider in placement, if"},{"line_number":808,"context_line":"        # it\u0027s not already there."},{"line_number":809,"context_line":"        if is_new_compute_node:"},{"line_number":810,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":811,"context_line":"        # update the compute_node"},{"line_number":812,"context_line":"        self._update(context, cn, startup\u003dstartup)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_042bdbe9","line":809,"updated":"2018-11-06 11:29:12.000000000","message":"(thinking out loud again) this will be true only when there wasn\u0027t a compute node record in the database, so when the nova-compute process runs, then stops, then runs again clear... won\u0027t be called","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"18ef270e76f92809b2d142aa0130ec2bccac839f","unresolved":false,"context_lines":[{"line_number":806,"context_line":""},{"line_number":807,"context_line":"        # We need to make sure to create the resource provider in placement, if"},{"line_number":808,"context_line":"        # it\u0027s not already there."},{"line_number":809,"context_line":"        if is_new_compute_node:"},{"line_number":810,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":811,"context_line":"        # update the compute_node"},{"line_number":812,"context_line":"        self._update(context, cn, startup\u003dstartup)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_597775d3","line":809,"in_reply_to":"3f79a3b5_042bdbe9","updated":"2018-11-06 20:32:26.000000000","message":"Nope, but the cache will be empty at that point anyway, so whether the provider exists at that point or not, we\u0027re covered.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd993d682d089d90befd67ac53f22cb144544a4","unresolved":false,"context_lines":[{"line_number":809,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":810,"context_line":"        # placement for performance reasons."},{"line_number":811,"context_line":"        if is_new_compute_node:"},{"line_number":812,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":813,"context_line":"        # update the compute_node"},{"line_number":814,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":815,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_4ecc1ebd","line":812,"updated":"2018-11-28 23:00:04.000000000","message":"right, but as mentioned in my commit message comment, this will also blow away all compute node providers representing Ironic nodes... which could number in the thousands. So, each time a new Ironic node is added to a nova-compute service with thousands of existing compute nodes already being tracked, the entire provider tree will be blown away and slowly re-created each time update_available_resource() is called for each Ironic node...","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a1114478112ba0699344d1c9bdb08470ff8274c3","unresolved":false,"context_lines":[{"line_number":819,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":820,"context_line":"        # placement for performance reasons."},{"line_number":821,"context_line":"        if is_new_compute_node:"},{"line_number":822,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":823,"context_line":"        # update the compute_node"},{"line_number":824,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":825,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_94660e27","line":822,"range":{"start_line":822,"start_character":47,"end_line":822,"end_character":69},"updated":"2018-12-07 14:54:54.000000000","message":"the problem that jaypipes raised here for ironic in PS9 is not addressed for this portion right ? where the new computes/nodes get added for ironic ? or am I missing something?","commit_id":"5375ef6b6acbabddafe0c405fa0586a1f54c597a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"676a78764c51c3801fe02434005a1e5c2b776e65","unresolved":false,"context_lines":[{"line_number":819,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":820,"context_line":"        # placement for performance reasons."},{"line_number":821,"context_line":"        if is_new_compute_node:"},{"line_number":822,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":823,"context_line":"        # update the compute_node"},{"line_number":824,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":825,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_865496f2","line":822,"range":{"start_line":822,"start_character":47,"end_line":822,"end_character":69},"in_reply_to":"3f79a3b5_1762fdda","updated":"2018-12-08 14:41:49.000000000","message":"Ah, okay, I fixed it in the ufpt code path, but it makes sense why it should be done here as well.\n\nI\u0027ll need to make _clear_provider_cache_for_tree public to do it; but otherwise the change should be fairly simple.","commit_id":"5375ef6b6acbabddafe0c405fa0586a1f54c597a"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"efda100f5a2b720731d5b1a8f65e313d689480e7","unresolved":false,"context_lines":[{"line_number":819,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":820,"context_line":"        # placement for performance reasons."},{"line_number":821,"context_line":"        if is_new_compute_node:"},{"line_number":822,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":823,"context_line":"        # update the compute_node"},{"line_number":824,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":825,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_1762fdda","line":822,"range":{"start_line":822,"start_character":47,"end_line":822,"end_character":69},"in_reply_to":"3f79a3b5_643aa1ef","updated":"2018-12-07 16:46:57.000000000","message":"Correct that this is the primary problem I identified for Ironic deployments in the earlier revision. A coarse-grained cache invalidation like the above clear_provider_cache() will cause a lot of unnecessary traffic for a nova-compute service that manages lots of Ironic nodes: as the resource tracker\u0027s update_available_resource() method is called, once for each Ironic node, the ProviderTree.new_root() method will be called after calling placement to re-fetch a provider\u0027s information. All of that is mostly unnecessary, since much of the information about Ironic nodes will remain pretty static.","commit_id":"5375ef6b6acbabddafe0c405fa0586a1f54c597a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0da14033b71a6fdcf6d5aaa245ee99c2d25b0968","unresolved":false,"context_lines":[{"line_number":819,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":820,"context_line":"        # placement for performance reasons."},{"line_number":821,"context_line":"        if is_new_compute_node:"},{"line_number":822,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":823,"context_line":"        # update the compute_node"},{"line_number":824,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":825,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"bfdaf3ff_533b5937","line":822,"range":{"start_line":822,"start_character":47,"end_line":822,"end_character":69},"in_reply_to":"3f79a3b5_865496f2","updated":"2019-01-11 23:13:52.000000000","message":"Done","commit_id":"5375ef6b6acbabddafe0c405fa0586a1f54c597a"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"527f6f94cc04a1d2790ddce62ada8dbb3f5ff94d","unresolved":false,"context_lines":[{"line_number":819,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":820,"context_line":"        # placement for performance reasons."},{"line_number":821,"context_line":"        if is_new_compute_node:"},{"line_number":822,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache()"},{"line_number":823,"context_line":"        # update the compute_node"},{"line_number":824,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":825,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":14,"id":"3f79a3b5_643aa1ef","line":822,"range":{"start_line":822,"start_character":47,"end_line":822,"end_character":69},"in_reply_to":"3f79a3b5_94660e27","updated":"2018-12-07 16:41:17.000000000","message":"I mean shouldn\u0027t this also be _clear_provider_cache_for_tree for the ironic new nodes?","commit_id":"5375ef6b6acbabddafe0c405fa0586a1f54c597a"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f0065b28c994753fc2fa9472871bbefd5626bb5","unresolved":false,"context_lines":[{"line_number":785,"context_line":"        self._pair_instances_to_migrations(migrations, instance_by_uuid)"},{"line_number":786,"context_line":"        self._update_usage_from_migrations(context, migrations, nodename)"},{"line_number":787,"context_line":""},{"line_number":788,"context_line":"        # A new compute node means there won\u0027t be a resource provider yet since"},{"line_number":789,"context_line":"        # that would be created via the _update() call below, and if there is"},{"line_number":790,"context_line":"        # no resource provider then there are no allocations against it."},{"line_number":791,"context_line":"        if not is_new_compute_node:"},{"line_number":792,"context_line":"            self._remove_deleted_instances_allocations("}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_cb91b19b","line":789,"range":{"start_line":788,"start_character":8,"end_line":789,"end_character":74},"updated":"2019-01-14 23:00:49.000000000","message":"Based on this, the resource provider shouldn\u0027t be in the cache if the resource provider doesn\u0027t exist, and it shouldn\u0027t exist until get_provider_tree_and_ensure_root is called, right?","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd04d33d8c55df3b791adeab6a2448e440743972","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":821,"context_line":"        # placement for performance reasons."},{"line_number":822,"context_line":"        if is_new_compute_node:"},{"line_number":823,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache_for_tree("},{"line_number":824,"context_line":"                cn.uuid)"},{"line_number":825,"context_line":"        # update the compute_node"},{"line_number":826,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":827,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_3828db84","line":824,"range":{"start_line":823,"start_character":12,"end_line":824,"end_character":24},"updated":"2019-01-14 13:52:23.000000000","message":"I guess I\u0027m not following this... if it\u0027s a new compute node, why would the scheduler client provider cache have anything in it for this provider?","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8bb146a717bf6b67c949279a0c660840c79211b","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":821,"context_line":"        # placement for performance reasons."},{"line_number":822,"context_line":"        if is_new_compute_node:"},{"line_number":823,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache_for_tree("},{"line_number":824,"context_line":"                cn.uuid)"},{"line_number":825,"context_line":"        # update the compute_node"},{"line_number":826,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":827,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_88ddef15","line":824,"range":{"start_line":823,"start_character":12,"end_line":824,"end_character":24},"in_reply_to":"bfdaf3ff_3828db84","updated":"2019-01-14 22:53:44.000000000","message":"Good question. Could it be... \"just in case\"?\n\nDammit, a month off really screwed with my recollection of the whys and wherefores of this series. Is it possible that \"is_new_compute_node\" doesn\u0027t always exactly mean it\u0027s a new compute node - at least from the perspective of guaranteeing the node isn\u0027t in the cache?","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ec7ac860f0881450514bda2f8835eb35865650db","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":821,"context_line":"        # placement for performance reasons."},{"line_number":822,"context_line":"        if is_new_compute_node:"},{"line_number":823,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache_for_tree("},{"line_number":824,"context_line":"                cn.uuid)"},{"line_number":825,"context_line":"        # update the compute_node"},{"line_number":826,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":827,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_cb77713a","line":824,"range":{"start_line":823,"start_character":12,"end_line":824,"end_character":24},"in_reply_to":"bfdaf3ff_6bf685fc","updated":"2019-01-15 19:09:28.000000000","message":"Bah. It\u0027s probably a leftover from an earlier iteration of the code when we were using the condition to make _ensure do extra refreshing down the tree, or something. I\u0027ll remove it and hope there wasn\u0027t a good reason for it. It\u0027ll make the code simpler anyway.\n\n(Note to self: we also talked about it here [1], which contains a link to the prototype code, which demonstrates the \"extra refreshing\" or something like it.)\n\n[1] https://review.openstack.org/#/c/615677/2/nova/compute/resource_tracker.py@808","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"5f0065b28c994753fc2fa9472871bbefd5626bb5","unresolved":false,"context_lines":[{"line_number":820,"context_line":"        # it\u0027s not already there. Otherwise _update could skip querying"},{"line_number":821,"context_line":"        # placement for performance reasons."},{"line_number":822,"context_line":"        if is_new_compute_node:"},{"line_number":823,"context_line":"            self.scheduler_client.reportclient.clear_provider_cache_for_tree("},{"line_number":824,"context_line":"                cn.uuid)"},{"line_number":825,"context_line":"        # update the compute_node"},{"line_number":826,"context_line":"        self._update(context, cn, startup\u003dstartup)"},{"line_number":827,"context_line":"        LOG.debug(\u0027Compute_service record updated for %(host)s:%(node)s\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_6bf685fc","line":824,"range":{"start_line":823,"start_character":12,"end_line":824,"end_character":24},"in_reply_to":"bfdaf3ff_88ddef15","updated":"2019-01-14 23:00:49.000000000","message":"Maybe you\u0027re conflating the is_new_compute_node variable with the startup variable? Because I remember us talking in circles about the need at one point to differentiate those two scenarios.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1c69dea96380a56108691f8793253294f865c9c0","unresolved":false,"context_lines":[{"line_number":668,"context_line":"        # - \"Cascading generations\" - i.e. a change to a leaf node percolates"},{"line_number":669,"context_line":"        #   generation bump up the tree so that we bounce 409 the next time we"},{"line_number":670,"context_line":"        #   try to update anything and have to refresh."},{"line_number":671,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":672,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":673,"context_line":"            for u in self._provider_tree.get_provider_uuids(uuid):"},{"line_number":674,"context_line":"                self._refresh_associations(context, u, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_7cb33f36","line":671,"range":{"start_line":671,"start_character":8,"end_line":671,"end_character":44},"updated":"2018-11-06 15:19:17.000000000","message":"OK so this is where the clear_provider_cache() from the \"if is_new_compute_node\" block comes into play. In the case of a new compute node, this would be False so we\u0027d drop down and ensure the provider exists by creating it if it doesn\u0027t already exist.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"18ef270e76f92809b2d142aa0130ec2bccac839f","unresolved":false,"context_lines":[{"line_number":668,"context_line":"        # - \"Cascading generations\" - i.e. a change to a leaf node percolates"},{"line_number":669,"context_line":"        #   generation bump up the tree so that we bounce 409 the next time we"},{"line_number":670,"context_line":"        #   try to update anything and have to refresh."},{"line_number":671,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":672,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":673,"context_line":"            for u in self._provider_tree.get_provider_uuids(uuid):"},{"line_number":674,"context_line":"                self._refresh_associations(context, u, force\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_a5753a35","line":671,"range":{"start_line":671,"start_character":8,"end_line":671,"end_character":44},"in_reply_to":"3f79a3b5_7cb33f36","updated":"2018-11-06 20:32:26.000000000","message":"Correctamundo","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e5b312f42d68796cefa6151f7e3c90d74ed2cf01","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        #   try to update anything and have to refresh."},{"line_number":671,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":672,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":673,"context_line":"            for u in self._provider_tree.get_provider_uuids(uuid):"},{"line_number":674,"context_line":"                self._refresh_associations(context, u, force\u003dFalse)"},{"line_number":675,"context_line":"            return uuid"},{"line_number":676,"context_line":""},{"line_number":677,"context_line":"        # We either don\u0027t have it locally, or it\u0027s stale. Pull it or create it."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_9110794e","line":674,"range":{"start_line":673,"start_character":0,"end_line":674,"end_character":67},"updated":"2018-11-06 22:24:57.000000000","message":"The condition above is masking checking the refresh timer for all providers in the subtree. That\u0027s probably usually not a big deal, but it\u0027s not strictly right.\n\nWhat we should do instead is use the conditional to decide whether to get rps_to_refresh from placement or from the cache, and then continue with the refresh-if-stale loop below.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"e5126bd51b91226403761eaa831445e6f27f46f7","unresolved":false,"context_lines":[{"line_number":1578,"context_line":"            try:"},{"line_number":1579,"context_line":"                yield"},{"line_number":1580,"context_line":"            except helper_exceptions:"},{"line_number":1581,"context_line":"                # Invalidate the cache. It gets rebuilt on the next pass."},{"line_number":1582,"context_line":"                self.clear_provider_cache()"},{"line_number":1583,"context_line":"                raise exception.ResourceProviderSyncFailed()"},{"line_number":1584,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_3f414038","line":1581,"updated":"2018-11-06 11:29:12.000000000","message":"Has there been any discussion of redoing whatever we were trying to do, now, instead of the next pass? For some people they may want to have long intervals between now and \"next\". Does that matter?","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"18ef270e76f92809b2d142aa0130ec2bccac839f","unresolved":false,"context_lines":[{"line_number":1578,"context_line":"            try:"},{"line_number":1579,"context_line":"                yield"},{"line_number":1580,"context_line":"            except helper_exceptions:"},{"line_number":1581,"context_line":"                # Invalidate the cache. It gets rebuilt on the next pass."},{"line_number":1582,"context_line":"                self.clear_provider_cache()"},{"line_number":1583,"context_line":"                raise exception.ResourceProviderSyncFailed()"},{"line_number":1584,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_45d1c6f8","line":1581,"in_reply_to":"3f79a3b5_3f414038","updated":"2018-11-06 20:32:26.000000000","message":"Yes, I agree, I think we have a hole here that needs to be closed. I used to think it was okay to fold the conflict exception in with the rest so the next periodic would be the retry, but we can do much better than that:\n\nWe have a retry decorator on _update that\u0027s set to trigger on ResourceProviderUpdateConflict. But because that one is getting swallowed by L1571 along with the rest, I don\u0027t think we would ever actually be taking advantage of it. If I pull that one out and let this method raise it, we should get that re-GET, re-upt, re-ufpt cycle right away, which is what we really want.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"e5126bd51b91226403761eaa831445e6f27f46f7","unresolved":false,"context_lines":[{"line_number":1580,"context_line":"            except helper_exceptions:"},{"line_number":1581,"context_line":"                # Invalidate the cache. It gets rebuilt on the next pass."},{"line_number":1582,"context_line":"                self.clear_provider_cache()"},{"line_number":1583,"context_line":"                raise exception.ResourceProviderSyncFailed()"},{"line_number":1584,"context_line":""},{"line_number":1585,"context_line":"        # Helper methods herein will be updating the local cache (this is"},{"line_number":1586,"context_line":"        # intentional) so we need to grab up front any data we need to operate"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_3f2ce092","line":1583,"updated":"2018-11-06 11:29:12.000000000","message":"Using ResourceProviderUpdateFailed as an example, I find it pretty hard to track and confirm that there is a log of some kind when such an error happens. I did find a LOG.error near the two places where it is raised, so there is some audit-ability, but it might be hard to associate that log message with the exception raised here because we\u0027re swallowing the first one and this second one is not explicitly caught and logged when _update is called, it just percolates out as part of the top-level exception handling in place to keep the long-running process alive.\n\nNot necessarily here, but at some point, we may wish to pass the helper_exception as exc into ResourceProviderSyncFailed so when that top-level logging happens, we have some association. Or we may wish to do some logging here.\n\nFailures during periodic jobs and the RPC handlers tend to be terribly hard to debug for people out there, so the more noise we can get into the logs the better.\n\nThat said, this particular situation isn\u0027t so much a failure as one of several expected ways in which things can temporarily go wrong that will be fixed later. However, if things get into a situation where the excepted things keep happening (perhaps because of concurrency issues) we will need lots of log in order to have any visibility.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"18ef270e76f92809b2d142aa0130ec2bccac839f","unresolved":false,"context_lines":[{"line_number":1580,"context_line":"            except helper_exceptions:"},{"line_number":1581,"context_line":"                # Invalidate the cache. It gets rebuilt on the next pass."},{"line_number":1582,"context_line":"                self.clear_provider_cache()"},{"line_number":1583,"context_line":"                raise exception.ResourceProviderSyncFailed()"},{"line_number":1584,"context_line":""},{"line_number":1585,"context_line":"        # Helper methods herein will be updating the local cache (this is"},{"line_number":1586,"context_line":"        # intentional) so we need to grab up front any data we need to operate"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f023ee55","line":1583,"in_reply_to":"3f79a3b5_3f2ce092","updated":"2018-11-06 20:32:26.000000000","message":"When I originally wrote ufpt, I did make sure that the methods responsible for raising these exceptions were logging appropriately before they did so. Between that and the log at the top level of the rt periodic, I thought the logs were noisy enough, and anything more would be redundant. But I can add more (in a separate patch) if you think it\u0027s necessary.","commit_id":"073c330f52dfa535e6f787f5bb2fc8ec52f1be9a"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b3d63a10c14890a6d6def4d2901a7c9caa76367","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        else:"},{"line_number":678,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":679,"context_line":"            created_rp \u003d None"},{"line_number":680,"context_line":"            rps_to_refresh \u003d self._get_providers_in_tree(context, uuid)"},{"line_number":681,"context_line":"            if not rps_to_refresh:"},{"line_number":682,"context_line":"                created_rp \u003d self._create_resource_provider("},{"line_number":683,"context_line":"                    context, uuid, name or uuid,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_258e0a26","line":680,"range":{"start_line":680,"start_character":34,"end_line":680,"end_character":56},"updated":"2018-11-22 10:49:30.000000000","message":"okay so this is the main optimization factor here: basically before this change we were always doing this call (i.e GET/resource_providers?in_tree\u003drp_uuid) and now we check \n\n1) if the provider tree cache of the resource provider with uuid exists already and if its fresh enough to not need refresh, then we go into the cache of the rest of the descendant nodes under this tree and directly add those node uuids whose cache is stale and needs refreshing (which would never be stake if association_refresh config is disabled). so that way we do not need to query placement to get the \"in_tree?\" nodes, since we pick it up from the cache of the provided uuid.\n\n2) only if either the tree doesn\u0027t exist (which would only be during startup and/or sighup) or if its cache is stale which wouldn\u0027t happen if the association_refresh config is disabled, would it do the \"in_tree?\" query to placement.","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f0b8146766cc2c308ca45aa0829e877ac0e27be4","unresolved":false,"context_lines":[{"line_number":677,"context_line":"        else:"},{"line_number":678,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":679,"context_line":"            created_rp \u003d None"},{"line_number":680,"context_line":"            rps_to_refresh \u003d self._get_providers_in_tree(context, uuid)"},{"line_number":681,"context_line":"            if not rps_to_refresh:"},{"line_number":682,"context_line":"                created_rp \u003d self._create_resource_provider("},{"line_number":683,"context_line":"                    context, uuid, name or uuid,"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_c70da536","line":680,"range":{"start_line":680,"start_character":34,"end_line":680,"end_character":56},"in_reply_to":"3f79a3b5_258e0a26","updated":"2018-11-26 23:23:01.000000000","message":"Correctamundo.\n\nIf you think there\u0027s a way that could be made clearer with comments, please feel free to author something.","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b3d63a10c14890a6d6def4d2901a7c9caa76367","unresolved":false,"context_lines":[{"line_number":678,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":679,"context_line":"            created_rp \u003d None"},{"line_number":680,"context_line":"            rps_to_refresh \u003d self._get_providers_in_tree(context, uuid)"},{"line_number":681,"context_line":"            if not rps_to_refresh:"},{"line_number":682,"context_line":"                created_rp \u003d self._create_resource_provider("},{"line_number":683,"context_line":"                    context, uuid, name or uuid,"},{"line_number":684,"context_line":"                    parent_provider_uuid\u003dparent_provider_uuid)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_e5e1f2cf","line":681,"updated":"2018-11-22 10:49:30.000000000","message":"and of course if either of this doesn\u0027t happen, then means rp is not existing so we create it,","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b3d63a10c14890a6d6def4d2901a7c9caa76367","unresolved":false,"context_lines":[{"line_number":695,"context_line":"                # But do mark it as having just been \"refreshed\"."},{"line_number":696,"context_line":"                self._association_refresh_time[uuid] \u003d time.time()"},{"line_number":697,"context_line":""},{"line_number":698,"context_line":"            self._provider_tree.populate_from_iterable("},{"line_number":699,"context_line":"                rps_to_refresh or [created_rp])"},{"line_number":700,"context_line":""},{"line_number":701,"context_line":"            uuids_to_refresh \u003d [rp[\u0027uuid\u0027] for rp in rps_to_refresh]"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_252a8ae5","line":698,"range":{"start_line":698,"start_character":32,"end_line":698,"end_character":54},"updated":"2018-11-22 10:49:30.000000000","message":"adds these to the tree and populate it.","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"6b3d63a10c14890a6d6def4d2901a7c9caa76367","unresolved":false,"context_lines":[{"line_number":702,"context_line":""},{"line_number":703,"context_line":"        # At this point, the whole tree exists in the local cache."},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        for uuid_to_refresh in uuids_to_refresh:"},{"line_number":706,"context_line":"            # NOTE(efried): _refresh_associations doesn\u0027t refresh inventory"},{"line_number":707,"context_line":"            # (yet) - see that method\u0027s docstring for the why."},{"line_number":708,"context_line":"            self._refresh_and_get_inventory(context, uuid_to_refresh)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_45f3066f","line":705,"updated":"2018-11-22 10:49:30.000000000","message":"okay so here is the second optimization: since now our uuids_to_refresh would mostly not consist of anything during normal periodic cycles as the tree would be existing in cache unless its startup of sighup and the cache would never be stale since association refresh would be disabled (in our case), we would essentially also cut down:\n\n1) GET/resource_providers/$rp_uuid/inventories\n2) GET/resource_providers/$rp_uuid/aggregates\n3) GET/resource_providers/$rp_uuid/traits\n4) GET/resource_providers?member_of\u003din:$aggs\u0026required\u003dos_traits.MISC_SHARES_VIA_AGGREGATE\"","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f0b8146766cc2c308ca45aa0829e877ac0e27be4","unresolved":false,"context_lines":[{"line_number":702,"context_line":""},{"line_number":703,"context_line":"        # At this point, the whole tree exists in the local cache."},{"line_number":704,"context_line":""},{"line_number":705,"context_line":"        for uuid_to_refresh in uuids_to_refresh:"},{"line_number":706,"context_line":"            # NOTE(efried): _refresh_associations doesn\u0027t refresh inventory"},{"line_number":707,"context_line":"            # (yet) - see that method\u0027s docstring for the why."},{"line_number":708,"context_line":"            self._refresh_and_get_inventory(context, uuid_to_refresh)"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_271c9963","line":705,"in_reply_to":"3f79a3b5_45f3066f","updated":"2018-11-26 23:23:01.000000000","message":"Yes","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd993d682d089d90befd67ac53f22cb144544a4","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        #   alerted when \u003cthing we care about happens in placement\u003e."},{"line_number":670,"context_line":"        # - \"Cascading generations\" - i.e. a change to a leaf node percolates"},{"line_number":671,"context_line":"        #   generation bump up the tree so that we bounce 409 the next time we"},{"line_number":672,"context_line":"        #   try to update anything and have to refresh."},{"line_number":673,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":674,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":675,"context_line":"            uuids_to_refresh \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_cef26ee4","line":672,"updated":"2018-11-28 23:00:04.000000000","message":"fourth option: do nothing and don\u0027t care.","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"aded707a5cb103357e28cb72c84123eb9640bc49","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        #   alerted when \u003cthing we care about happens in placement\u003e."},{"line_number":670,"context_line":"        # - \"Cascading generations\" - i.e. a change to a leaf node percolates"},{"line_number":671,"context_line":"        #   generation bump up the tree so that we bounce 409 the next time we"},{"line_number":672,"context_line":"        #   try to update anything and have to refresh."},{"line_number":673,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":674,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":675,"context_line":"            uuids_to_refresh \u003d ["}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_6e76ba0d","line":672,"in_reply_to":"3f79a3b5_cef26ee4","updated":"2018-11-28 23:15:38.000000000","message":"which is what we\u0027re doing now","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd993d682d089d90befd67ac53f22cb144544a4","unresolved":false,"context_lines":[{"line_number":672,"context_line":"        #   try to update anything and have to refresh."},{"line_number":673,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":674,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":675,"context_line":"            uuids_to_refresh \u003d ["},{"line_number":676,"context_line":"                u for u in self._provider_tree.get_provider_uuids(uuid)"},{"line_number":677,"context_line":"                if self._associations_stale(u)]"},{"line_number":678,"context_line":"        else:"},{"line_number":679,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":680,"context_line":"            created_rp \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_eedbaa48","line":677,"range":{"start_line":675,"start_character":12,"end_line":677,"end_character":47},"updated":"2018-11-28 23:00:04.000000000","message":"hold up, I\u0027m not sure I\u0027m following this ...\n\nif the UUID exists but the associations for that UUID are *NOT* stale, then we look for all providers that are *children* of that UUID and check to see if any of *them* are stale.\n\nIs that correct?","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"aded707a5cb103357e28cb72c84123eb9640bc49","unresolved":false,"context_lines":[{"line_number":672,"context_line":"        #   try to update anything and have to refresh."},{"line_number":673,"context_line":"        if (self._provider_tree.exists(uuid)"},{"line_number":674,"context_line":"                and not self._associations_stale(uuid)):"},{"line_number":675,"context_line":"            uuids_to_refresh \u003d ["},{"line_number":676,"context_line":"                u for u in self._provider_tree.get_provider_uuids(uuid)"},{"line_number":677,"context_line":"                if self._associations_stale(u)]"},{"line_number":678,"context_line":"        else:"},{"line_number":679,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":680,"context_line":"            created_rp \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_8e6c7634","line":677,"range":{"start_line":675,"start_character":12,"end_line":677,"end_character":47},"in_reply_to":"3f79a3b5_eedbaa48","updated":"2018-11-28 23:15:38.000000000","message":"Yes, see commentary L660-2, lmk if I can make this clearer somehow.","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd993d682d089d90befd67ac53f22cb144544a4","unresolved":false,"context_lines":[{"line_number":1594,"context_line":"                # signals the resource tracker to redrive the update right away"},{"line_number":1595,"context_line":"                # rather than waiting until the next periodic."},{"line_number":1596,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1597,"context_line":"                    self.clear_provider_cache()"},{"line_number":1598,"context_line":"            except helper_exceptions:"},{"line_number":1599,"context_line":"                # Invalidate the cache. It gets rebuilt on the next pass."},{"line_number":1600,"context_line":"                self.clear_provider_cache()"}],"source_content_type":"text/x-python","patch_set":9,"id":"3f79a3b5_8e947643","line":1597,"updated":"2018-11-28 23:00:04.000000000","message":"again, I think this MAY be a bridge too far for Ironic nova-computes...","commit_id":"fd7ca473daef93548bc11bf2bd8ae74a6bd7e817"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"450befc03d0f891cb7a1a30432d1b0b4bfd2cfb8","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        self._provider_tree \u003d provider_tree.ProviderTree()"},{"line_number":281,"context_line":"        self._association_refresh_time \u003d {}"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def _clear_provider_cache_for_tree(self, rp_uuid):"},{"line_number":284,"context_line":"        \"\"\"Clear the provider cache for only the tree containing rp_uuid."},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        This exists for situations where we encounter an error updating"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_bb263921","line":283,"updated":"2018-11-29 23:04:26.000000000","message":"UT for this","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"543638850513ae2e4483fad61a96e78bc9c10eb4","unresolved":false,"context_lines":[{"line_number":280,"context_line":"        self._provider_tree \u003d provider_tree.ProviderTree()"},{"line_number":281,"context_line":"        self._association_refresh_time \u003d {}"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"    def _clear_provider_cache_for_tree(self, rp_uuid):"},{"line_number":284,"context_line":"        \"\"\"Clear the provider cache for only the tree containing rp_uuid."},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        This exists for situations where we encounter an error updating"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_dffc3076","line":283,"in_reply_to":"3f79a3b5_bb263921","updated":"2018-11-30 17:23:16.000000000","message":"Meh, happy with how the func test covers it.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"450befc03d0f891cb7a1a30432d1b0b4bfd2cfb8","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            return"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":308,"context_line":"        # is the root."},{"line_number":309,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":310,"context_line":"        for uuid in uuids:"},{"line_number":311,"context_line":"            self._association_refresh_time.pop(uuid, None)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_9bd33d09","line":308,"updated":"2018-11-29 23:04:26.000000000","message":"...and ProviderTree.remove() removes the specified provider and its descendants.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"543638850513ae2e4483fad61a96e78bc9c10eb4","unresolved":false,"context_lines":[{"line_number":305,"context_line":"            return"},{"line_number":306,"context_line":""},{"line_number":307,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":308,"context_line":"        # is the root."},{"line_number":309,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":310,"context_line":"        for uuid in uuids:"},{"line_number":311,"context_line":"            self._association_refresh_time.pop(uuid, None)"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_3ff684d7","line":308,"in_reply_to":"3f79a3b5_9bd33d09","updated":"2018-11-30 17:23:16.000000000","message":"Done","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"450befc03d0f891cb7a1a30432d1b0b4bfd2cfb8","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"                # redrive the update right away rather than waiting until the"},{"line_number":1626,"context_line":"                # next periodic."},{"line_number":1627,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1628,"context_line":"                    self._clear_provider_cache_for_tree(rp_uuid)"},{"line_number":1629,"context_line":"            except helper_exceptions:"},{"line_number":1630,"context_line":"                # Invalidate the relevant part of the cache. It gets rebuilt on"},{"line_number":1631,"context_line":"                # the next pass."},{"line_number":1632,"context_line":"                self._clear_provider_cache_for_tree(rp_uuid)"},{"line_number":1633,"context_line":"                raise exception.ResourceProviderSyncFailed()"},{"line_number":1634,"context_line":""},{"line_number":1635,"context_line":"        # Helper methods herein will be updating the local cache (this is"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_3b32295d","line":1632,"range":{"start_line":1628,"start_character":0,"end_line":1632,"end_character":60},"updated":"2018-11-29 23:04:26.000000000","message":"Fix func test failures for this. If there aren\u0027t any, make some. Otherwise, probably don\u0027t need anything additional.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"543638850513ae2e4483fad61a96e78bc9c10eb4","unresolved":false,"context_lines":[{"line_number":1625,"context_line":"                # redrive the update right away rather than waiting until the"},{"line_number":1626,"context_line":"                # next periodic."},{"line_number":1627,"context_line":"                with excutils.save_and_reraise_exception():"},{"line_number":1628,"context_line":"                    self._clear_provider_cache_for_tree(rp_uuid)"},{"line_number":1629,"context_line":"            except helper_exceptions:"},{"line_number":1630,"context_line":"                # Invalidate the relevant part of the cache. It gets rebuilt on"},{"line_number":1631,"context_line":"                # the next pass."},{"line_number":1632,"context_line":"                self._clear_provider_cache_for_tree(rp_uuid)"},{"line_number":1633,"context_line":"                raise exception.ResourceProviderSyncFailed()"},{"line_number":1634,"context_line":""},{"line_number":1635,"context_line":"        # Helper methods herein will be updating the local cache (this is"}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_3fe764da","line":1632,"range":{"start_line":1628,"start_character":0,"end_line":1632,"end_character":60},"in_reply_to":"3f79a3b5_3b32295d","updated":"2018-11-30 17:23:16.000000000","message":"The func test was only checking that the failing provider was removed from the cache - which of course it was - so it still passed. Fixed it to validate the whole cache.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"450befc03d0f891cb7a1a30432d1b0b4bfd2cfb8","unresolved":false,"context_lines":[{"line_number":1705,"context_line":"        # given to us in top-down order per ProviderTree.get_provider_uuids().)"},{"line_number":1706,"context_line":"        for uuid in reversed(new_uuids):"},{"line_number":1707,"context_line":"            pd \u003d new_tree.data(uuid)"},{"line_number":1708,"context_line":"            with catch_all(pd.uuid):"},{"line_number":1709,"context_line":"                self._set_inventory_for_provider("},{"line_number":1710,"context_line":"                    context, pd.uuid, pd.inventory)"},{"line_number":1711,"context_line":"                self.set_aggregates_for_provider("}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_5bf60595","line":1708,"range":{"start_line":1708,"start_character":27,"end_line":1708,"end_character":34},"updated":"2018-11-29 23:04:26.000000000","message":"This could probably be just `uuid` here and below. It\u0027s probably pd.* below for visual consistency though.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"543638850513ae2e4483fad61a96e78bc9c10eb4","unresolved":false,"context_lines":[{"line_number":1705,"context_line":"        # given to us in top-down order per ProviderTree.get_provider_uuids().)"},{"line_number":1706,"context_line":"        for uuid in reversed(new_uuids):"},{"line_number":1707,"context_line":"            pd \u003d new_tree.data(uuid)"},{"line_number":1708,"context_line":"            with catch_all(pd.uuid):"},{"line_number":1709,"context_line":"                self._set_inventory_for_provider("},{"line_number":1710,"context_line":"                    context, pd.uuid, pd.inventory)"},{"line_number":1711,"context_line":"                self.set_aggregates_for_provider("}],"source_content_type":"text/x-python","patch_set":11,"id":"3f79a3b5_bf305456","line":1708,"range":{"start_line":1708,"start_character":27,"end_line":1708,"end_character":34},"in_reply_to":"3f79a3b5_5bf60595","updated":"2018-11-30 17:23:16.000000000","message":"Leaving this alone.","commit_id":"9106fc2c9a57968f67b8d0a4f1377faa63a68be9"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd04d33d8c55df3b791adeab6a2448e440743972","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                        including descendants, ancestors up to the root,"},{"line_number":301,"context_line":"                        siblings/cousins and *their* ancestors/descendants."},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            uuids \u003d self._provider_tree.get_provider_uuids("},{"line_number":305,"context_line":"                rp_uuid, whole_tree\u003dTrue)"},{"line_number":306,"context_line":"        except ValueError:"},{"line_number":307,"context_line":"            # If the provider isn\u0027t in the tree, it should also not be in the"},{"line_number":308,"context_line":"            # timer dict, so nothing to clear."},{"line_number":309,"context_line":"            return"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":312,"context_line":"        # is the root; and .remove() is recursive."},{"line_number":313,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":314,"context_line":"        for uuid in uuids:"},{"line_number":315,"context_line":"            self._association_refresh_time.pop(uuid, None)"},{"line_number":316,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_784ae355","line":313,"range":{"start_line":303,"start_character":0,"end_line":313,"end_character":44},"updated":"2019-01-14 13:52:23.000000000","message":"if you take my advice about having a separate function on ProviderTree that does the tree-traversal stuff here, instead of passing a whole_tree\u003dTrue argument, you might consider making that new function ProviderTree.remove_subtree() and wrap the removal code into the function. You can have ProviderTree.remove_subtree() return the list of remove UUIDs, as well, so that lines 314-315 will be possible.\n\nThat way, you wrap the related tree-fetch-and-removal code together in a single-purpose method on ProviderTree and leave ProviderTree.get_provider_uuids() method as-is.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8bb146a717bf6b67c949279a0c660840c79211b","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                        including descendants, ancestors up to the root,"},{"line_number":301,"context_line":"                        siblings/cousins and *their* ancestors/descendants."},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            uuids \u003d self._provider_tree.get_provider_uuids("},{"line_number":305,"context_line":"                rp_uuid, whole_tree\u003dTrue)"},{"line_number":306,"context_line":"        except ValueError:"},{"line_number":307,"context_line":"            # If the provider isn\u0027t in the tree, it should also not be in the"},{"line_number":308,"context_line":"            # timer dict, so nothing to clear."},{"line_number":309,"context_line":"            return"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":312,"context_line":"        # is the root; and .remove() is recursive."},{"line_number":313,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":314,"context_line":"        for uuid in uuids:"},{"line_number":315,"context_line":"            self._association_refresh_time.pop(uuid, None)"},{"line_number":316,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_a84f9390","line":313,"range":{"start_line":303,"start_character":0,"end_line":313,"end_character":44},"in_reply_to":"bfdaf3ff_784ae355","updated":"2019-01-14 22:53:44.000000000","message":".remove already does remove the subtree. The new method would have a different name but do the same thing except for what it returns? May as well modify .remove to return the UUIDs.\n\n...which I also considered. Ended up going with this approach as the lesser (or at least a coin flip) of two evils. Again, if you feel strongly, I can twiddle it to be different-but-equally-awkward. LMK.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ec7ac860f0881450514bda2f8835eb35865650db","unresolved":false,"context_lines":[{"line_number":300,"context_line":"                        including descendants, ancestors up to the root,"},{"line_number":301,"context_line":"                        siblings/cousins and *their* ancestors/descendants."},{"line_number":302,"context_line":"        \"\"\""},{"line_number":303,"context_line":"        try:"},{"line_number":304,"context_line":"            uuids \u003d self._provider_tree.get_provider_uuids("},{"line_number":305,"context_line":"                rp_uuid, whole_tree\u003dTrue)"},{"line_number":306,"context_line":"        except ValueError:"},{"line_number":307,"context_line":"            # If the provider isn\u0027t in the tree, it should also not be in the"},{"line_number":308,"context_line":"            # timer dict, so nothing to clear."},{"line_number":309,"context_line":"            return"},{"line_number":310,"context_line":""},{"line_number":311,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":312,"context_line":"        # is the root; and .remove() is recursive."},{"line_number":313,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":314,"context_line":"        for uuid in uuids:"},{"line_number":315,"context_line":"            self._association_refresh_time.pop(uuid, None)"},{"line_number":316,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_6f3afb98","line":313,"range":{"start_line":303,"start_character":0,"end_line":313,"end_character":44},"in_reply_to":"bfdaf3ff_a84f9390","updated":"2019-01-15 19:09:28.000000000","message":"Having written get_provider_uuids_in_tree, I\u0027d like to keep the deletion non-consolidated to facilitate the test cases if that\u0027s cool.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd04d33d8c55df3b791adeab6a2448e440743972","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"                            that no reshape is being performed."},{"line_number":1582,"context_line":"        :raises: ResourceProviderUpdateConflict if a generation conflict was"},{"line_number":1583,"context_line":"                 encountered - i.e. we are attempting to update placement based"},{"line_number":1584,"context_line":"                 on a stale initial view of it."},{"line_number":1585,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1586,"context_line":"                 attempting to perform the necessary API operations, except"},{"line_number":1587,"context_line":"                 reshape (see below)."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_38a1bbaa","line":1584,"range":{"start_line":1584,"start_character":28,"end_line":1584,"end_character":36},"updated":"2019-01-14 13:52:23.000000000","message":"not necessarily an \"initial\" view, but ok.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"ec7ac860f0881450514bda2f8835eb35865650db","unresolved":false,"context_lines":[{"line_number":1581,"context_line":"                            that no reshape is being performed."},{"line_number":1582,"context_line":"        :raises: ResourceProviderUpdateConflict if a generation conflict was"},{"line_number":1583,"context_line":"                 encountered - i.e. we are attempting to update placement based"},{"line_number":1584,"context_line":"                 on a stale initial view of it."},{"line_number":1585,"context_line":"        :raises: ResourceProviderSyncFailed if any errors were encountered"},{"line_number":1586,"context_line":"                 attempting to perform the necessary API operations, except"},{"line_number":1587,"context_line":"                 reshape (see below)."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_af4f8332","line":1584,"range":{"start_line":1584,"start_character":28,"end_line":1584,"end_character":36},"in_reply_to":"bfdaf3ff_38a1bbaa","updated":"2019-01-15 19:09:28.000000000","message":"Done","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"eee6b53d9d3f656cb33b5c202c85ad71a6919b4a","unresolved":false,"context_lines":[{"line_number":307,"context_line":"            # timer dict, so nothing to clear."},{"line_number":308,"context_line":"            return"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":311,"context_line":"        # is the root; and .remove() is recursive."},{"line_number":312,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":313,"context_line":"        for uuid in uuids:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fdfeff1_12596afd","line":310,"range":{"start_line":310,"start_character":10,"end_line":310,"end_character":28},"updated":"2019-01-18 18:32:24.000000000","message":"get_provider_uuids_in_tree","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"1fba7c21247df984c4443115ab50d4822ab9ef8e","unresolved":false,"context_lines":[{"line_number":307,"context_line":"            # timer dict, so nothing to clear."},{"line_number":308,"context_line":"            return"},{"line_number":309,"context_line":""},{"line_number":310,"context_line":"        # get_provider_uuids returns UUIDs in top-down order, so the first one"},{"line_number":311,"context_line":"        # is the root; and .remove() is recursive."},{"line_number":312,"context_line":"        self._provider_tree.remove(uuids[0])"},{"line_number":313,"context_line":"        for uuid in uuids:"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fdfeff1_933385f6","line":310,"range":{"start_line":310,"start_character":10,"end_line":310,"end_character":28},"in_reply_to":"9fdfeff1_12596afd","updated":"2019-01-28 22:20:40.000000000","message":"Done","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"eee6b53d9d3f656cb33b5c202c85ad71a6919b4a","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        else:"},{"line_number":710,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":711,"context_line":"            created_rp \u003d None"},{"line_number":712,"context_line":"            rps_to_refresh \u003d self._get_providers_in_tree(context, uuid)"},{"line_number":713,"context_line":"            if not rps_to_refresh:"},{"line_number":714,"context_line":"                created_rp \u003d self._create_resource_provider("},{"line_number":715,"context_line":"                    context, uuid, name or uuid,"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fdfeff1_9201dab2","line":712,"updated":"2019-01-18 18:32:24.000000000","message":"for reviewers, *this* spot right here is the call to placement that we are reducing in this patch. This is the GET /resource_providers?in_tree\u003d\u003cUUID\u003e referred to in the commit message.\n\nWith this patch, we no longer make this call when the ProviderTree has a fresh record of the provider (which is the case in all cases except when initially creating the compute node).","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"e596206f63959058d1bf38191cbfa13f7fbd35ca","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        else:"},{"line_number":710,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":711,"context_line":"            created_rp \u003d None"},{"line_number":712,"context_line":"            rps_to_refresh \u003d self._get_providers_in_tree(context, uuid)"},{"line_number":713,"context_line":"            if not rps_to_refresh:"},{"line_number":714,"context_line":"                created_rp \u003d self._create_resource_provider("},{"line_number":715,"context_line":"                    context, uuid, name or uuid,"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fdfeff1_3e823551","line":712,"in_reply_to":"9fdfeff1_03703e72","updated":"2019-01-18 22:35:00.000000000","message":"Precisely.","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"ddd4766db5194f584c11632d3923d953df5bf312","unresolved":false,"context_lines":[{"line_number":709,"context_line":"        else:"},{"line_number":710,"context_line":"            # We either don\u0027t have it locally or it\u0027s stale. Pull or create it."},{"line_number":711,"context_line":"            created_rp \u003d None"},{"line_number":712,"context_line":"            rps_to_refresh \u003d self._get_providers_in_tree(context, uuid)"},{"line_number":713,"context_line":"            if not rps_to_refresh:"},{"line_number":714,"context_line":"                created_rp \u003d self._create_resource_provider("},{"line_number":715,"context_line":"                    context, uuid, name or uuid,"}],"source_content_type":"text/x-python","patch_set":20,"id":"9fdfeff1_03703e72","line":712,"in_reply_to":"9fdfeff1_9201dab2","updated":"2019-01-18 21:40:06.000000000","message":"So in other words, we actually get benefit from our cache now...","commit_id":"deef31729bd54f3747b7adba4132f148559c2242"}],"nova/tests/functional/libvirt/test_report_cpu_traits.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"0e34c720eb6ac48c2456c001d4ef200179e254cc","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self._create_trait(\u0027CUSTOM_TRAITS\u0027)"},{"line_number":46,"context_line":"        new_traits \u003d [\u0027CUSTOM_TRAITS\u0027, \u0027HW_CPU_X86_AVX\u0027]"},{"line_number":47,"context_line":"        self._set_provider_traits(self.host_uuid, new_traits)"},{"line_number":48,"context_line":"        # The above is an out-of-band placement operation, as if the operator"},{"line_number":49,"context_line":"        # used the CLI. So now we have to \"SIGHUP the compute process\" to clear"},{"line_number":50,"context_line":"        # the report client cache so the subsequent update picks up the change."},{"line_number":51,"context_line":"        self.compute.manager.reset()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_ebac6388","line":48,"updated":"2018-11-22 12:16:46.000000000","message":"shouldn\u0027t this go with https://review.openstack.org/#/c/615646/7 the patchset below? why is this not failing below ?","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f0b8146766cc2c308ca45aa0829e877ac0e27be4","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        self._create_trait(\u0027CUSTOM_TRAITS\u0027)"},{"line_number":46,"context_line":"        new_traits \u003d [\u0027CUSTOM_TRAITS\u0027, \u0027HW_CPU_X86_AVX\u0027]"},{"line_number":47,"context_line":"        self._set_provider_traits(self.host_uuid, new_traits)"},{"line_number":48,"context_line":"        # The above is an out-of-band placement operation, as if the operator"},{"line_number":49,"context_line":"        # used the CLI. So now we have to \"SIGHUP the compute process\" to clear"},{"line_number":50,"context_line":"        # the report client cache so the subsequent update picks up the change."},{"line_number":51,"context_line":"        self.compute.manager.reset()"}],"source_content_type":"text/x-python","patch_set":8,"id":"3f79a3b5_2751f90c","line":48,"in_reply_to":"3f79a3b5_ebac6388","updated":"2018-11-26 23:23:01.000000000","message":"The key is \"out-of-band\". Prior to this patch, we would pick up (certain) out-of-band changes every time because of the ?in_tree call we were always doing. Now that we\u0027ve gotten rid of that, the out-of-band change would not have been detected and we would have overwritten the traits completely instead of merging them.","commit_id":"801ef1adca667ecdf243464b083282418d5987e4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"cd04d33d8c55df3b791adeab6a2448e440743972","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        # The above is an out-of-band placement operation, as if the operator"},{"line_number":49,"context_line":"        # used the CLI. So now we have to \"SIGHUP the compute process\" to clear"},{"line_number":50,"context_line":"        # the report client cache so the subsequent update picks up the change."},{"line_number":51,"context_line":"        self.compute.manager.reset()"},{"line_number":52,"context_line":"        self._run_periodics()"},{"line_number":53,"context_line":"        # HW_CPU_X86_AVX is filtered out because nova-compute owns CPU traits"},{"line_number":54,"context_line":"        # and it\u0027s not in the baseline for the host."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_78ac238c","line":51,"updated":"2019-01-14 13:52:23.000000000","message":"hmm, this seems out of place in this patch. perhaps this is leftover from a different patch?","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8bb146a717bf6b67c949279a0c660840c79211b","unresolved":false,"context_lines":[{"line_number":48,"context_line":"        # The above is an out-of-band placement operation, as if the operator"},{"line_number":49,"context_line":"        # used the CLI. So now we have to \"SIGHUP the compute process\" to clear"},{"line_number":50,"context_line":"        # the report client cache so the subsequent update picks up the change."},{"line_number":51,"context_line":"        self.compute.manager.reset()"},{"line_number":52,"context_line":"        self._run_periodics()"},{"line_number":53,"context_line":"        # HW_CPU_X86_AVX is filtered out because nova-compute owns CPU traits"},{"line_number":54,"context_line":"        # and it\u0027s not in the baseline for the host."}],"source_content_type":"text/x-python","patch_set":17,"id":"bfdaf3ff_ab022d08","line":51,"in_reply_to":"bfdaf3ff_78ac238c","updated":"2019-01-14 22:53:44.000000000","message":"I don\u0027t think so. Before we were always rebuilding the cache, but now we\u0027re not, so we need to flush the cache \"manually\" whenever we \"manually\" change placement, as we\u0027ve done here.\n\nI can\u0027t test locally yet, but I think if you remove this the test will fail.","commit_id":"40a9d195de13e47eab37a07ccb9eb9518970875e"}]}