| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-08-30 | |||
| 14:05:33 | mriedem | if the allocation ratio in the db record is 16.0 but the object value in the RT is 0.0, we know that _copy_resources is what's changing our in-memory value and we're not persisting the change | |
| 14:06:14 | sean-k-mooney | mriedem: well we dont actully want to persist the change to the db in this case | |
| 14:06:22 | sean-k-mooney | the db has the correct default | |
| 14:06:54 | mriedem | the db values are likely actually NULL | |
| 14:07:03 | mriedem | which is what the compute node object keys off of | |
| 14:07:09 | bauzas | mriedem: so we directly change the object value without reading it from the DB thru the facade ? | |
| 14:07:14 | mriedem | https://github.com/openstack/nova/blob/f534495a427d1683bc536cf003ec02edbf6d8a45/nova/objects/compute_node.py#L194 | |
| 14:07:21 | sean-k-mooney | the db perhaps but the object that is constrted form the db entry gets defaulted correctly | |
| 14:07:48 | sean-k-mooney | yep that is the line i was thinking of | |
| 14:07:53 | mriedem | sean-k-mooney: yes but my theory is we're not "fixing" the allocation ratios within the object after setting the values to 0.0 | |
| 14:07:59 | mriedem | because of https://review.openstack.org/#/c/520024/ | |
| 14:08:25 | mriedem | zigo: is it possible that you have ^ in your nova package somehow? | |
| 14:08:31 | sean-k-mooney | right because we removed the update call which update the resouce tracker | |
| 14:08:51 | mriedem | zigo: iow, are your nova rocky packages based on stable/rocky or 18.0.0 tags rather than just pulling from master? | |
| 14:09:27 | sean-k-mooney | mriedem: we proably should have kept the self._update on line 574 | |
| 14:09:50 | mriedem | that wouldn't have helped us in this case, | |
| 14:09:58 | mriedem | that's for a nova-compute restart where the cn record already exists, | |
| 14:10:04 | mriedem | what we're hitting is the condition above | |
| 14:10:05 | bauzas | mriedem: just to be clear, https://github.com/openstack/nova/blob/f534495a427d1683bc536cf003ec02edbf6d8a45/nova/objects/compute_node.py#L199-L207 is only intended to be executed if on nova-scheduler | |
| 14:10:28 | bauzas | mriedem: because https://github.com/openstack/nova/blob/f534495a427d1683bc536cf003ec02edbf6d8a45/nova/objects/compute_node.py#L198 will always tell you a value if you're on nova-compute | |
| 14:10:46 | mriedem | umm | |
| 14:11:06 | mriedem | that will also always tell you a value if you're on nova-scheduler | |
| 14:11:09 | mriedem | b/c conf is global | |
| 14:11:15 | mriedem | and the value defaults to 0.0 in config | |
| 14:12:19 | bauzas | if executed in separate workers, CONF.cpu_allocation_ratio wouldn't be defined for nova-scheduler | |
| 14:12:36 | bauzas | oh wait, sec | |
| 14:12:51 | mriedem | it doesn't need to be defined in config, we have a default | |
| 14:12:53 | mriedem | which is global | |
| 14:13:17 | mriedem | anyway, that doesn't really matter for this bug | |
| 14:13:28 | mriedem | the compute reports the inventory to placement and is reporting 0.0 allocation ratios | |
| 14:14:20 | cdent | mriedem: one thing that remains unclear for me (becuase apparently I can't read python code) is why the second inventory PUT (the one with the 0.0) is happening at all (and so soon) | |
| 14:18:03 | bauzas | mriedem: I think your working theory you stated in the bug comment is valid | |
| 14:18:22 | bauzas | I'm trying to wrap my head around on exactly when we pull the DB values | |
| 14:19:04 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Revert "Update resources once in update_available_resource" https://review.openstack.org/598176 | |
| 14:21:12 | bauzas | mriedem: I think we began having problems with https://github.com/openstack/nova/commit/7b95b4d60726b6c8d0e0fe939c408a91ada79e0c | |
| 14:21:36 | bauzas | I'm not saying it's all the bugs cause | |
| 14:21:57 | bauzas | just that it implicitly creates a dependency on us calling .save() after it | |
| 14:22:10 | bauzas | because if not, we would then have 0.0 values | |
| 14:23:48 | mriedem | well, it seems pretty obvious to me that we just shouldn't set 0.0 values on the compute node record in _copy_resources if the config is still just 0.0 | |
| 14:24:31 | bauzas | mriedem: sure, we could leave the defaults | |
| 14:24:44 | bauzas | because once we pull the object, the facade fixes it for free | |
| 14:27:01 | mriedem | except we're no longer pulling the object in the RT | |
| 14:27:09 | mriedem | which is i think the problem | |
| 14:27:58 | zigo | mriedem: Sure, let me add it. | |
| 14:28:24 | zigo | mriedem: Yes, I package tags. | |
| 14:29:06 | mriedem | zigo: you don't want to add it | |
| 14:29:24 | bauzas | mriedem: sorry, you mean we're getting the CPU, RAM and disk values out of the DB directly ? | |
| 14:29:39 | bauzas | I thought we were still getting by id ? | |
| 14:29:42 | mriedem | zigo: i'm asking because https://review.openstack.org/#/c/520024/ is in master only, not rocky, but it looks like the culprit of the failure - but that wouldn't then explain how you'd be failing in rocky | |
| 14:30:17 | bauzas | me looks at https://github.com/openstack/nova/blob/f534495a427d1683bc536cf003ec02edbf6d8a45/nova/compute/resource_tracker.py#L85 | |
| 14:31:35 | zigo | mriedem: I have rc2 packaged, not rc3. | |
| 14:31:38 | zigo | Maybe I should update? | |
| 14:32:08 | mriedem | yeah probably | |
| 14:32:13 | mriedem | or just the GA | |
| 14:32:16 | mriedem | which was released today? | |
| 14:32:36 | mriedem | https://github.com/openstack/nova/tree/18.0.0 | |
| 14:32:59 | mriedem | yeah 20 minutes ago http://git.openstack.org/cgit/openstack/nova/tag/?h=18.0.0 | |
| 14:35:59 | zigo | Ah, right ! | |
| 14:36:01 | zigo | Doing that. | |
| 14:36:21 | cdent | I'm thinking that the positive way of looking at this is that the second _update has been masking a nasty bug for a long time | |
| 14:41:48 | mriedem | oh i'm not surprised that there would be a big pile of tight coupling tape holding this all together | |
| 14:42:05 | mriedem | and why i didn't want to merge that change before the GA | |
| 14:49:43 | bauzas | mriedem: again, the more I read, the more I think we possibly had the original problem once we had https://github.com/openstack/nova/commit/7b95b4d60726b6c8d0e0fe939c408a91ada79e0c | |
| 14:49:53 | bauzas | and .update() was just hiding it | |
| 14:50:12 | bauzas | mriedem: so, do you want me to modify the above and only set the values if not 0.0 ? | |
| 14:50:19 | bauzas | or are you working on this ? | |
| 14:52:34 | bauzas | disclaimer: looking at a long list of openstack-dev threads | |
| 15:00:27 | mriedem | Kevin_Zheng: yikun: you might be interested in https://review.openstack.org/#/c/591976/ | |
| 15:00:47 | mriedem | bauzas: i'm working it | |
| 15:00:59 | bauzas | okay | |
| 15:01:21 | Kevin_Zheng | Got it | |
| 15:02:31 | zigo | Nova 18.0.0 built, doing a recheck... | |
| 15:03:01 | openstackgerrit | Merged openstack/nova master: Fix soft deleting vm fails after "nova resize" vm https://review.openstack.org/546920 | |
| 15:04:36 | melwitt | . | |
| 15:16:00 | melwitt | stephenfin: on https://review.openstack.org/595592, if the bug is new for rocky, how did moshele run into it on OSP13 (queens)? | |
| 15:16:24 | stephenfin | melwitt: If he's using OSP, then it's because we backported it downstream | |
| 15:16:38 | stephenfin | Even if not, I'd imagine it's a feature backport | |
| 15:18:17 | melwitt | stephenfin: ohh, ok. I didn't think of that, backport of a feature | |
| 15:30:40 | mriedem | efried: does any kind of nova/cyborg integration actually exist to do anything with servers in rocky/ | |
| 15:30:40 | mriedem | ? | |
| 15:31:02 | mriedem | b/c https://docs.openstack.org/releasenotes/cyborg/rocky.html says it does | |
| 15:31:36 | mriedem | 1) rocky release notes talk about queens specs | |
| 15:31:47 | mriedem | 2) it sounds like "we completed specs" rather than "we have functioning code" | |
| 15:32:51 | Kevin_Zheng | mriedem: you made me laugh | |
| 15:33:43 | mriedem | i just don't want people showing up in -nova saying "hey why can't i attach fpgas to my vm?" | |
| 15:38:28 | efried | mriedem: No, cyborg has nothing in nova atm | |
| 15:38:46 | efried | oh, yeah, that's poorly worded. | |
| 15:41:16 | Kevin_Zheng | mriedem: we were talking today, about the batch when listing patch | |
| 15:42:24 | Kevin_Zheng | The batch size could vary depending on sort key and dir | |
| 15:42:44 | Kevin_Zheng | Like for sort with uuid | |
| 15:43:01 | Kevin_Zheng | It could be evenly distributed | |
| 15:43:24 | dansmith | Kevin_Zheng: you mean the *optimal* batch size? | |
| 15:43:40 | Kevin_Zheng | dansmith: yeah | |
| 15:44:40 | dansmith | Kevin_Zheng: obviously if you're sorting by uuid it should be fairly evenly distributes and sorting by other things could be massively less well-distributed, but I'm not sure how we could (efficiently) optimize that at runtime | |
| 15:44:45 | dansmith | Kevin_Zheng: do you have ideas? | |
| 15:45:00 | Kevin_Zheng | I was thinking a tool that analyzes dB data | |
| 15:45:16 | Kevin_Zheng | And feed to nova periodically | |
| 15:45:30 | Kevin_Zheng | But that seems to much :) | |
| 15:45:36 | dansmith | yeah :) | |
| 15:46:08 | dansmith | it likely varies by tenant, sort_key, cloud layout, scheduler weights, etc | |