)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"205c36450d93004e2bab21e1f3b7886dc3df798c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"e8ce8a29_003128ac","updated":"2022-08-11 18:37:35.000000000","message":"Looks OK to me. +1, want gibi to look when he\u0027s back from PTO.","commit_id":"df90cbfb5be8d13d3341c15e36d7dee9b8a4a464"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"5a6b57f7e0be29d2117a04f8d8e4f3d6a4840af4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"770cdd33_2ee2f6cf","updated":"2022-08-11 19:09:09.000000000","message":"Looks good","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"6f42b1d0f6bd6ae1a6b51b21aa4098da93cb9608","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"03ed3eba_497c3cb1","updated":"2022-08-11 20:45:03.000000000","message":"recheck infra post_failure","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"540db54808812efc20514ac849a29552a8b92339","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"4a11aba1_6a0492ed","updated":"2022-08-19 00:53:09.000000000","message":"Changes look good","commit_id":"c178d9360665c219cbcc71c9f37b9e6e3055a5e5"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"55b294eeca9b16ff79a56576ed37a8b36425ea68","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"bac2bbf4_dce77ac8","updated":"2022-08-18 15:18:01.000000000","message":"Looks good","commit_id":"c178d9360665c219cbcc71c9f37b9e6e3055a5e5"}],"nova/cmd/manage.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":2209,"context_line":"                output(_(\u0027No cells to process.\u0027))"},{"line_number":2210,"context_line":"                return 4"},{"line_number":2211,"context_line":""},{"line_number":2212,"context_line":"        placement \u003d report.report_client_singleton()"},{"line_number":2213,"context_line":""},{"line_number":2214,"context_line":"        neutron \u003d None"},{"line_number":2215,"context_line":"        if heal_port_allocations:"}],"source_content_type":"text/x-python","patch_set":3,"id":"e31f573d_990d08fd","line":2212,"updated":"2022-08-18 08:58:20.000000000","message":"this is safe as only one CLI command runs in a single process","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/compute/api.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":6405,"context_line":""},{"line_number":6406,"context_line":"    @property"},{"line_number":6407,"context_line":"    def placement_client(self):"},{"line_number":6408,"context_line":"        return report.report_client_singleton()"},{"line_number":6409,"context_line":""},{"line_number":6410,"context_line":"    @wrap_exception()"},{"line_number":6411,"context_line":"    def create_aggregate(self, context, aggregate_name, availability_zone):"}],"source_content_type":"text/x-python","patch_set":3,"id":"76df9a4c_df769d19","line":6408,"updated":"2022-08-18 08:58:20.000000000","message":"now we share a single client between AggregateAPI and API within the same service process. We use the client in AggregateAPI to sync nova aggregates to placement aggregates and updates the aggregate cache in the provider_tree. We use the client in the API to delete instance allocation (local delete) or query for RP existence and traits on RPs.\n\nSo it seems we have no overlapping use. So this is probably safe.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/compute/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d1d55596881081a8b941d47bf7bdcd1abb0e6a81","unresolved":true,"context_lines":[{"line_number":622,"context_line":"        # We want the ComputeManager, ResourceTracker and ComputeVirtAPI all"},{"line_number":623,"context_line":"        # using the same instance of SchedulerReportClient which has the"},{"line_number":624,"context_line":"        # ProviderTree cache for this compute service. That means we do not"},{"line_number":625,"context_line":"        # use the global singleton here."},{"line_number":626,"context_line":"        self.reportclient \u003d report.SchedulerReportClient()"},{"line_number":627,"context_line":"        self.virtapi \u003d ComputeVirtAPI(self)"},{"line_number":628,"context_line":"        self.network_api \u003d neutron.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"bce38a59_a8a60f9d","line":625,"updated":"2022-08-16 10:23:42.000000000","message":"I\u0027m wondering if there is only one global client then using that here would also mean that ComputeManager, ResourceTracker and ComputeVirtAPI use the same client instance, the global one. So why do we do it differently here?","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":true,"context_lines":[{"line_number":622,"context_line":"        # We want the ComputeManager, ResourceTracker and ComputeVirtAPI all"},{"line_number":623,"context_line":"        # using the same instance of SchedulerReportClient which has the"},{"line_number":624,"context_line":"        # ProviderTree cache for this compute service. That means we do not"},{"line_number":625,"context_line":"        # use the global singleton here."},{"line_number":626,"context_line":"        self.reportclient \u003d report.SchedulerReportClient()"},{"line_number":627,"context_line":"        self.virtapi \u003d ComputeVirtAPI(self)"},{"line_number":628,"context_line":"        self.network_api \u003d neutron.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"cdd66230_64d24e41","line":625,"in_reply_to":"bb25fcc3_a6566d45","updated":"2022-08-18 08:58:20.000000000","message":"Do we have a ComputeManager instance per ironic node? I doubt it. Also for some reason (maybe due to multiple ironic nodes per compute manager) the provider_tree supports multiple roots.\n\nThe comment suggest that we need the same client but it does not say when and why we need different clients. If we only know that we need the same client then it would make sense to use the global one.\n\nCould you at least make the comment clearer?","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"86470c51fc10d8e6f0a0ba349eaa38b7992cc40b","unresolved":true,"context_lines":[{"line_number":622,"context_line":"        # We want the ComputeManager, ResourceTracker and ComputeVirtAPI all"},{"line_number":623,"context_line":"        # using the same instance of SchedulerReportClient which has the"},{"line_number":624,"context_line":"        # ProviderTree cache for this compute service. That means we do not"},{"line_number":625,"context_line":"        # use the global singleton here."},{"line_number":626,"context_line":"        self.reportclient \u003d report.SchedulerReportClient()"},{"line_number":627,"context_line":"        self.virtapi \u003d ComputeVirtAPI(self)"},{"line_number":628,"context_line":"        self.network_api \u003d neutron.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"bb25fcc3_a6566d45","line":625,"in_reply_to":"bce38a59_a8a60f9d","updated":"2022-08-16 18:27:30.000000000","message":"Because of the internal state you mention elsewhere? Perhaps the first part of the comment is wrong, but I assume that if we\u0027re managing multiple compute nodes (i.e. ironic) they all need separate copies for the provider tree internal state right?","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"becdaf77aaa183768fb2a3bef207dbbdca27bbfd","unresolved":true,"context_lines":[{"line_number":622,"context_line":"        # We want the ComputeManager, ResourceTracker and ComputeVirtAPI all"},{"line_number":623,"context_line":"        # using the same instance of SchedulerReportClient which has the"},{"line_number":624,"context_line":"        # ProviderTree cache for this compute service. That means we do not"},{"line_number":625,"context_line":"        # use the global singleton here."},{"line_number":626,"context_line":"        self.reportclient \u003d report.SchedulerReportClient()"},{"line_number":627,"context_line":"        self.virtapi \u003d ComputeVirtAPI(self)"},{"line_number":628,"context_line":"        self.network_api \u003d neutron.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"ef24a6bc_7155af31","line":625,"in_reply_to":"cdd66230_64d24e41","updated":"2022-08-18 13:49:32.000000000","message":"\u003e Do we have a ComputeManager instance per ironic node? I doubt it.\n\nNo, just one of course.\n\n\u003e Also for some reason (maybe due to multiple ironic nodes per compute manager) the provider_tree supports multiple roots.\n\u003e \n\u003e The comment suggest that we need the same client but it does not say when and why we need different clients. If we only know that we need the same client then it would make sense to use the global one.\n\u003e \n\u003e Could you at least make the comment clearer?\n\nI initially tried this as global, but functional tests fail, likely due to re-using compute node uuids across multiple phases of the tests. When I saw this comment, I think I interpreted it as the opposite of what it says here, implying that the RTs for each node need to be separate (which is, of course, not what it says).\n\nSo perhaps this *should* be the singleton and I\u0027ll have to figure out what the tests are doing that trip them up?","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"40600edc218cd570c9ade2d108c53cf1c1897a08","unresolved":true,"context_lines":[{"line_number":622,"context_line":"        # We want the ComputeManager, ResourceTracker and ComputeVirtAPI all"},{"line_number":623,"context_line":"        # using the same instance of SchedulerReportClient which has the"},{"line_number":624,"context_line":"        # ProviderTree cache for this compute service. That means we do not"},{"line_number":625,"context_line":"        # use the global singleton here."},{"line_number":626,"context_line":"        self.reportclient \u003d report.SchedulerReportClient()"},{"line_number":627,"context_line":"        self.virtapi \u003d ComputeVirtAPI(self)"},{"line_number":628,"context_line":"        self.network_api \u003d neutron.API()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5444a81d_1dad08e5","line":625,"in_reply_to":"ef24a6bc_7155af31","updated":"2022-08-18 14:20:22.000000000","message":"Per our discussion, yes, gibi is right and I\u0027m going to write a more verbose comment here.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/compute/resource_tracker.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":103,"context_line":"        monitor_handler \u003d monitors.MonitorHandler(self)"},{"line_number":104,"context_line":"        self.monitors \u003d monitor_handler.monitors"},{"line_number":105,"context_line":"        self.old_resources \u003d collections.defaultdict(objects.ComputeNode)"},{"line_number":106,"context_line":"        self.reportclient \u003d reportclient or report.report_client_singleton()"},{"line_number":107,"context_line":"        self.ram_allocation_ratio \u003d CONF.ram_allocation_ratio"},{"line_number":108,"context_line":"        self.cpu_allocation_ratio \u003d CONF.cpu_allocation_ratio"},{"line_number":109,"context_line":"        self.disk_allocation_ratio \u003d CONF.disk_allocation_ratio"}],"source_content_type":"text/x-python","patch_set":3,"id":"f9d2a677_a59ddd48","line":106,"updated":"2022-08-18 08:58:20.000000000","message":"We only not pass a reportclient at __init__ from the test, the ComputeManager always pass down its own client. So this is safe (depending on the ComputeManager being safe).","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/conductor/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":true,"context_lines":[{"line_number":243,"context_line":"        self.network_api \u003d neutron.API()"},{"line_number":244,"context_line":"        self.servicegroup_api \u003d servicegroup.API()"},{"line_number":245,"context_line":"        self.query_client \u003d query.SchedulerQueryClient()"},{"line_number":246,"context_line":"        self.report_client \u003d report.report_client_singleton()"},{"line_number":247,"context_line":"        self.notifier \u003d rpc.get_notifier(\u0027compute\u0027)"},{"line_number":248,"context_line":"        # Help us to record host in EventReporter"},{"line_number":249,"context_line":"        self.host \u003d CONF.host"}],"source_content_type":"text/x-python","patch_set":3,"id":"8b8500f0_8fbc158c","line":246,"updated":"2022-08-18 08:58:20.000000000","message":"I think we have one ComputeTaskManager per process. So I think it is not worse than before. The original code also shared the client between parallel Task objects (i.e. two LiveMigrationTask handling different source and dest nodes). So even if that can cause races this change does not make it worse.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/conductor/tasks/migrate.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":true,"context_lines":[{"line_number":54,"context_line":"        # and do any rollback required"},{"line_number":55,"context_line":"        raise"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    reportclient \u003d report.report_client_singleton()"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    orig_alloc \u003d reportclient.get_allocs_for_consumer("},{"line_number":60,"context_line":"        context, instance.uuid)[\u0027allocations\u0027]"}],"source_content_type":"text/x-python","patch_set":3,"id":"512ee29b_a1087089","line":57,"updated":"2022-08-18 08:58:20.000000000","message":"Now this is interesting. This could use the client from the MigrationTask or the LiveMigrationTask it is called from but it does not use that but creates a new one per task. So here in the past we had independent clients per parallel tasks instances. But now we will share a single instance across.\n\nSo we need to look at the actual calls.\n\n//later\n\nIt seems every call is independent from the shared state in the client. So this is safe. I suggest to took the opportunity to pass the client from the caller instead.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    reportclient \u003d report.report_client_singleton()"},{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    orig_alloc \u003d reportclient.get_allocs_for_consumer("},{"line_number":60,"context_line":"        context, instance.uuid)[\u0027allocations\u0027]"},{"line_number":61,"context_line":"    root_alloc \u003d orig_alloc.get(source_cn.uuid, {}).get(\u0027resources\u0027, {})"},{"line_number":62,"context_line":"    if not root_alloc:"}],"source_content_type":"text/x-python","patch_set":3,"id":"70b8e79c_cca4f5d0","line":59,"updated":"2022-08-18 08:58:20.000000000","message":"this call does not depend on the provider_tree ✔","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":71,"context_line":"    # FIXME(gibi): This method is flawed in that it does not handle allocations"},{"line_number":72,"context_line":"    # against sharing providers in any special way. This leads to duplicate"},{"line_number":73,"context_line":"    # allocations against the sharing provider during migration."},{"line_number":74,"context_line":"    success \u003d reportclient.move_allocations(context, instance.uuid,"},{"line_number":75,"context_line":"                                            migration.uuid)"},{"line_number":76,"context_line":"    if not success:"},{"line_number":77,"context_line":"        LOG.error(\u0027Unable to replace resource claim on source \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"f168a73c_5f8fc72f","line":74,"updated":"2022-08-18 08:58:20.000000000","message":"this call also does not depend on the provider_tree ✔","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":94,"context_line":"def revert_allocation_for_migration(context, source_cn, instance, migration):"},{"line_number":95,"context_line":"    \"\"\"Revert an allocation made for a migration back to the instance.\"\"\""},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"    reportclient \u003d report.report_client_singleton()"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    # FIXME(gibi): This method is flawed in that it does not handle allocations"},{"line_number":100,"context_line":"    # against sharing providers in any special way. This leads to duplicate"}],"source_content_type":"text/x-python","patch_set":3,"id":"5106c5f3_9fa40f12","line":97,"updated":"2022-08-18 08:58:20.000000000","message":"ditto","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    # FIXME(gibi): This method is flawed in that it does not handle allocations"},{"line_number":100,"context_line":"    # against sharing providers in any special way. This leads to duplicate"},{"line_number":101,"context_line":"    # allocations against the sharing provider during migration."},{"line_number":102,"context_line":"    success \u003d reportclient.move_allocations(context, migration.uuid,"},{"line_number":103,"context_line":"                                            instance.uuid)"},{"line_number":104,"context_line":"    if not success:"},{"line_number":105,"context_line":"        LOG.error(\u0027Unable to replace resource claim on source \u0027"}],"source_content_type":"text/x-python","patch_set":3,"id":"941f605b_c8ab6c94","line":102,"updated":"2022-08-18 08:58:20.000000000","message":"✔","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/limit/placement.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":43,"context_line":"def _get_placement_usages("},{"line_number":44,"context_line":"    context: \u0027nova.context.RequestContext\u0027, project_id: str"},{"line_number":45,"context_line":") -\u003e ty.Dict[str, int]:"},{"line_number":46,"context_line":"    return report.report_client_singleton().get_usages_counts_for_limits("},{"line_number":47,"context_line":"        context, project_id)"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"3225e2e6_5f3d75e6","line":46,"updated":"2022-08-18 08:58:20.000000000","message":"this call does not depend on the internal state so this is safe","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/quota.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":1348,"context_line":""},{"line_number":1349,"context_line":""},{"line_number":1350,"context_line":"def _cores_ram_count_placement(context, project_id, user_id\u003dNone):"},{"line_number":1351,"context_line":"    return report.report_client_singleton().get_usages_counts_for_quota("},{"line_number":1352,"context_line":"        context, project_id, user_id\u003duser_id)"},{"line_number":1353,"context_line":""},{"line_number":1354,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"22d9b06e_b4f4c6d4","line":1351,"updated":"2022-08-18 08:58:20.000000000","message":"this call does not depend on the internal state of the client so this is safe","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/scheduler/client/report.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d1d55596881081a8b941d47bf7bdcd1abb0e6a81","unresolved":true,"context_lines":[{"line_number":76,"context_line":"    (particularly ks_exc.*) but context-specific error messages are"},{"line_number":77,"context_line":"    logged for consistency."},{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":"    global PLACEMENTCLIENT"},{"line_number":80,"context_line":"    if PLACEMENTCLIENT is None:"},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            PLACEMENTCLIENT \u003d SchedulerReportClient()"}],"source_content_type":"text/x-python","patch_set":3,"id":"e22385ef_2cafdb15","line":79,"updated":"2022-08-16 10:23:42.000000000","message":"So this forces nova to use a single client per process while previously there were separate client instances embedded in separate objects for example in the compute API instances. The report client has internal state, the provider_tree. \n\nSo I\u0027m a bit worried that this change might introduce new race condition possibilities via the shared provider_tree instance. I don\u0027t have a specific example based on existing code so this is highly hypothetical.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":true,"context_lines":[{"line_number":76,"context_line":"    (particularly ks_exc.*) but context-specific error messages are"},{"line_number":77,"context_line":"    logged for consistency."},{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":"    global PLACEMENTCLIENT"},{"line_number":80,"context_line":"    if PLACEMENTCLIENT is None:"},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            PLACEMENTCLIENT \u003d SchedulerReportClient()"}],"source_content_type":"text/x-python","patch_set":3,"id":"be654ff0_99ba8913","line":79,"in_reply_to":"1ee3822f_be3b1c91","updated":"2022-08-18 08:58:20.000000000","message":"Now I looked through the usage of the report client and as far as I can tell all the calls within the same service is either:\n* not depend on the internal state of the client\n* use different independent parts of the internal state\n\nSo I think the globalization is safe. (In the future we might introduce new calls to the shared client that break this and introduce races).\n\nSo if you agree with my observations (see inline comments in the change sites) then I\u0027m fine globalizing the client.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"86470c51fc10d8e6f0a0ba349eaa38b7992cc40b","unresolved":true,"context_lines":[{"line_number":76,"context_line":"    (particularly ks_exc.*) but context-specific error messages are"},{"line_number":77,"context_line":"    logged for consistency."},{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":"    global PLACEMENTCLIENT"},{"line_number":80,"context_line":"    if PLACEMENTCLIENT is None:"},{"line_number":81,"context_line":"        try:"},{"line_number":82,"context_line":"            PLACEMENTCLIENT \u003d SchedulerReportClient()"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ee3822f_be3b1c91","line":79,"in_reply_to":"e22385ef_2cafdb15","updated":"2022-08-16 18:27:30.000000000","message":"TBH, I had forgotten that there was (so much) internal state in the client until I wrote this patch, which I think is somewhat of a bad decision on our part (although I know why we did it).\n\nThe goal here was to avoid the startup cost (and hit) of going to keystone all the time, both because it should be unnecessary and also to reduce the number of places where we may fail to do that because keystone is down at a random time, when we may already have an initialized client.\n\nI could remove the singleton-ness and just keep the \"ask me for a client\" behavior if you want, but I\u0027d want to know that the provider tree state is important in other places than just in compute. We do this sort of thing in all the other client implementations, AFAIK, so this is increasing consistency otherwise.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"55b294eeca9b16ff79a56576ed37a8b36425ea68","unresolved":false,"context_lines":[{"line_number":84,"context_line":"    # pieces of the actual keystone client are separate from the"},{"line_number":85,"context_line":"    # internal state, so that we can return a new object here with a"},{"line_number":86,"context_line":"    # context-specific local state object, but with the client bits"},{"line_number":87,"context_line":"    # shared."},{"line_number":88,"context_line":"    global PLACEMENTCLIENT"},{"line_number":89,"context_line":"    if PLACEMENTCLIENT is None:"},{"line_number":90,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"672707fb_218f0983","line":87,"updated":"2022-08-18 15:18:01.000000000","message":"+1","commit_id":"c178d9360665c219cbcc71c9f37b9e6e3055a5e5"}],"nova/scheduler/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        self.host_manager \u003d host_manager.HostManager()"},{"line_number":67,"context_line":"        self.servicegroup_api \u003d servicegroup.API()"},{"line_number":68,"context_line":"        self.notifier \u003d rpc.get_notifier(\u0027scheduler\u0027)"},{"line_number":69,"context_line":"        self.placement_client \u003d report.report_client_singleton()"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        super().__init__(service_name\u003d\u0027scheduler\u0027, *args, **kwargs)"},{"line_number":72,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"e875e139_c8bcdc28","line":69,"updated":"2022-08-18 08:58:20.000000000","message":"I think we have one manager instance per process so this is as safe as before","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/scheduler/request_filter.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"71a5814dfa87dbb101a0b6fe70e46efec3ae61b8","unresolved":false,"context_lines":[{"line_number":311,"context_line":""},{"line_number":312,"context_line":"    # Get the clients we need"},{"line_number":313,"context_line":"    network_api \u003d neutron.API()"},{"line_number":314,"context_line":"    report_api \u003d report.report_client_singleton()"},{"line_number":315,"context_line":""},{"line_number":316,"context_line":"    for requested_network in requested_networks:"},{"line_number":317,"context_line":"        network_id \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"e5c7171f_0514f9c1","line":314,"updated":"2022-08-18 08:58:20.000000000","message":"So in the past we had independent client instances for parallel scheduling tasks running the prefilter. Now we one one shared client per scheduler process.\n\n//later\n\nTraced the below calls to the client and none of them using the share state in the client. So I think this is safe.","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}],"nova/test.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"d1d55596881081a8b941d47bf7bdcd1abb0e6a81","unresolved":false,"context_lines":[{"line_number":293,"context_line":"        wsgi_app.init_global_data.reset()"},{"line_number":294,"context_line":""},{"line_number":295,"context_line":"        # Reset the placement client singleton"},{"line_number":296,"context_line":"        report.PLACEMENTCLIENT \u003d None"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"    def _setup_cells(self):"},{"line_number":299,"context_line":"        \"\"\"Setup a normal cellsv2 environment."}],"source_content_type":"text/x-python","patch_set":3,"id":"83b1f118_e384919e","line":296,"updated":"2022-08-16 10:23:42.000000000","message":"+100","commit_id":"ae1d08f1f8a3c2a7bb101695da4c557302526f18"}]}