| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-07-12 | |||
| 21:27:54 | dansmith | doing claim in conductor made that super symmetric | |
| 21:28:09 | dansmith | doing it in scheduler means it's pre-claimed before we return, which is what I was arguing against in the first place :) | |
| 21:30:17 | jaypipes | dansmith: but we can't claim in the conductor when we have num_instances > 0 unfortunately. see: instance groups, affinity, and similar issues. | |
| 21:31:25 | edleafe | jaypipes: so assuming a retry is necessary, how will the cell conductor deal with those things? | |
| 21:32:16 | jaypipes | edleafe: unfortunately, the only way to deal with those things on retry is the last-minute check on the alternate target host :( | |
| 21:32:35 | dansmith | jaypipes: I don't think I understand why that would be, but you guys steamrolled past that anyway, so it doesn't matter | |
| 21:33:19 | jaypipes | dansmith: because of this line: https://github.com/openstack/nova/blob/master/nova/scheduler/filter_scheduler.py#L127-L128 | |
| 21:33:29 | jaypipes | dansmith: those little lines are the bane of my existence. | |
| 21:34:24 | dansmith | I don't see how that affects claiming in the conductor, but whatever, not important | |
| 21:52:49 | mriedem | melwitt: so i didn't get to the resize stuff yet https://review.openstack.org/#/c/416521/ but with fresh eyes i think i've found 2 issues already discussed about over quota on multiple create not cleaning up, and restore | |
| 22:04:00 | melwitt | mriedem: hm, okay. will look at that | |
| 22:21:12 | melwitt | mriedem: replied about restore. reading through multi create | |
| 22:28:03 | melwitt | mriedem: though now I see the change to state "soft-deleted" doesn't happen until after an rpc call to compute, so I think that's why I didn't bother with recheck_quota there | |
| 22:28:41 | melwitt | maybe I should change it to set the state right away before the cast. not sure if that would be okay | |
| 22:37:34 | mriedem | ok still going through some other parts of this | |
| 22:38:18 | openstackgerrit | Maciej Kucia proposed openstack/nova-specs master: Extended PCI alias https://review.openstack.org/474218 | |
| 22:42:21 | mriedem | ok reading your replies now | |
| 22:45:53 | mriedem | hmm, so when you soft delete an instance, | |
| 22:45:58 | mriedem | the deleted_at field is set on the instance | |
| 22:46:08 | mriedem | but when you restore it, i don't see the deleted_at field set to None | |
| 22:46:14 | mriedem | so it's both ACTIVE and deleted_at | |
| 22:46:32 | melwitt | *thumbs up* | |
| 22:46:38 | mriedem | unless i'm missing something | |
| 22:46:46 | melwitt | I doubt it | |
| 22:47:10 | melwitt | either it was missed or someone thought it helpful to let it show that it had been deleted in the past | |
| 22:47:12 | mriedem | i don't have a devstack handy | |
| 22:47:21 | mriedem | yeah....but instance actions... | |
| 22:47:26 | mriedem | if you want an audit record | |
| 22:47:47 | melwitt | yeah | |
| 22:49:55 | openstackgerrit | Vipin Balachandran proposed openstack/nova master: VMware:Use power_state const for power state check https://review.openstack.org/483122 | |
| 22:53:10 | mriedem | melwitt: ok, replies are all in - still haven't gotten t oresize | |
| 22:53:12 | mriedem | *to resize | |
| 22:53:29 | mriedem | to summarize, i think we're ok with restore, except maybe that we don't have a recheck deltas in there? | |
| 22:53:46 | mriedem | and i also got a bit hung up on the fact we aren't accounting for the hw video ram when counting existing usage | |
| 22:53:49 | mriedem | as dansmith pointed out | |
| 22:53:57 | mriedem | for some reason i was ok with that last night, but now i'm not so sure | |
| 22:54:37 | melwitt | mriedem: yeah, I think restore is fine. we might probably want to move the vm_state change to compute/api and do a recheck yeah | |
| 22:55:01 | mriedem | oh i see, | |
| 22:55:18 | melwitt | mriedem: yeah, for the majority of the time, I think we were counting video ram on allocate and not on deallocate. someone recently fixed a bug to add subtraction of video ram usage upon deallocate | |
| 22:55:26 | mriedem | because today it's (1) check, (2) change state (api or compute), and rechecking at 1.5 wouldn't do anything since the state didn't change to affect the count, | |
| 22:55:40 | mriedem | so to recheck, we'd have to (1) check, (2) change state, (3) recheck, (4) cast | |
| 22:55:48 | melwitt | yes | |
| 22:56:07 | mriedem | and older computes are setting the vm state to ACTIVE but that shouldn't cause a problem | |
| 22:56:17 | mriedem | if the api did it i mean | |
| 22:56:52 | mriedem | it's a bit of a lie though...for example, if we set the state to ACTIVE, counted, and then cast to compute, but the rpc message never got there | |
| 22:57:03 | mriedem | we'd say the instance is active even though the guest is dead | |
| 22:57:19 | melwitt | oh yeah. maybe it could be a problem if an old compute goes to set SOFT_DELETED and it's already SOFT_DELETED (if it expected a different vm_state) | |
| 22:57:45 | mriedem | i think vm_state is ok, | |
| 22:58:03 | mriedem | instance.save(expected_task_state=[task_states.SOFT_DELETING]) | |
| 22:58:03 | mriedem | the problem is task_state | |
| 22:58:03 | mriedem | you can set that to ACTIVE twice and it's fine | |
| 22:58:13 | melwitt | oh, guh sorry getting it backward. restore sets to active | |
| 22:58:13 | mriedem | and setting vm_state=ACTIVE while task_state is SOFT_DELETING is a problem, | |
| 22:58:15 | mriedem | because when an instance is ACTIVE it's task_state is supposed to be None | |
| 22:58:19 | mriedem | right | |
| 22:59:06 | mriedem | you would almost have to temporarily change the state to active for the recheck, and then set the vm_state back to SOFT_DELETED for the cast to the compute - which makes me barf a little | |
| 22:59:11 | melwitt | yeah. that would suck to call it active if RPC message lost | |
| 22:59:35 | melwitt | lemme see | |
| 23:00:15 | mriedem | melwitt: btw, let me hand you a power pack of good will for sticking with this, because this makes my brain bleed | |
| 23:00:18 | melwitt | okay yeah there's no in between vm_state of RESTORING. that was a task_state | |
| 23:02:09 | melwitt | mriedem: heh ... thanks. I felt like a burned out husk for quite a time. props to you and those who have braved reviewing it. I kind of cringed every time I asked someone to review it | |
| 23:04:36 | melwitt | mriedem: these state changes in the API change deleted_at to None. and I guess I had been thinking to just punt this and assume parallel restores are not really a thing in real life | |
| 23:06:08 | melwitt | maybe I could add a filter to count RESTORING as usage too and do handle the recheck that way. I dunno | |
| 23:07:28 | mriedem | ha | |
| 23:07:32 | mriedem | instance.deleted_at = None | |
| 23:07:37 | mriedem | right in front of my gd eyes | |
| 23:08:01 | melwitt | mine too. and I still didn't see it until like the fourth time I looked at it | |
| 23:15:40 | mriedem | melwitt: so, | |
| 23:15:46 | mriedem | re the hw video ram thing, | |
| 23:15:54 | mriedem | we might want to just burn that turd out of the api before your series | |
| 23:16:01 | dansmith | +10,000,000 | |
| 23:16:05 | melwitt | lol | |
| 23:16:06 | mriedem | it was added back in icehouse https://review.openstack.org/#/q/topic:bp/libvirt-video-driver-selection | |
| 23:16:12 | dansmith | going over that just now, that's nutsypants mcgee | |
| 23:16:20 | mriedem | and is the only special virt-driver specific extra spec accounted against quota in the api | |
| 23:16:39 | melwitt | well, that makes me feel bad for the person who has been trying to fix those video ram quota bugs | |
| 23:16:54 | melwitt | but yeah, I was thinking to at least start off with no video ram | |
| 23:16:55 | mriedem | well, they fixed something that was screwing up their quota | |
| 23:17:00 | melwitt | yes | |
| 23:17:04 | mriedem | if we don't count it against usage in the first place, that's not a problem | |
| 23:17:12 | melwitt | yeah, that's what I was thinking | |
| 23:17:40 | melwitt | if anything they'll get more out of their quota if we don't count it :P | |
| 23:18:32 | mriedem | well, it's not like there aren't other things we don't account for in quota, which are calculated as overheads in the virt driver | |
| 23:18:37 | mriedem | this could potentially move there for the RT claim stuff | |
| 23:18:41 | mriedem | or wherever | |
| 23:18:43 | melwitt | yeah | |
| 23:18:53 | melwitt | fine by me. makes my life easier too | |
| 23:19:11 | mriedem | i'll probably poke the ops list first to be safe | |
| 23:19:20 | mriedem | dansmith: agree i assume? ^ | |
| 23:19:49 | dansmith | mriedem: I'd rather you not but whatever | |
| 23:19:56 | mriedem | so sneaky remove? | |
| 23:20:07 | dansmith | bugfix, imho | |
| 23:20:36 | melwitt | yeah. the bug where video ram usage wasn't ever decremented, that was a bug from whenever the feature was added to ... like last month | |
| 23:20:50 | melwitt | so I'm guessing around 5 years at least | |
| 23:20:59 | dansmith | dat bug yo | |
| 23:21:08 | melwitt | I assume people can't have cared about it too much | |
| 23:21:31 | dansmith | only people with video ram called out too | |
| 23:22:57 | mriedem | https://review.openstack.org/#/c/456050/ | |
| 23:23:02 | mriedem | was the recent fix | |
| 23:23:51 | dansmith | rage quit! | |