| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-31 | |||
| 17:44:10 | mriedem | god this is terrible | |
| 17:44:14 | jroll | removing the copy seems bad for ironic though | |
| 17:44:26 | mriedem | yeah that was efried's point | |
| 17:44:35 | jroll | right, ok | |
| 17:52:21 | mriedem | even with that clear at the beginning, | |
| 17:52:28 | mriedem | i think we're not tracking stats properly per node | |
| 17:52:41 | mriedem | because we're looping over instances managed by this host, | |
| 17:52:47 | mriedem | and instances are 1:1 with nodes, | |
| 17:52:50 | mriedem | for ironic, | |
| 17:53:01 | mriedem | and then we do this *per instance*: | |
| 17:53:05 | mriedem | self.stats.update_stats_for_instance(instance, is_removed_instance) | |
| 17:53:05 | mriedem | cn.stats = copy.deepcopy(self.stats) | |
| 17:53:32 | mriedem | so over the course of let's say 10 instances, wouldn't the stats on all by the first node be wrong? | |
| 17:53:55 | mriedem | i.e. stats.num_instances > 1 for all but the first node | |
| 18:00:04 | dansmith | mriedem: we have one RT per node, thus one stats per node, yeah? | |
| 18:00:39 | mriedem | don't think so, | |
| 18:00:50 | mriedem | compute manager has a single RT which is managing multiple nodes | |
| 18:01:23 | mriedem | that's why we have ResourceTrack.compute_nodes | |
| 18:02:08 | mriedem | the problem is for vmware, a single host manages 1500 VMs or something | |
| 18:02:19 | dansmith | I always forget how this works, | |
| 18:02:22 | mriedem | and the RT does a deepcopy on the stats object per instance during the update_available_resource periodic | |
| 18:02:38 | dansmith | but I thought we did have multiple RTs but the single-rt bits of compute manager only work when they're 1:1 | |
| 18:03:21 | dansmith | I guess I'm wrong, | |
| 18:03:23 | mriedem | that might have been true before 2016 | |
| 18:03:29 | dansmith | oh | |
| 18:03:30 | mriedem | when jaypipes refactored all of the node management in the RT | |
| 18:03:33 | dansmith | ah | |
| 18:03:44 | dansmith | I see we call it with a node name | |
| 18:04:04 | mriedem | https://review.openstack.org/#/c/398473/ | |
| 18:04:14 | mriedem | ^ makes ComputeManager go from >1 RT to 1 RT | |
| 18:04:18 | dansmith | ah yeah | |
| 18:04:22 | dansmith | glad I'm not crazy | |
| 18:04:45 | mriedem | so, | |
| 18:04:49 | mriedem | i think i'm right, | |
| 18:04:58 | mriedem | in that self.stats would be cumulative for all nodes managed by this host | |
| 18:05:03 | mriedem | in the before times, it wouldn't, | |
| 18:05:06 | mriedem | because we'd have 1 RT per node | |
| 18:05:10 | mriedem | managed by the compute manager | |
| 18:07:20 | mriedem | hmm, i guess no major baremetal deployments have been affected since ocata, or aren't at ocata yet, or aren't using the ComputeCapabilitiesFilter | |
| 18:07:27 | mriedem | or they are affected but don't know it | |
| 18:07:44 | jroll | tripleo uses ComputeCapabilitiesFilter for everything, iirc | |
| 18:07:46 | dansmith | baremetal stats reporting has been wrong for a long time right? | |
| 18:07:55 | jroll | you'd think they'd have noticed by now | |
| 18:08:05 | mriedem | i guess it depends on what they are using it for, | |
| 18:08:19 | mriedem | HostState.num_instances, which is derived from stats.num_instances, would be wrong | |
| 18:08:42 | mriedem | but i think the ironic driver passes through stats from the ironic node which goes into the stats object which then gets reported to the HostState for filtering | |
| 18:09:16 | mriedem | yeah https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L330 | |
| 18:09:28 | mriedem | ironic node capabillities get proxied to the scheduler via the Stats object | |
| 18:12:34 | mriedem | jroll: i have to assume you guys at oath are using ComputeCapabilitiesFilter for ironic scheduling | |
| 18:12:38 | mriedem | or have you moved to traits? | |
| 18:12:48 | jroll | mriedem: we are not | |
| 18:13:01 | jroll | our flavors are basically ram/disk/cpu or model numbers | |
| 18:13:12 | mriedem | mgagne: are you using ComputeCapabilitiesFilter for baremetal scheduling? | |
| 18:15:23 | jroll | I think dtantsur|afk would be a good person to talk to about this, but would be tomorrow morning | |
| 18:16:10 | mriedem | yeah i need to get a functional test working to actually verify that we have busted stats per node | |
| 18:16:25 | mriedem | e.g. 2 nodes on a single host, and show that compute_node.stats.num_instances > 1 for the 2nd node processed | |
| 18:16:31 | mriedem | b/c that should never be true with ironic | |
| 18:17:03 | dansmith | tripleo gate jobs have that filter enabled | |
| 18:17:24 | mriedem | do their flavors have any capabilities extra specs? | |
| 18:17:25 | dansmith | they also have their own filter in front of it | |
| 18:18:01 | dansmith | idk, looking for evidence | |
| 18:19:22 | dansmith | not sure I'll be able to find it really | |
| 18:23:02 | mgagne | mriedem: yes, heavily iirc http://lists.openstack.org/pipermail/openstack-dev/2018-April/130022.html | |
| 18:24:06 | mriedem | mgagne: ok, is your baremetal deployment running >=ocata? | |
| 18:24:08 | mriedem | or still mitaka? | |
| 18:24:56 | mgagne | nova and ironic are mitaka | |
| 18:25:07 | mriedem | ok, so you wouldn't have seen this issue yet | |
| 18:25:22 | mriedem | only other group i know to ask is cern | |
| 18:28:25 | openstackgerrit | Merged openstack/nova stable/queens: Fix TypeError in prep_resize allocation cleanup https://review.openstack.org/581741 | |
| 18:28:31 | openstackgerrit | Merged openstack/nova master: [placement] Retry allocation writes server side https://review.openstack.org/586048 | |
| 18:28:40 | openstackgerrit | Merged openstack/nova master: Remove redundant join in _anchors_for_sharing_providers https://review.openstack.org/587377 | |
| 18:28:47 | openstackgerrit | Merged openstack/nova stable/queens: Handle HostMappingNotFound when deleting a compute service https://review.openstack.org/583566 | |
| 18:28:55 | openstackgerrit | Merged openstack/nova master: Don't poison Host._init_events if it's already mocked https://review.openstack.org/585748 | |
| 18:30:51 | mgagne | mriedem: we do have a patch for _node_resource | |
| 18:32:03 | mgagne | mriedem: we use it to include more properties from the Ironic node to the resource stats | |
| 18:32:47 | mriedem | ok. my worry is that a change in ocata makes the stats reported per compute node no longer per compute node, i.e. makes it cumulative across all nodes managed by a given host | |
| 18:33:04 | mriedem | working on a functional test wrinkle to show it | |
| 18:33:07 | mgagne | hmm | |
| 18:33:12 | mgagne | our use case: https://gist.github.com/mgagne/078859de7bd8e3e8e93e64f9cf756ad0 | |
| 18:34:10 | mgagne | where inheritable_host_properties is a list of props we wish to inherit from the Ironic node, currently some project/user reservation IDs and supported network models which are all scoped per Ironic node. | |
| 18:43:28 | sean-k-mooney | melwitt: thanks for taking a look at the disk_bus bug. ill create some better test cases to reproduce the bug and then assert that this fixes it. | |
| 18:45:55 | melwitt | sean-k-mooney: cool, thanks | |
| 18:53:05 | openstackgerrit | Merged openstack/nova stable/queens: unquiesce instance after quiesce failure https://review.openstack.org/581451 | |
| 19:19:26 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add functional scale test for Stats.__deepcopy__ time https://review.openstack.org/587606 | |
| 19:19:47 | mriedem | efried: heh ^ | |
| 19:20:13 | efried | ... | |
| 19:22:30 | mriedem | "Took 4.21 with stats deepcopy, took 3.85 without stats deepcopy" for 100 instances | |
| 19:22:39 | mriedem | but that's totally variable at that scale | |
| 19:22:43 | mriedem | and has failed a couple of times | |
| 19:23:37 | efried | mriedem: Is it also going to matter that we're running "locally"? | |
| 19:24:43 | mriedem | locally? | |
| 19:24:43 | efried | um | |
| 19:24:56 | efried | yeah, meaning no remote-ness from conductor to compute and all that. | |
| 19:25:32 | mriedem | yeah for all the db stuf | |
| 19:25:47 | mriedem | obviously would be better done in a devstack env | |
| 19:55:26 | mriedem | i've got a recreate on that leaked node.stats thing | |
| 20:00:06 | efried | woot | |
| 20:04:17 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add recreate test for bug 1784705 https://review.openstack.org/587614 | |
| 20:04:17 | openstack | bug 1784705 in OpenStack Compute (nova) "ResourceTracker.stats can leak across multiple ironic nodes" [High,Triaged] https://launchpad.net/bugs/1784705 | |
| 20:04:18 | mriedem | efried: jaypipes: dansmith: ^ | |