| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-12 | |||
| 20:40:41 | openstackgerrit | Merged openstack/nova master: Document lack of side-effects in AllocationList.create_all() https://review.openstack.org/530997 | |
| 20:41:29 | mtreinish | mriedem: +A | |
| 20:49:05 | melwitt | lyarwood: your review needed please for the libvirt driver re-init on hard reboot backport https://review.openstack.org/#/c/531422 | |
| 21:00:47 | openstackgerrit | Eric Fried proposed openstack/nova master: Test helper: validate provider summaries https://review.openstack.org/518982 | |
| 21:00:47 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix accumulated nits in refactor series https://review.openstack.org/521189 | |
| 21:00:48 | openstackgerrit | Eric Fried proposed openstack/nova master: Add aggregates check in allocation candidates https://review.openstack.org/522407 | |
| 21:00:48 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix missing rps in allocation candidates https://review.openstack.org/522409 | |
| 21:00:49 | openstackgerrit | Eric Fried proposed openstack/nova master: placement: func tests for multiple shared RPs https://review.openstack.org/498737 | |
| 21:00:49 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix comments in get_all_with_shared() https://review.openstack.org/533195 | |
| 21:01:28 | fried_rolls | Cores, can I please get re+Ws on ^ after rebasing to pick up gibi fix? | |
| 21:03:09 | mriedem | fried_rice: done | |
| 21:03:15 | fried_rice | mriedem Thanks | |
| 21:03:36 | fried_rice | Considering how to deal with similar on https://review.openstack.org/#/c/521686/ series... | |
| 21:08:36 | mriedem | figleaf: thanks, comments ilnine | |
| 21:08:38 | mriedem | *inline | |
| 21:09:00 | mriedem | fried_rice: w/o rechecking? | |
| 21:09:12 | openstackgerrit | Merged openstack/nova master: Fix typo in release note https://review.openstack.org/531854 | |
| 21:09:32 | fried_rice | mriedem That bottom one has been languishing all week; I kinda don't want to try a recheck only to find that I needed a rebase. I'm gonna rebase the series... | |
| 21:10:16 | openstackgerrit | Eric Fried proposed openstack/nova master: Track provider traits in report client https://review.openstack.org/521686 | |
| 21:10:16 | openstackgerrit | Eric Fried proposed openstack/nova master: Raise conflict exception on RP create 409 https://review.openstack.org/532563 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: SchedulerReportClient.set_traits_for_provider https://review.openstack.org/532564 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: Track associated sharing RPs in report client https://review.openstack.org/526539 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: Raise on API errors getting aggregates/traits https://review.openstack.org/526540 | |
| 21:10:18 | openstackgerrit | Eric Fried proposed openstack/nova master: ProviderTree.populate_from_iterable https://review.openstack.org/520756 | |
| 21:10:19 | openstackgerrit | Eric Fried proposed openstack/nova master: Track tree-associated providers in report client https://review.openstack.org/526541 | |
| 21:10:19 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Scheduler[Report]Client.get_provider_tree https://review.openstack.org/521098 | |
| 21:10:20 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: ComputeDriver.update_provider_tree() https://review.openstack.org/521187 | |
| 21:10:20 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Use update_provider_tree from resource tracker https://review.openstack.org/520246 | |
| 21:10:21 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix nits in update_provider_tree series https://review.openstack.org/531260 | |
| 21:10:24 | fried_rice | mriedem Done, please re+W https://review.openstack.org/#/c/521686/ | |
| 21:25:42 | mriedem | fried_rice: it's just that one on the bottom right? | |
| 21:25:48 | fried_rice | mriedem Yessir. | |
| 21:26:33 | fried_rice | Been bouncing around zuul-land since Tuesday morning. | |
| 21:26:36 | fried_rice | Thank you sir. | |
| 21:37:26 | figleaf | mriedem: about your comment on https://review.openstack.org/#/c/526436/23/nova/conductor/tasks/migrate.py@242 | |
| 21:37:52 | figleaf | The _revert_allocation() method has already been called at that point | |
| 21:38:33 | mriedem | figleaf: _revert_allocation on the compute deletes the source node allocation for the migration record and moves it back to the instance so the instance allocations are doubled up on the source and dest node | |
| 21:38:48 | mriedem | it doesn't remove the allocation that the instance has on the dest node, just moves it back from migration->instance | |
| 21:39:10 | mriedem | but once we're back in conductor, the instance allocation on the dest node is now the wrong dest node (the failed one), and we need to move it from that to the alternate | |
| 21:39:21 | figleaf | I'm not seeing that in the tests | |
| 21:39:42 | mriedem | unless claim_resources for the instance does a total rewrite of what the instance consumer has allocations on, | |
| 21:39:44 | mriedem | that could be the case | |
| 21:39:49 | mriedem | is it an update/overwrite> | |
| 21:39:50 | mriedem | ? | |
| 21:40:00 | figleaf | It's a PUT, so overwrite | |
| 21:40:10 | mriedem | ok, well that's excellent news | |
| 21:40:29 | mriedem | then i think the only major thing is the migration obj lookup | |
| 21:40:44 | figleaf | mriedem: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1136 | |
| 21:41:26 | figleaf | I added a filter on status, and also added some allocation tests to verify that only the host that succeeds has allocations | |
| 21:41:27 | mriedem | figleaf: https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1096 | |
| 21:41:47 | mriedem | are you sure we don't hit https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1132 and it thinks we're doing a move | |
| 21:41:52 | mriedem | and double the allocations for the instance again? | |
| 21:42:18 | figleaf | mriedem: I put a break on that line and never hit it. Let me run it again to be sure | |
| 21:43:14 | mriedem | and your test is asserting the failed hosts don't have any allocations on them when we're done rescheduling? | |
| 21:43:34 | mriedem | i guess you said that already | |
| 21:43:48 | figleaf | yeah | |
| 21:44:21 | mriedem | the other thing is, dest_alloc_req is going to be in the new format here now https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L180 | |
| 21:44:29 | mriedem | so i'd expect to hit some issues there, like superdan did in his patch | |
| 21:44:40 | figleaf | I just re-ran the functional test with debug output for the current_allocs in the code, and it's empty every time | |
| 21:44:43 | mriedem | i could have sworn we talked about this during the reschedules on the server create path | |
| 21:44:45 | mriedem | *patch | |
| 21:45:39 | mriedem | idk, doesn't make sense to me that that report client code doesn't think we're doing a move operation b/c the instance should still have the allocation on the originally chosen dest node | |
| 21:45:56 | figleaf | mriedem: got the new format covered: https://review.openstack.org/#/c/526436/23/nova/tests/functional/test_servers.py@2730 | |
| 21:46:00 | mriedem | unless something is deleting that before we get there | |
| 21:46:54 | mriedem | figleaf: i don't understand why your functional test needs to stub any of that out | |
| 21:47:01 | mriedem | kind of defeats the purpose of using a functional test | |
| 21:48:15 | mriedem | anyway, i'm on serious vacation brain mode at this point and trying to wrap up some stuff around the house before i turn things off | |
| 21:48:19 | mriedem | so can't really dig into this in detail | |
| 21:50:03 | figleaf | mriedem: the only things that are stubbed/mocked are a) the prep_resize, so I can force it to pass/fail, and select_destinations, so I can return the selections and also verify that it only called once (i.e., alternates are used) | |
| 21:50:35 | mriedem | we shouldn't have to stub out select_destinations | |
| 21:50:39 | mriedem | i left a comment in the patch, | |
| 21:50:48 | mriedem | if you need predictable sort order on the hosts, add a custom weigher to the test | |
| 21:50:54 | mriedem | there are some examples in other tests | |
| 21:51:12 | figleaf | and how can I verify that it isn't called repeatedly? | |
| 21:52:22 | mriedem | if prep_resize is called > len(hosts) then you'd know right? | |
| 21:52:32 | mriedem | or max_attempts or whatever | |
| 21:54:08 | figleaf | prep_resize will be called once for each attempted migration | |
| 21:54:27 | figleaf | select_destinations is called from the task | |
| 22:14:51 | mriedem | figleaf: i think you were right about _revert_allocation, | |
| 22:15:10 | mriedem | i think that deletes the allocation for the migration record on the source node, and overwrites the instance allocation to only be on the source node, | |
| 22:15:30 | mriedem | i remember why you were hitting the format issue where the reschedules on build thought we were doubling up again when claiming on the alterante, | |
| 22:15:44 | mriedem | and it was because we hit that before the compute had removed the allocation the instance had on the failed node | |
| 22:15:55 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L4091 | |
| 22:16:02 | mriedem | "either way we need to cleanup any allocations created # by the scheduler for the destination node." | |
| 22:16:25 | mriedem | so yeah, that's why on the reschedule, claim_resources doesn't think we're doubling, because the instance doesn't have a claim on the old failed dest node | |
| 22:17:03 | figleaf | mriedem: yeah, that was the racy part that I fixed by moving it earlier before the cast | |
| 22:17:39 | figleaf | I ran some traces, and set_and_clear_allocations() is being called for each reschedule | |
| 22:22:14 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change compute RPC to use alternates for resize https://review.openstack.org/526436 | |
| 22:22:28 | figleaf | mriedem: ^^ if you still have brain cycles | |
| 22:22:59 | fried_rice | figleaf It occurs to me I should be prepping the agenda for the sched meeting. Is ^ the only remaining thing for alternates? | |
| 22:23:58 | figleaf | fried_rice: yes | |
| 22:24:08 | fried_rice | k | |
| 22:27:39 | mriedem | figleaf: comments inline, | |
| 22:27:52 | mriedem | figleaf: plus a question about ordering, something for probably superdan to look at on monday for a 2nd opinion | |
| 22:30:03 | figleaf | mriedem: ok, thx | |
| 22:30:21 | figleaf | mriedem: now turn off your brain and enjoy your holiday | |
| 22:45:41 | openstackgerrit | Ed Leafe proposed openstack/nova master: Change compute RPC to use alternates for resize https://review.openstack.org/526436 | |
| 22:48:27 | ildikov | mriedem: if you have any homework for me too lemme know otherwise have fun!!! :) | |
| 22:50:02 | mriedem | can't think of any | |
| 22:50:27 | mriedem | melwitt: i updated the novaclient release patch to depend on the top of the series for the novaclient removal changes, once those are merged we'd just need to modify the hash in the release patch and then that should be good to go | |