Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-08-23
19:57:58 exarr mriedem: I assume you're not talking to me? :-)
19:58:03 mriedem correct
20:00:37 cdent mriedem: maybe something else (non-local) changed in the interim?
20:01:11 mriedem it has been 3 years, but looking at the code at the time that change was added, it's basically the same code
20:06:29 dansmith mriedem: did you figure out anything with the oslo timeutils thing yesterday?
20:06:52 dansmith I'm getting a weird native/naive comparison failure in unit tests when loading an object
20:06:55 dansmith for no apparent reason
20:09:01 mriedem dansmith: nope
20:09:03 mriedem wasn't looking
20:09:06 openstackgerrit Matt Riedemann proposed openstack/nova master: Add some inline code docs tracing the cold migrate flow https://review.openstack.org/496861
20:09:09 dansmith okay
20:09:26 mriedem and got totally distracted by https://review.openstack.org/#/c/73387/
20:13:21 mriedem dansmith: melwitt: are we having a sassy cells meeting this week? we should talk about cells v2 stuff for the ptg at some point, but that could be next week
20:13:45 dansmith mriedem: melwitt is still out today afaik, so whatever you want
20:14:01 dansmith I have a few things I'm planning on starting here pretty soon, but nothing pressing that anyone needs to know about
20:14:18 dansmith so if you want to punt again that's cool, and if you want it to be just us, then that's cool too
20:17:33 mriedem is there anything that's *not* cool?
20:17:37 mriedem fonzy
20:17:52 mriedem let's just punt until next week when mel is back
20:21:19 cdent dansmith: i saw that comparison problem yesterday while messing with timestamps and ovo. somewhere I had utcnow() and using utcnow(with_timezone=True) fixed it. May be totally unrelated but your comment collided with a memory
20:21:33 dansmith yeah totally unrelated
20:21:46 dansmith I was not doing anything with timestamps at all,
20:21:52 dansmith but I figured out why I was triggering the comparison
20:22:14 dansmith don't know why it was doing a naive/aware comparison in the first place, but once I stopped doing my stupid thing it went away
20:22:20 dansmith mriedem: ack on the meeting
20:22:24 cdent no, worries, it cost me little to type that
20:24:02 openstackgerrit Matt Riedemann proposed openstack/nova master: Add missing tests for _remove_deleted_instances_allocations https://review.openstack.org/496847
20:35:50 openstackgerrit Chris Dent proposed openstack/nova-specs master: Add a spec for minimal cache headers in placement https://review.openstack.org/496853
20:39:40 openstackgerrit Matt Riedemann proposed openstack/nova-specs master: Add a new section: "Upgrade impact" to the template https://review.openstack.org/456756
20:40:38 openstackgerrit Ed Leafe proposed openstack/nova master: WIP - add alternate hosts https://review.openstack.org/486215
20:40:39 openstackgerrit Ed Leafe proposed openstack/nova master: WIP - Add allocations to the values returned from the scheduler https://review.openstack.org/495854
20:40:39 openstackgerrit Ed Leafe proposed openstack/nova master: return alternates along with their allocations https://review.openstack.org/486253
20:41:26 cfriesen_ with placement/allocations where do we free up resources on deletion of an instance?
20:43:20 exarr I have a pastebin link if anyone is willing to take a look :-( https://pastebin.com/7Qqfv6SU
20:44:21 mriedem cfriesen_: the resource tracker in the compute service
20:44:31 openstackgerrit Merged openstack/nova master: nova-manage: Deprecate 'cell' commands https://review.openstack.org/496815
20:46:10 cfriesen_ mriedem: which call? I see _update() calling self.scheduler_client.set_inventory_for_provider(), but that looks like it's just setting the inventory. I was expecting to see something removing an allocation.
20:49:55 mriedem _remove_deleted_instances_allocations
20:50:04 mriedem called from _update_usage_from_instances
20:50:24 mriedem called from update_usage
20:50:35 mriedem which is called from ComputeManager._update_resource_tracker
20:50:45 cdent cfriesen_: it begins wiith a great comment: “all this code sucks”
20:50:48 mriedem which is called from _complete_deletion
20:52:10 cfriesen_ mriedem: cdent: update_usage() calls _update_usage_from_instance(), with no "s" on the end
20:52:30 mriedem yeah you're right, which would call update_instance_allocation if we had ocata computes
20:52:58 mriedem _update_usage_from_instances is called via the periodic task
20:53:06 openstackgerrit Ed Leafe proposed openstack/nova master: docs: Document the scheduler workflow https://review.openstack.org/475810
20:53:11 mriedem update_available_resource in the manager
20:53:23 cfriesen_ mriedem: okay, so currently pike won't free up deleted resources until the audit runs? that sucks.
20:54:06 openstackgerrit Chris Dent proposed openstack/nova-specs master: Add a spec for minimal cache headers in placement https://review.openstack.org/496853
20:54:13 mriedem that appears to be the case
20:55:04 mriedem yeah the ServerMovingTests functional tests paper over this by forcing the run of the periodic before checking the allocations are gone in Placement
20:55:09 mriedem cfriesen_: open a bug
20:55:12 cfriesen_ will do
20:55:31 cfriesen_ do we have a unit test for resource update on instance deletion?
20:57:06 mriedem we have functional tests that assert the allocations are removed after the periodic runs, that's the ServerMovingTests functional i mentioned
20:57:24 mriedem but since those force the periodic to run, we glossed over the fact that we're having to wait for the audit
21:01:01 cdent I was under the impression that periodic being required for deletes was effectively a known issue, something we decided was just how it is for now. I agree that we should have a bug for it.
21:02:37 cfriesen_ seems potentially confusing that it'll get freed up immediately in a mixed Ocata/Pike cloud, but once you're fully Pike it's audit-based.
21:03:59 mriedem i just don't know that we thought about the delete case
21:04:05 mriedem the ocata/pike stuff was for a different issue
21:04:16 mriedem where ocata computes would overwrite non-deleted instances being moved to another host
21:04:24 mriedem overwrite allocations i mean
21:04:38 mriedem so not surprisingly while fixing one thing, another issue is introduced
21:06:09 cfriesen_ https://bugs.launchpad.net/nova/+bug/1712684
21:06:10 openstack Launchpad bug 1712684 in OpenStack Compute (nova) "allocations not immediately removed when instance deleted" [Undecided,New]
21:16:07 mriedem rt.delete_allocation_for_shelve_offloaded_instance(instance)
21:16:07 mriedem yeah so recently (last week), this was added when shelve offloading an instance
21:16:45 mriedem which is basically the exact same thing that happens during the audit when getting the no longer tracked instance results in an InstanceNotFound
21:17:46 cdent there’s for_migrated and for_evacuated as well
21:17:57 mriedem those aren't deleted instances
21:19:01 cdent yeah, I just stumbled on them and realized they are identical
21:19:30 cdent (supporting the theory that the fixing going in concurrently has left some gaps)
21:20:05 mriedem http://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22Failed%20to%20clean%20allocation%20of%20a%20shelve%20offloaded%5C%22%20AND%20tags%3A%5C%22screen-n-cpu.txt%5C%22&from=10d
21:20:07 mriedem shite ^
21:21:34 mriedem ffs you know why
21:21:37 mriedem b/c if True
21:22:10 mriedem gdi, ok patching that quick
21:25:44 cfriesen_ delete_allocation_for_migrated_instance() was explicitly copied from the evacuate case
21:26:00 mriedem both of those call a different method
21:26:06 mriedem which returns a boolean
21:26:11 mriedem i just looked at those
21:28:34 openstackgerrit Matt Riedemann proposed openstack/nova master: How about not logging errors every time we shelve offload? https://review.openstack.org/496930
21:28:35 mriedem dansmith: are you going to make me change this commit message title? ^
21:29:21 dansmith proper capitalization, grammar, and punctuation.. it's better than 100% of sdague's messages, so I don't see the problem
21:29:33 mriedem ha
21:29:39 mriedem i'm going to say it
21:29:46 mriedem I like cookies. I also like pizza.
21:29:50 dansmith lol
21:29:55 cfriesen_ Betteridge's law of headlines says the answer is "no"
21:30:48 openstackgerrit Dan Smith proposed openstack/nova master: Add placeholder migrations for Pike backports https://review.openstack.org/496932
21:30:49 openstackgerrit Dan Smith proposed openstack/nova master: Add uuid to migration object and migrate-on-load https://review.openstack.org/496934
21:30:49 openstackgerrit Dan Smith proposed openstack/nova master: Add uuid to migration table https://review.openstack.org/496933
21:31:12 dansmith mriedem: need to land that placeholder patch fairly soonish
21:32:07 cfriesen_ it feels wrong somehow to +1 a patch that has no test changes. :)
21:35:17 openstackgerrit Chris Dent proposed openstack/nova master: De-duplicate two delete_allocation_for_* methods https://review.openstack.org/496936
21:35:55 cdent mriedem: that ^ is not necessary, but would be great to merge once the dust settles, so we can avoid some duplication
21:36:10 mriedem dansmith: good point, i hadn't looked over https://wiki.openstack.org/wiki/Nova/ReleaseChecklist
21:36:48 mriedem cdent: yeah that came up when the 2nd method was added
21:38:58 cdent welp, now it’s ready for whenever

Earlier   Later