[watcher] Compute CDM builder issues (mostly perf related)
Hi all, I was looking over the NovaClusterDataModelCollector code today and trying to learn more about how watcher builds the nova CDM (and when) and got digging into this change from Stein [1] where I noted what appear to be several issues. I'd like to enumerate a few of those issues here and then figure out how to proceed. 1. In general, a lot of this code for building the compute node model is based on at least using the 2.53 microversion (Pike) in nova where the hypervisor.id is a UUID - this is actually necessary for a multi-cell environment like CERN. The nova_client.api_version config option already defaults to 2.56 which was in Queens. I'm not sure what the compatibility matrix looks like for Watcher, but would it be possible for us to say that Watcher requires nova at least at Queens level API (so nova_client.api_version >= 2.60), add a release note and a "watcher-status upgrade check" if necessary. This might make things a bit cleaner in the nova CDM code to know we can rely on a given minimum version. 2. I had a question about when the nova CDM gets built now [2]. It looks like the nova CDM only gets built when there is an audit? But I thought the CDM was supposed to get built on start of the decision-engine service and then refreshed every hour (by default) on a periodic task or as notifications are processed that change the model. Does this mean the nova CDM is rebuilt fresh whenever there is an audit even if the audit is not scoped? If so, isn't that potentially inefficient (and an unnecessary load on the compute API every time an audit runs?). 3. The host_aggregates and availability_zone compute audit scopes don't appear to be documented in the docs or the API reference, just the spec [3]. Should I open a docs bug about what are the supported audit scopes and how they work (it looks like the host_aggregates scope works for aggregate ids or names and availability_zone scope works for AZ names). 4. There are a couple of issues with how the unscoped compute nodes are retrieved from nova [4]. a) With microversion 2.33 there is a server-side configurable limit applied when listing hypervisors (defaults to 1000). In a large cloud this could be a problem since the watch client-side code is not paging. b) The code is listing hypervisors with details, but then throwing away those details to just get the hypervisor_hostname, then iterating over each of those node names and getting the details per hypervisor again. I see why this is done because of the scope vs unscoped cases, but we could still optimize this I think (we might need some changes to python-novaclient for this though, which should be easy enough to add). 5. For each server on a node, we get the details of the server in separate API calls to nova [5]. Why can't we just do a GET /servers/detail and filter on "host" or "node" so it's a single API call to nova per hypervisor? I'm happy to work on any of this but if there are any reasons things need to be done this way please let me know before I get started. Also, how would the core team like these kinds of improvements tracked? With bugs? [1] https://review.opendev.org/#/c/640585/ [2] https://review.opendev.org/#/c/640585/10/watcher/decision_engine/model/colle... [3] https://specs.openstack.org/openstack/watcher-specs/specs/stein/implemented/... [4] https://review.opendev.org/#/c/640585/10/watcher/decision_engine/model/colle... [5] https://review.opendev.org/#/c/640585/10/watcher/decision_engine/model/colle... -- Thanks, Matt
On 5/14/2019 3:34 PM, Matt Riedemann wrote:
2. I had a question about when the nova CDM gets built now [2]. It looks like the nova CDM only gets built when there is an audit? But I thought the CDM was supposed to get built on start of the decision-engine service and then refreshed every hour (by default) on a periodic task or as notifications are processed that change the model. Does this mean the nova CDM is rebuilt fresh whenever there is an audit even if the audit is not scoped? If so, isn't that potentially inefficient (and an unnecessary load on the compute API every time an audit runs?).
Also, it looks like https://bugs.launchpad.net/watcher/+bug/1828582 is due to a regression caused by that change. The problem is a nova notification is received before the nova CDM is built which results in an AttributeError traceback in the decision-engine logs. Should we be building the nova CDM if nova is sending notifications and there is no model yet? Or should we just handle the case that the nova CDM hasn't been built yet when we start getting notifications (and before an audit builds the CDM)? -- Thanks, Matt
On 5/14/2019 3:34 PM, Matt Riedemann wrote:
2. I had a question about when the nova CDM gets built now [2]. It looks like the nova CDM only gets built when there is an audit? But I thought the CDM was supposed to get built on start of the decision-engine service and then refreshed every hour (by default) on a periodic task or as notifications are processed that change the model. Does this mean the nova CDM is rebuilt fresh whenever there is an audit even if the audit is not scoped? If so, isn't that potentially inefficient (and an unnecessary load on the compute API every time an audit runs?).
Also, it looks like https://bugs.launchpad.net/watcher/+bug/1828582 is due to a regression caused by that change. The problem is a nova notification is received before the nova CDM is built which results in an AttributeError traceback in the decision-engine logs. Should we be building the nova CDM if nova is sending notifications and there is no model yet? Or should we just handle the case that the nova CDM hasn't been built yet when we start getting notifications (and before an audit builds the CDM)? [licanwei]:please refer to https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... When a nova notification is received before the nova CDM is built or no node in the CDM, the node will be add to the CDM. -- Thanks, Matt
On 5/15/2019 4:13 AM, li.canwei2@zte.com.cn wrote:
[licanwei]:please refer to <https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/notification/nova.py#L144>https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/notification/nova.py#L144
When a nova notification is received before the nova CDM is built or no node in the CDM,
the node will be add to the CDM.
That's not what's happening in this bug. We're getting an instance.update event from nova during scheduling/building of an instance before it has a host, so when this is called: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... node_uuid is None. Which means we never call get_or_create_node here: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... And then we blow up here: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... because self.cluster_data_model is None. Based on your earlier reply about when the nova CDM is built:
[licanwei]:Yes, the CDM will be built when the first audit being created.
It seems the fix for this notification traceback bug is to just make sure the self.cluster_data_model is not None and return early if it's not, i.e. gracefully handle receiving notifications before we've ever done an audit and built the nova CDM. -- Thanks, Matt
On 5/15/2019 4:13 AM, li.canwei2@zte.com.cn wrote:
[licanwei]:please refer to <https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/notification/nova.py#L144>https://github.com/openstack/watcher/blob/master/watcher/decision_engine/model/notification/nova.py#L144
When a nova notification is received before the nova CDM is built or no node in the CDM,
the node will be add to the CDM.
That's not what's happening in this bug. We're getting an instance.update event from nova during scheduling/building of an instance before it has a host, so when this is called: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... node_uuid is None. Which means we never call get_or_create_node here: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... And then we blow up here: https://github.com/openstack/watcher/blob/master/watcher/decision_engine/mod... because self.cluster_data_model is None. Based on your earlier reply about when the nova CDM is built:
[licanwei]:Yes, the CDM will be built when the first audit being created.
It seems the fix for this notification traceback bug is to just make sure the self.cluster_data_model is not None and return early if it's not, i.e. gracefully handle receiving notifications before we've ever done an audit and built the nova CDM. [licanwei]: In this case, I think we can just ignore the exception. -- Thanks, Matt
On 5/14/2019 3:34 PM, Matt Riedemann wrote:
1. In general, a lot of this code for building the compute node model is based on at least using the 2.53 microversion (Pike) in nova where the hypervisor.id is a UUID - this is actually necessary for a multi-cell environment like CERN. The nova_client.api_version config option already defaults to 2.56 which was in Queens. I'm not sure what the compatibility matrix looks like for Watcher, but would it be possible for us to say that Watcher requires nova at least at Queens level API (so nova_client.api_version >= 2.60), add a release note and a "watcher-status upgrade check" if necessary. This might make things a bit cleaner in the nova CDM code to know we can rely on a given minimum version.
I tried changing nova_client.api_version to a FloatOpt but that gets messy because of how things like 2.60 are handled (str(2.60) gets turned into '2.6' which is not what we'd want). I was hoping we could use FloatOpt with a min version to enforce the minimum required version, but I guess we could do this other ways in the client helper code itself by comparing to some minimum required version in the code. -- Thanks, Matt
On 5/14/2019 3:34 PM, Matt Riedemann wrote:
1. In general, a lot of this code for building the compute node model is based on at least using the 2.53 microversion (Pike) in nova where the hypervisor.id is a UUID - this is actually necessary for a multi-cell environment like CERN. The nova_client.api_version config option already defaults to 2.56 which was in Queens. I'm not sure what the compatibility matrix looks like for Watcher, but would it be possible for us to say that Watcher requires nova at least at Queens level API (so nova_client.api_version >= 2.60), add a release note and a "watcher-status upgrade check" if necessary. This might make things a bit cleaner in the nova CDM code to know we can rely on a given minimum version.
I tried changing nova_client.api_version to a FloatOpt but that gets messy because of how things like 2.60 are handled (str(2.60) gets turned into '2.6' which is not what we'd want). I was hoping we could use FloatOpt with a min version to enforce the minimum required version, but I guess we could do this other ways in the client helper code itself by comparing to some minimum required version in the code. [licanwei]: Maybe we can refer to https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.... Thanks, Matt
On 5/15/2019 4:19 AM, li.canwei2@zte.com.cn wrote:
I tried changing nova_client.api_version to a FloatOpt but that gets messy because of how things like 2.60 are handled (str(2.60) gets turned into '2.6' which is not what we'd want). I was hoping we could use FloatOpt with a min version to enforce the minimum required version, but I guess we could do this other ways in the client helper code itself by comparing to some minimum required version in the code. [licanwei]: Maybe we can refer to https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper....
I just did this which seems more explicit: https://review.opendev.org/#/c/659194/ That change leaves the default of 2.56 since the 2.56 code does version discovery so it's backward compatible, but I think we can assert that you need at least 2.53 because of how the scoped nova CDM code works (and to support nova deployments with multiple cells properly). Also note that 2.53 is pike-era nova and 2.56 is queens-era nova and those seem old enough that it's safe to require 2.53 as a minimum for watcher in train. -- Thanks, Matt
On 5/15/2019 4:19 AM, li.canwei2@zte.com.cn wrote:
I tried changing nova_client.api_version to a FloatOpt but that gets messy because of how things like 2.60 are handled (str(2.60) gets turned into '2.6' which is not what we'd want). I was hoping we could use FloatOpt with a min version to enforce the minimum required version, but I guess we could do this other ways in the client helper code itself by comparing to some minimum required version in the code. [licanwei]: Maybe we can refer to https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper....
I just did this which seems more explicit: https://review.opendev.org/#/c/659194/ That change leaves the default of 2.56 since the 2.56 code does version discovery so it's backward compatible, but I think we can assert that you need at least 2.53 because of how the scoped nova CDM code works (and to support nova deployments with multiple cells properly). Also note that 2.53 is pike-era nova and 2.56 is queens-era nova and those seem old enough that it's safe to require 2.53 as a minimum for watcher in train. [licanwei]: Because we need to specify the destination host when migration, at least 2.56 is required. https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.... -- Thanks, Matt
Hi all, I was looking over the NovaClusterDataModelCollector code today and trying to learn more about how watcher builds the nova CDM (and when) and got digging into this change from Stein [1] where I noted what appear to be several issues. I'd like to enumerate a few of those issues here and then figure out how to proceed. 1. In general, a lot of this code for building the compute node model is based on at least using the 2.53 microversion (Pike) in nova where the hypervisor.id is a UUID - this is actually necessary for a multi-cell environment like CERN. The nova_client.api_version config option already defaults to 2.56 which was in Queens. I'm not sure what the compatibility matrix looks like for Watcher, but would it be possible for us to say that Watcher requires nova at least at Queens level API (so nova_client.api_version >= 2.60), add a release note and a "watcher-status upgrade check" if necessary. This might make things a bit cleaner in the nova CDM code to know we can rely on a given minimum version. [licanwei]:We set the default nova api version to 2.56 , but it's better to add a release note 2. I had a question about when the nova CDM gets built now [2]. It looks like the nova CDM only gets built when there is an audit? But I thought the CDM was supposed to get built on start of the decision-engine service and then refreshed every hour (by default) on a periodic task or as notifications are processed that change the model. Does this mean the nova CDM is rebuilt fresh whenever there is an audit even if the audit is not scoped? If so, isn't that potentially inefficient (and an unnecessary load on the compute API every time an audit runs?). [licanwei]:Yes, the CDM will be built when the first audit being created. and don't rebuild if the next new audit with the same scope. 3. The host_aggregates and availability_zone compute audit scopes don't appear to be documented in the docs or the API reference, just the spec [3]. Should I open a docs bug about what are the supported audit scopes and how they work (it looks like the host_aggregates scope works for aggregate ids or names and availability_zone scope works for AZ names). [licanwei]:There is an example in CLI command 'watcher help create audittemplate' and it's a good idea to documented these. 4. There are a couple of issues with how the unscoped compute nodes are retrieved from nova [4]. a) With microversion 2.33 there is a server-side configurable limit applied when listing hypervisors (defaults to 1000). In a large cloud this could be a problem since the watch client-side code is not paging. b) The code is listing hypervisors with details, but then throwing away those details to just get the hypervisor_hostname, then iterating over each of those node names and getting the details per hypervisor again. I see why this is done because of the scope vs unscoped cases, but we could still optimize this I think (we might need some changes to python-novaclient for this though, which should be easy enough to add). [licanwei]: Yes, If novaclient can do some changes, we can optimize the code. 5. For each server on a node, we get the details of the server in separate API calls to nova [5]. Why can't we just do a GET /servers/detail and filter on "host" or "node" so it's a single API call to nova per hypervisor? [licanwei] This also depends on novaclient. I'm happy to work on any of this but if there are any reasons things need to be done this way please let me know before I get started. Also, how would the core team like these kinds of improvements tracked? With bugs? [licanwei]: welcome to improve Watcher. bug or other kind is not important [1] https://review.opendev.org/#/c/640585/ [2] https://review.opendev.org/#/c/640585/10/watcher/decision_engine/model/colle... [3] https://specs.openstack.org/openstack/watcher-specs/specs/stein/implemented/... [4] https://review.opendev.org/#/c/640585/10/watcher/decision_engine/model/colle... [5] https://review.opendev.org/#/c/640585/10/watcher/decision_engine/model/colle... -- Thanks, Matt
participants (2)
-
li.canwei2＠zte.com.cn
-
Matt Riedemann