| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-20 | |||
| 17:52:50 | mriedem | conductor, but note, that diff is against master branch code | |
| 17:53:20 | tasker | let me see if I can adapt to the branch that I'm using. | |
| 17:54:09 | mriedem | http://paste.openstack.org/show/621559/ is latest stable/newton | |
| 17:55:12 | tasker | cool, thanks. I'm going to step out for some "fresh air" and then apply this. | |
| 17:55:19 | mriedem | it does stand to reason that if this instance failed to build originally on those 2 hosts, that live migrating it there might fail too...but we don't know why it originally failed, could have been a resource claim issue at the time | |
| 17:55:36 | tasker | that's a fair observation. | |
| 17:57:56 | melwitt | yeah, often it's a failed claim. and also what if that compute host is eventually replaced over the lifetime of the cluster, making it a fresh candidate for several instances that might still avoid it because they once failed to build there back when it was a different machine | |
| 18:06:45 | tasker | live migration successful | |
| 18:08:17 | tasker | mriedem and melwitt -- thank you so much for helping to figure out what was wrong. | |
| 18:11:01 | mriedem | nice | |
| 18:11:07 | mriedem | tasker: want to open a bug? i have a fix with the test locally | |
| 18:11:40 | tasker | sure. let me collect my notes. | |
| 18:12:31 | cdent | dansmith: responded to some of your comments on https://review.openstack.org/#/c/500410/ . you semi-accidentally identified a separate problem. The nullable thing I’m not quite sure how to proceed, depending on what we want to do. | |
| 18:14:56 | mriedem | cdent: congratulations, you have the first complete blueprint in queens https://blueprints.launchpad.net/nova/+spec/placement-deregister-objects | |
| 18:15:35 | dansmith | cdent: replied, I don't think there's an issue.. just make those not nullable and (separately) always set them and I think we're good | |
| 18:24:25 | cdent | dansmith: except that they are only ever used on input, never output, so why bother reading them? | |
| 18:25:01 | tasker | mriedem: https://bugs.launchpad.net/nova/+bug/1718512 | |
| 18:25:02 | openstack | Launchpad bug 1718512 in OpenStack Compute (nova) "migration fails if instance build failed on destination host" [Undecided,New] | |
| 18:25:07 | mriedem | thanks | |
| 18:25:19 | dansmith | cdent: because it's (effectively) free, it makes the object consistent | |
| 18:25:26 | tasker | I hope that summary / description is clear enough. | |
| 18:28:40 | cdent | dansmith: okay, I’m happy to do that. I’m not sure why consistency matters _now_ but if we’d like it as a general rule, that’s fine. | |
| 18:29:48 | dansmith | cdent: clearly it doesn't have a user now, but we're an abstract model on top of the data store and unless there is a reason not to, we should do that thing, IMHO | |
| 18:30:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Ignore original retried hosts when live migrating https://review.openstack.org/505771 | |
| 18:30:17 | mriedem | tasker: ^ i'm going to wip that and send something to the dev/ops lists for this, because if we change this for live migration, we also have to do it for cold migrate/evacuate and unshelve | |
| 18:30:23 | mriedem | melwitt: ^ | |
| 18:30:43 | melwitt | ack | |
| 18:30:58 | cdent | dansmith: does YAGNI count as a reason not to? | |
| 18:31:42 | dansmith | cdent: I dunno what GNI is in there, but if you think there's a reason not to, then don't do it and we can just leave my -1 on there | |
| 18:32:52 | cdent | I’ve just always been brought up on the idea of not doing things unless there is a reason to do so, but as I said, I’m happy to do it and will do it. | |
| 18:34:24 | mriedem | if they are in the same table it seems easy peasy to just load them, it'd be one thing if we were lazy loading a big table join or something, | |
| 18:34:42 | mriedem | but if/when someone needs these, it's going to be weird debugging why they just aren't already in the object when it's read from the db | |
| 18:34:45 | mriedem | like every other object we have | |
| 18:34:48 | dansmith | mriedem: it's a join, but it's super tiny | |
| 18:34:52 | mriedem | oh | |
| 18:35:12 | mriedem | like the usages/consumers table join thing? | |
| 18:35:13 | dansmith | mriedem: like a 1:1 integer join | |
| 18:35:15 | dansmith | yes | |
| 18:35:16 | dansmith | consumers | |
| 18:35:39 | mriedem | i'm assuming at some point we actually intend on using these fields? | |
| 18:36:07 | dansmith | mriedem: if they're fields they should be loadable, and if we're never going to use them that way then maybe we shouldn't have them be fields | |
| 18:36:31 | mriedem | yeah i'd agree with that | |
| 18:36:36 | mriedem | i don't have context here though clearly | |
| 18:36:46 | dansmith | since consumer isn't a top level object in placement, getting them with allocations is the only way we can pull them out otherwise | |
| 18:36:51 | dansmith | unless we're querying for a consumer already | |
| 18:37:18 | dansmith | like if we needed to get all allocations against a resource provider | |
| 18:48:45 | mriedem | tasker: http://lists.openstack.org/pipermail/openstack-operators/2017-September/014233.html | |
| 18:49:08 | openstackgerrit | Chris Dent proposed openstack/nova master: WIP: [placement] manage cache headers https://review.openstack.org/495380 | |
| 18:55:54 | tasker | mriedem: thanks! I'll keep an eye out on these. | |
| 18:57:45 | tasker | mriedem and melwitt: I want to thank you two for taking the time to work with me on my issues for the last few days. your support has been a positive influence in my cloudy days. with your help, I've been able to progress with my upgrade trials. | |
| 18:59:20 | tasker | If you two are ever in the Kansas City area, let me know. first round's on me. | |
| 19:05:32 | mriedem | tasker: cool, you're welcome. you hit some weird issues so i'm glad we flushed those out and have fixes | |
| 19:13:18 | bauzas | mriedem: back now, wazzup ? | |
| 19:14:45 | mriedem | bauzas: http://lists.openstack.org/pipermail/openstack-operators/2017-September/014233.html | |
| 19:18:09 | bauzas | holy shit | |
| 19:18:34 | bauzas | mriedem: honestly, no reason to persist that | |
| 19:18:58 | bauzas | mriedem: if some host wasn't accepted for an instance create, there is no reason it will be excluded for a live migration | |
| 19:19:14 | bauzas | at least, we should still verify it by the scheduler | |
| 19:20:17 | bauzas | mriedem: but here, the main problem is that I should possibly modify the ReqSpec object to have a specific way to say which fields are needed to be persisted and others not | |
| 19:20:43 | bauzas | mriedem: that's something we didn't discussed with alaski when he provided the .save() method | |
| 19:21:43 | melwitt | tasker: yep, thanks for helping us fix those bugs. and I learned yet more things I didn't know about live migration and retries | |
| 19:27:55 | mriedem | bauzas: yeah i was thinking about renaming the reset_forced_destinations() method to something like reset_for_move() and that would reset forced_hosts/forced_nodes/retry | |
| 19:28:08 | mriedem | so it could be handled for live migrate/cold migrate/unshelve and evacuate in the same place | |
| 19:29:05 | bauzas | mriedem: yeah, or just modify _get_update_primitives to disallow the non-needed fields | |
| 19:29:29 | bauzas | mriedem: like using a propery that'd give you which fields are non-persistent | |
| 19:29:51 | bauzas | that way a call to .create() or .save() would avoid persist those | |
| 19:29:57 | bauzas | persisting* | |
| 19:30:07 | mriedem | that won't help us for any existing req specs | |
| 19:30:12 | bauzas | but that wouldn't solve existing specs | |
| 19:30:14 | bauzas | yeah | |
| 19:30:49 | bauzas | okay, I think we could just rename reset_forced_dests() | |
| 19:30:59 | bauzas | and modify the comments of course | |
| 19:31:03 | mriedem | yeah | |
| 19:31:09 | mriedem | to fix all 4 move operations | |
| 19:31:13 | mriedem | i'm going to want to backport this too | |
| 19:31:18 | bauzas | want me to help on that or you feel enough brave ? | |
| 19:31:28 | mriedem | i'm brave | |
| 19:31:34 | bauzas | good boy | |
| 19:31:53 | mriedem | anything that can help me procrastinate from reviewing dansmith's efficient multi-cell instance list / sort thing | |
| 19:31:55 | mriedem | :) | |
| 19:32:06 | bauzas | honestly, I think it would be a good opportunity for discussing which fields shouldn't be persisted | |
| 19:32:20 | dansmith | mriedem: my set should be pretty easy to follow I think | |
| 19:32:20 | bauzas | like requested_destination | |
| 19:32:34 | dansmith | mriedem: I tried really hard to add functionality in layers | |
| 19:32:51 | mriedem | dansmith: yeah, i'm going to hit it now - i'll wait to deal with this reqspec stuff for tomorrow morning | |
| 19:32:55 | mriedem | gives me something to look forward to | |
| 19:40:17 | cdent | dansmith: there’s loads of pre-existings tests lying around that don’t require AllocationList to have a project and user. That makes it feel like more effort than it is worth to start requiring it in the Allocation object, so what I’m thinking of doing is not requiring it, but if they are there in the DB, loading it up when reading the object. Concur? | |
| 19:41:00 | dansmith | cdent: I'm not sure what you mean.. surely we require it from the API now right? | |
| 19:41:22 | dansmith | I guess one of the early microversions didn't require it so we _could_ have things in the DB that don't? | |
| 19:41:43 | mriedem | consumers requires project_id/user_id in the API in the latest microversion, | |
| 19:41:48 | mriedem | allocations doesn't | |
| 19:42:31 | melwitt | I thought PUT allocations required project/user | |
| 19:42:36 | cdent | they do | |
| 19:42:46 | cdent | but we don’t enforce that at the object level | |
| 19:42:51 | cdent | only the http api level | |
| 19:43:09 | dansmith | cdent: we should have migrated any existing allocations explicitly or incidentally | |
| 19:43:22 | cdent | what I’m saying is: to make the change to requiring things will change a pile of tests | |
| 19:43:31 | dansmith | cdent: if the http api always forces it (which it should) then we should raise an error when we hit one in the DB that doesn't, not just return an object with those things missing | |
| 19:43:45 | melwitt | the existing allocations get overwritten upon update with newer code to have project/user | |
| 19:43:53 | melwitt | (periodic update) | |