)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":27,"context_line":"returned from the placement service, a total of 20 queries were made to"},{"line_number":28,"context_line":"retrieve information."},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"With this patch, when CONF.filter_scheduler.track_instance_changes is"},{"line_number":31,"context_line":"False, we now do a single query per target cell to retrieve all instance"},{"line_number":32,"context_line":"UUIDs associated with all the hosts included in the selected"},{"line_number":33,"context_line":"destinations. This essentially changes the algorithm\u0027s performance from"},{"line_number":34,"context_line":"O(N) to O(1)."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"WORK IN PROGRESS:"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"9fdfeff1_1cae19e7","line":33,"range":{"start_line":30,"start_character":0,"end_line":33,"end_character":13},"updated":"2019-02-07 15:14:13.000000000","message":"+","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"9c06c396186083ce14683483f3e5069676033506","unresolved":false,"context_lines":[{"line_number":42,"context_line":"failures. Hoping to get some initial early feedback from CERN folks"},{"line_number":43,"context_line":"whether they are able to increase CONF.max_placement_results to 100 or"},{"line_number":44,"context_line":"more with this patch and see better performance from their scheduler..."},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"Change-Id: If1c2794b8926aad4b44ab8c63fb5646c913a522c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bfdaf3ff_9b69bebf","line":45,"updated":"2019-01-14 16:21:32.000000000","message":"This should be linked to bug 1737465 somehow, either Related or Partial.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ce803e21d4e202bc740fe88d6eabfb43fa80d71","unresolved":false,"context_lines":[{"line_number":10,"context_line":"False (in order to reduce the chattiness on the message queue when there"},{"line_number":11,"context_line":"are thousands of compute nodes sending notifications about instance"},{"line_number":12,"context_line":"changes), performance of the HostManager\u0027s _get_host_states() method can"},{"line_number":13,"context_line":"be negatively impacted."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"HostManager._get_host_states() fetches information about each"},{"line_number":16,"context_line":"placement-selected compute host\u0027s instances and aggregates. This"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3fa7e38b_a64dff3b","line":13,"updated":"2019-10-14 16:26:53.000000000","message":"(to myself) sort of ironic since the help on that option says if you don\u0027t need it, disable it to improve performance:\n\nhttps://docs.openstack.org/nova/latest/configuration/config.html#filter_scheduler.track_instance_changes","commit_id":"a8adc03948e55d8bfb915c0e295baccf538f7909"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ce803e21d4e202bc740fe88d6eabfb43fa80d71","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"When CONF.filter_scheduler.track_instance_changes is False, two separate"},{"line_number":20,"context_line":"database calls were being made to get the list of instance UUIDs"},{"line_number":21,"context_line":"associated with each host in the set of selected desinations: one to get"},{"line_number":22,"context_line":"the HostMapping information for the host (in order to grab cell"},{"line_number":23,"context_line":"connection information) and another call to the cell database to"},{"line_number":24,"context_line":"retrieve the instance UUIDs for that host."},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"So, assuming 10 compute nodes were in the selected destination hosts"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3fa7e38b_46076bcb","line":23,"range":{"start_line":21,"start_character":62,"end_line":23,"end_character":23},"updated":"2019-10-14 16:26:53.000000000","message":"Note to self to remind dansmith about this as well:\n\nhttps://review.opendev.org/#/c/687139/10/nova/conductor/manager.py@1655\n\nAlso note that since Ic6b1edfad2e384eb32c6942edc522ee301123cbc I think the host mapping query is only done once per compute host until the scheduler worker is restarted since we cache the cell mapping result now. So yeah the first schedule attempt to a given host is a hit here, but after that it should be reduced to just querying the instances on the host.","commit_id":"a8adc03948e55d8bfb915c0e295baccf538f7909"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ce803e21d4e202bc740fe88d6eabfb43fa80d71","unresolved":false,"context_lines":[{"line_number":33,"context_line":"destinations. This essentially changes the algorithm\u0027s performance from"},{"line_number":34,"context_line":"O(N) to O(1)."},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"The mail loop of _get_host_states is simplified to yield instead of"},{"line_number":37,"context_line":"return. This is done because we know that we only need one HostSate"},{"line_number":38,"context_line":"per compute. It allows us to remove some extraneous key checking."},{"line_number":39,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3fa7e38b_e6d9b782","line":36,"range":{"start_line":36,"start_character":4,"end_line":36,"end_character":8},"updated":"2019-10-14 16:26:53.000000000","message":"main","commit_id":"a8adc03948e55d8bfb915c0e295baccf538f7909"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ce803e21d4e202bc740fe88d6eabfb43fa80d71","unresolved":false,"context_lines":[{"line_number":36,"context_line":"The mail loop of _get_host_states is simplified to yield instead of"},{"line_number":37,"context_line":"return. This is done because we know that we only need one HostSate"},{"line_number":38,"context_line":"per compute. It allows us to remove some extraneous key checking."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Change-Id: If1c2794b8926aad4b44ab8c63fb5646c913a522c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3fa7e38b_c6c7db95","line":39,"updated":"2019-10-14 16:26:53.000000000","message":"I would somehow link to https://techblog.web.cern.ch/techblog/post/scheduling-optimizations/ since that has details on performance with and without this change applied in a multi-cell deployment.","commit_id":"a8adc03948e55d8bfb915c0e295baccf538f7909"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"1ce803e21d4e202bc740fe88d6eabfb43fa80d71","unresolved":false,"context_lines":[{"line_number":37,"context_line":"return. This is done because we know that we only need one HostSate"},{"line_number":38,"context_line":"per compute. It allows us to remove some extraneous key checking."},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"Change-Id: If1c2794b8926aad4b44ab8c63fb5646c913a522c"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":10,"id":"3fa7e38b_4639aba1","line":40,"updated":"2019-10-14 16:26:53.000000000","message":"This should be linked to bug 1737465 somehow, either Related or Partial.","commit_id":"a8adc03948e55d8bfb915c0e295baccf538f7909"}],"nova/scheduler/host_manager.py":[{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        \"\"\""},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_416efe96","line":620,"range":{"start_line":619,"start_character":12,"end_line":620,"end_character":61},"updated":"2019-02-07 15:14:13.000000000","message":"note exactly related to this patch, but I do not understand as to why we always fetch all the services in all the cells even if we never loop through/use all of them since we are only concerned with the services running on the specific compute_nodes returned by placement if any...\n\nanyways this part of looping through all the cells to get the computes and services is what I thought to improvise in order to reduce scheduling time for us: here is the POC - https://review.openstack.org/635532","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"1327646fb8ce4ed455542051779a25d828fd795f","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        \"\"\""},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_0f4366b1","line":620,"range":{"start_line":619,"start_character":12,"end_line":620,"end_character":61},"in_reply_to":"9fdfeff1_39e4780e","updated":"2019-02-19 18:28:24.000000000","message":"thanks Jay, that might reduce the scheduling time even more cumulatively like you said.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e875d248fbf235a0f58fcfc43697ac01eca3bfb6","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        \"\"\""},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_80964d3e","line":620,"range":{"start_line":619,"start_character":12,"end_line":620,"end_character":61},"in_reply_to":"9fdfeff1_416efe96","updated":"2019-02-19 15:48:26.000000000","message":"This is only executed once on startup and is not used in the primary scheduling loop. The _get_host_states() method is what is executed on each primary scheduling loop, which is why this patch focuses on reducing the number of database calls in that method.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"19b4f97ec03f05fcb3aadff0145ab940b852a877","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        \"\"\""},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_7c02c571","line":620,"range":{"start_line":619,"start_character":12,"end_line":620,"end_character":61},"in_reply_to":"9fdfeff1_416efe96","updated":"2019-02-07 15:25:49.000000000","message":"s/note/not","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"4dda072fbab046a4ea58640cc4d3a9e83137e250","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        \"\"\""},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_39e4780e","line":620,"range":{"start_line":619,"start_character":12,"end_line":620,"end_character":61},"in_reply_to":"9fdfeff1_631fb370","updated":"2019-02-19 17:05:40.000000000","message":"Yes, absolutely correct. I should also make a patch that changes the objects.ServiceList.get_by_binary() to objects.ServiceList.get_by_binary_and_hostnames() and call that *after* this one: https://github.com/openstack/nova/blob/d2a87a44468f91d5a468e1262e7e2fd8c7add35c/nova/scheduler/host_manager.py#L624-L625\n\nI\u0027ll do a followup patch to this so you can run the benchmarks to see the difference in performance between the patches (I suspect it will be a cumulative improvement).\n\n*This* patch is only fixing the inner-most loop in the  _get_instance_info() call to not do a DB query for each *instance* on the selected hosts.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"9dde88221f1062f4bae0edbf1c6f25608efa4570","unresolved":false,"context_lines":[{"line_number":616,"context_line":"        \"\"\""},{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_631fb370","line":620,"range":{"start_line":619,"start_character":12,"end_line":620,"end_character":61},"in_reply_to":"9fdfeff1_80964d3e","updated":"2019-02-19 16:16:08.000000000","message":"err, isn\u0027t it being called on every scheduling loop from here: https://github.com/openstack/nova/blob/d2a87a44468f91d5a468e1262e7e2fd8c7add35c/nova/scheduler/host_manager.py#L691 ?","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":704,"context_line":"    @staticmethod"},{"line_number":705,"context_line":"    def _filter_enabled_cell_computes(cell_computes, services):"},{"line_number":706,"context_line":"        \"\"\"Returns a dict, keyed by cell UUID, of the ComputeNode objects that"},{"line_number":707,"context_line":"        are in that cell and are managed by an enabled nova-compute service."},{"line_number":708,"context_line":""},{"line_number":709,"context_line":"        :param cell_computes: dict, keyed by cell UUID, of a list of"},{"line_number":710,"context_line":"                              ComputeNode objects in that cell"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_d6aab217","line":707,"range":{"start_line":707,"start_character":47,"end_line":707,"end_character":75},"updated":"2019-02-07 15:14:13.000000000","message":"umm, sorry but what does this mean ? I mean I get what it means but I am  confused about how you are determining if the service is enabled/disabled compute service when all you are checking is *if* a compute service is running on the compute_node for all the compute_nodes/resource_providers returned by placement. If that\u0027s what you meant, then maybe using the enabled/disabled terminology is not so good since its confusing with the actual service enabling/disabling.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":725,"context_line":""},{"line_number":726,"context_line":"                enabled_computes.append(compute)"},{"line_number":727,"context_line":"            if not enabled_computes:"},{"line_number":728,"context_line":"                LOG.warning(\"After removing disabled compute services, \""},{"line_number":729,"context_line":"                            \"cell %(cell_uuid)s ended up with no enabled \""},{"line_number":730,"context_line":"                            \"compute hosts. Removing this cell from \""},{"line_number":731,"context_line":"                            \"scheduling consideration.\","}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_76166658","line":728,"range":{"start_line":728,"start_character":44,"end_line":728,"end_character":69},"updated":"2019-02-07 15:14:13.000000000","message":"same","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":750,"context_line":"        host_state_map \u003d {}"},{"line_number":751,"context_line":"        seen_nodes \u003d set()"},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"        # First filter out any compute nodes managed by disabled services"},{"line_number":754,"context_line":"        enabled_cell_computes \u003d self._filter_enabled_cell_computes("},{"line_number":755,"context_line":"            cell_computes, services)"},{"line_number":756,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_766f46fc","line":753,"range":{"start_line":753,"start_character":56,"end_line":753,"end_character":73},"updated":"2019-02-07 15:14:13.000000000","message":"see my comment above on why this is confusing because you are not actually checking for the \"disabled/enabled\" factor on the services right? That would be done by the Compute Filter later on.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":761,"context_line":"            # to get instance information, we grab all that information for all"},{"line_number":762,"context_line":"            # relevant compute nodes in a single pass and then update each"},{"line_number":763,"context_line":"            # HostState object with that collected information. This allows us"},{"line_number":764,"context_line":"            # to avoid doing repeated calls to HostMapping.get_by_host() and"},{"line_number":765,"context_line":"            # InstanceList.get_uuids_by_host()."},{"line_number":766,"context_line":"            instance_infos \u003d self._get_instance_infos_for_hosts("},{"line_number":767,"context_line":"                context, enabled_cell_computes)"},{"line_number":768,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_81594636","line":765,"range":{"start_line":764,"start_character":46,"end_line":765,"end_character":47},"updated":"2019-02-07 15:14:13.000000000","message":"note to self: this was called from the _get_instance_info() for each compute.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":817,"context_line":"        # majority of cases, we\u0027re not going to be asking for \u003e1 cell here, so"},{"line_number":818,"context_line":"        # for now, just go the simple route."},{"line_number":819,"context_line":"        cell_mappings \u003d {}"},{"line_number":820,"context_line":"        for cell_uuid, computes in cell_computes.items():"},{"line_number":821,"context_line":"            try:"},{"line_number":822,"context_line":"                cell_mapping \u003d objects.CellMapping.get_by_uuid("},{"line_number":823,"context_line":"                    context, cell_uuid)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_21217a4e","line":820,"range":{"start_line":820,"start_character":12,"end_line":820,"end_character":54},"updated":"2019-02-07 15:14:13.000000000","message":"cool so we just reuse the cell info instead of querying for the host_mapping to get the cell_mapping, that is improvement1.\n\nhowever this particular loop is being done three times (having dejavu as I follow it in all the three functions), maybe the stuff in _filter_enabled_cell_computes need not be done as I don\u0027t exactly see that improving anything? I mean of course looping through these computes if max_limit is set should be negligible. Only if deployments have a large number of computes it might add up.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":841,"context_line":"                # the hypervisor_hostname, not the Instance.host value, which"},{"line_number":842,"context_line":"                # refers to the Service.host. Yeah, it\u0027s silly and confusing..."},{"line_number":843,"context_line":"                host_names \u003d [cn.host for cn in computes]"},{"line_number":844,"context_line":"                uuids_by_host \u003d objects.InstanceList.get_uuids_by_hosts("},{"line_number":845,"context_line":"                    cctxt, host_names)"},{"line_number":846,"context_line":"                for host_name, uuids in uuids_by_host.items():"},{"line_number":847,"context_line":"                    instance_infos[host_name] \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_e1eff2e6","line":844,"range":{"start_line":844,"start_character":32,"end_line":844,"end_character":71},"updated":"2019-02-07 15:14:13.000000000","message":"note to self: this call is also done with the fetching of the compute_info call per cell, improvement2.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a368a0934392cc20f6d533468e357933cd8431db","unresolved":false,"context_lines":[{"line_number":883,"context_line":"        relying on the version in _instance_info."},{"line_number":884,"context_line":"        \"\"\""},{"line_number":885,"context_line":"        host_name \u003d compute.host"},{"line_number":886,"context_line":"        host_info \u003d instance_infos.get(host_name)"},{"line_number":887,"context_line":"        if host_info and host_info.get(\"updated\"):"},{"line_number":888,"context_line":"            inst_dict \u003d host_info[\"instances\"]"},{"line_number":889,"context_line":"        else:"},{"line_number":890,"context_line":"            # Host is running old version, or updates aren\u0027t flowing."},{"line_number":891,"context_line":"            inst_dict \u003d self._get_instances_by_host(context, host_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"9fdfeff1_e15372e1","line":888,"range":{"start_line":886,"start_character":8,"end_line":888,"end_character":46},"updated":"2019-02-07 15:14:13.000000000","message":"ok so basically if we run through the new code flow (with self.track_instance_changes\u003dFalse), it is guaranteed that we will always have a populated instance_infos and will always go into the if part and not the else.","commit_id":"e1e9d7a4ce0437768f66cdf0509f7a80012472e4"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a48db561a88e7eaca06f7394b57907259428b62d","unresolved":false,"context_lines":[{"line_number":617,"context_line":""},{"line_number":618,"context_line":"        def targeted_operation(cctxt):"},{"line_number":619,"context_line":"            services \u003d objects.ServiceList.get_by_binary("},{"line_number":620,"context_line":"                cctxt, \u0027nova-compute\u0027, include_disabled\u003dTrue)"},{"line_number":621,"context_line":"            if compute_uuids is None:"},{"line_number":622,"context_line":"                return services, objects.ComputeNodeList.get_all(cctxt)"},{"line_number":623,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_561a287d","line":620,"range":{"start_line":620,"start_character":39,"end_line":620,"end_character":60},"updated":"2019-06-06 08:22:51.000000000","message":"now I remember! so this is the confusion I had. We return disabled services also from this function and what _filter_enabled_cell_computes does it not exactly filtering \"enabled\" computes (which is what the compute filter does).","commit_id":"e6b6ece5046fe886f6118026f29c94180b98dc68"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"a48db561a88e7eaca06f7394b57907259428b62d","unresolved":false,"context_lines":[{"line_number":787,"context_line":""},{"line_number":788,"context_line":"                enabled_computes.append(compute)"},{"line_number":789,"context_line":"            if not enabled_computes:"},{"line_number":790,"context_line":"                LOG.warning(\"After removing disabled compute services, \""},{"line_number":791,"context_line":"                            \"cell %(cell_uuid)s ended up with no enabled \""},{"line_number":792,"context_line":"                            \"compute hosts. Removing this cell from \""},{"line_number":793,"context_line":"                            \"scheduling consideration.\","}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_d60db83e","line":790,"range":{"start_line":790,"start_character":20,"end_line":790,"end_character":27},"updated":"2019-06-06 08:22:51.000000000","message":"change this.","commit_id":"e6b6ece5046fe886f6118026f29c94180b98dc68"},{"author":{"_account_id":26936,"name":"Surya Seetharaman","email":"suryaseetharaman.9@gmail.com","username":"tssurya"},"change_message_id":"537ead9dae0d58ffbeace0d99d3a07a6f36a005d","unresolved":false,"context_lines":[{"line_number":889,"context_line":"                            \u0027instance infos for this cell.\u0027, cell_uuid)"},{"line_number":890,"context_line":"                continue"},{"line_number":891,"context_line":""},{"line_number":892,"context_line":"        for cell_uuid, cell_mappping in cell_mappings.items():"},{"line_number":893,"context_line":"            computes \u003d cell_computes[cell_uuid]"},{"line_number":894,"context_line":"            with context_module.target_cell(context, cell_mapping) as cctxt:"},{"line_number":895,"context_line":"                # NOTE(jaypipes): Note that we look up instance UUIDs"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_7414ffa0","line":892,"range":{"start_line":892,"start_character":30,"end_line":892,"end_character":33},"updated":"2019-06-06 13:49:23.000000000","message":":/","commit_id":"e6b6ece5046fe886f6118026f29c94180b98dc68"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"14550c0999b39cf38ec9c786a4ac268f9b138396","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                    }"},{"line_number":919,"context_line":"        return instance_infos"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"    def _get_instances_by_host(self, context, host_name):"},{"line_number":922,"context_line":"        try:"},{"line_number":923,"context_line":"            hm \u003d objects.HostMapping.get_by_host(context, host_name)"},{"line_number":924,"context_line":"        except exception.HostMappingNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_7b09622a","line":921,"updated":"2019-06-05 15:17:31.000000000","message":"One simple thing I thought about doing was simply cache the host to the cell_uuid in a dict so we only do this lookup once, and then we could change the cells/enabled_cells lists to a dict, keyed by cell_uuid, and then we only need to lookup the host mapping once to figure out which cell it\u0027s in, the rest of the time we just get the cell mapping by host from the cache. I might post that separately since it\u0027s a much simpler change than the bigger DB query refactor here (though I agree this change is a better long-term solution). I\u0027m just not sure I\u0027d want to backport this patch given the size.","commit_id":"e6b6ece5046fe886f6118026f29c94180b98dc68"},{"author":{"_account_id":6873,"name":"Matt Riedemann","email":"mriedem.os@gmail.com","username":"mriedem"},"change_message_id":"20bf4fbc596ae99866b0211844b08e1f40972059","unresolved":false,"context_lines":[{"line_number":918,"context_line":"                    }"},{"line_number":919,"context_line":"        return instance_infos"},{"line_number":920,"context_line":""},{"line_number":921,"context_line":"    def _get_instances_by_host(self, context, host_name):"},{"line_number":922,"context_line":"        try:"},{"line_number":923,"context_line":"            hm \u003d objects.HostMapping.get_by_host(context, host_name)"},{"line_number":924,"context_line":"        except exception.HostMappingNotFound:"}],"source_content_type":"text/x-python","patch_set":2,"id":"9fb8cfa7_ececba2a","line":921,"in_reply_to":"9fb8cfa7_7b09622a","updated":"2019-06-05 17:37:28.000000000","message":"https://review.opendev.org/#/c/663388/","commit_id":"e6b6ece5046fe886f6118026f29c94180b98dc68"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8afe8b99b032e2d51c8df7e0c0631dff1a81d96","unresolved":false,"context_lines":[{"line_number":803,"context_line":"                    continue"},{"line_number":804,"context_line":""},{"line_number":805,"context_line":"                enabled_computes.append(compute)"},{"line_number":806,"context_line":"            if not enabled_computes:"},{"line_number":807,"context_line":"                LOG.warning(\"After removing disabled compute services, \""},{"line_number":808,"context_line":"                            \"cell %(cell_uuid)s ended up with no enabled \""},{"line_number":809,"context_line":"                            \"compute hosts. Removing this cell from \""},{"line_number":810,"context_line":"                            \"scheduling consideration.\","},{"line_number":811,"context_line":"                            {\u0027cell_uuid\u0027: cell_uuid})"},{"line_number":812,"context_line":"                continue"},{"line_number":813,"context_line":"            enabled_cell_computes[cell_uuid] \u003d enabled_computes"},{"line_number":814,"context_line":"        return enabled_cell_computes"},{"line_number":815,"context_line":""},{"line_number":816,"context_line":"    def _get_host_states(self, context, cell_computes, services):"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_3e7e2be4","line":813,"range":{"start_line":806,"start_character":0,"end_line":813,"end_character":63},"updated":"2019-08-15 17:07:40.000000000","message":"unit test this path","commit_id":"64131d11f989444c70cddeacca10ceca2660b37d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8afe8b99b032e2d51c8df7e0c0631dff1a81d96","unresolved":false,"context_lines":[{"line_number":858,"context_line":"                                                     compute\u003dcompute)"},{"line_number":859,"context_line":"                    host_state_map[state_key] \u003d host_state"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"                seen_nodes.add(state_key)"},{"line_number":862,"context_line":"                host_state.update(compute,"},{"line_number":863,"context_line":"                                  dict(service),"},{"line_number":864,"context_line":"                                  self._get_aggregates_info(host),"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_d3e590a5","line":861,"range":{"start_line":861,"start_character":16,"end_line":861,"end_character":41},"updated":"2019-08-15 17:07:40.000000000","message":"Any reason for reversing the order of this and the next line? I can\u0027t see how it would affect anything either way.\n\n...actually, I can\u0027t see a reason for seen_nodes to exist at all. Maybe I\u0027m missing something, but it appears to be the same as host_state_map.keys(). So it could go away and L868 could just be\n\n return host_state_map.values()\n\n(I realize that\u0027s not technically a generator in py2 - if that really matters, you could\n\n return (x for x in host_state_map.values())\n\n)","commit_id":"64131d11f989444c70cddeacca10ceca2660b37d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8afe8b99b032e2d51c8df7e0c0631dff1a81d96","unresolved":false,"context_lines":[{"line_number":859,"context_line":"                    host_state_map[state_key] \u003d host_state"},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"                seen_nodes.add(state_key)"},{"line_number":862,"context_line":"                host_state.update(compute,"},{"line_number":863,"context_line":"                                  dict(service),"},{"line_number":864,"context_line":"                                  self._get_aggregates_info(host),"},{"line_number":865,"context_line":"                                  self._get_instance_info("}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_beabfbd3","line":862,"range":{"start_line":862,"start_character":16,"end_line":862,"end_character":34},"updated":"2019-08-15 17:07:40.000000000","message":"I\u0027m a bit confused why this can\u0027t happen inside the `if`, after L858. The comment that you removed, when I start reading it, gives me hope of an explanation - but this looks like a pretty tight loop, so I\u0027m not understanding under what circumstances you would want to re-update an already-seen host_state.\n\n...and if you did that, you could conceivably just\n\n yield host_state\n\nfrom there and do away with the caches and the return.\n\nBut I guess that\u0027s all probably unrelated to the intent of this patch, which is to compute instance_infos just once.","commit_id":"64131d11f989444c70cddeacca10ceca2660b37d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8afe8b99b032e2d51c8df7e0c0631dff1a81d96","unresolved":false,"context_lines":[{"line_number":902,"context_line":"        self.host_to_cell_uuid[host_name] \u003d cell_mapping.uuid"},{"line_number":903,"context_line":"        return cell_mapping"},{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    def _get_instance_infos_for_hosts(self, context, cell_computes):"},{"line_number":906,"context_line":"        \"\"\"Returns a dict, keyed by ComputeNode.host, of a dict containing"},{"line_number":907,"context_line":"        two keys: \"updated\" which is always True, and \"instances\" which is a"},{"line_number":908,"context_line":"        dict, keyed by instance UUID, of Instance objects associated with that"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_de57d759","line":905,"updated":"2019-08-15 17:07:40.000000000","message":"Yeah, I would think we would want UT for this method.","commit_id":"64131d11f989444c70cddeacca10ceca2660b37d"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"f8afe8b99b032e2d51c8df7e0c0631dff1a81d96","unresolved":false,"context_lines":[{"line_number":937,"context_line":"                            \u0027instance infos for this cell.\u0027, cell_uuid)"},{"line_number":938,"context_line":"                continue"},{"line_number":939,"context_line":""},{"line_number":940,"context_line":"        for cell_uuid, cell_mapping in cell_mappings.items():"},{"line_number":941,"context_line":"            computes \u003d cell_computes[cell_uuid]"},{"line_number":942,"context_line":"            with context_module.target_cell(context, cell_mapping) as cctxt:"},{"line_number":943,"context_line":"                # NOTE(jaypipes): Note that we look up instance UUIDs"}],"source_content_type":"text/x-python","patch_set":5,"id":"7faddb67_deb11703","line":940,"range":{"start_line":940,"start_character":8,"end_line":940,"end_character":61},"updated":"2019-08-15 17:07:40.000000000","message":"Any reason not to consolidate this loop into the previous (i.e. delete this line)? Then you can get rid of the cell_mappings dict.","commit_id":"64131d11f989444c70cddeacca10ceca2660b37d"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"ffaad04125f69886028454eb575be7acf8c2ab19","unresolved":false,"context_lines":[{"line_number":836,"context_line":"                context, enabled_cell_computes)"},{"line_number":837,"context_line":""},{"line_number":838,"context_line":"        for cell_uuid, computes in enabled_cell_computes.items():"},{"line_number":839,"context_line":"            for compute in computes:"},{"line_number":840,"context_line":"                service \u003d services.get(compute.host)"},{"line_number":841,"context_line":"                host \u003d compute.host"},{"line_number":842,"context_line":"                node \u003d compute.hypervisor_hostname"}],"source_content_type":"text/x-python","patch_set":6,"id":"7faddb67_6e45c36e","line":839,"updated":"2019-09-02 13:31:24.000000000","message":"With these changes we are making the assumption that there are no duplicates in computes, of if there are it doesn\u0027t matter, and we only need one host state for each compute, thus it is safe to yield each time through the loop.\n\nBased on the value of the previous \u0027state_key\u0027 this seems a legit assumption but parsing all this is... ugh","commit_id":"7a71d49711488e7a8761b8be9e443afd190318ef"}]}