Posted	Nick	Remark
#openstack-nova - 2017-07-12
21:00:35	jaypipes	edleafe: well, not *yet*... I'm currently referring to what filter scheduler _schedule() returns to filter scheduler's select_destinations() call.
21:00:45	jaypipes	edleafe: still need to figure out what's best for the RPC changes.
21:01:01	jaypipes	edleafe: it *may* be that structure...
21:01:12	jaypipes	haven't fully thought the RPC layer changes through yet
21:01:26	edleafe	jaypipes: I'm still not clear why the selected host has to be in a separate structure than the alternates
21:03:30	jaypipes	edleafe: the alternates are meant to be opaque structures. the selected host is the host identifier. the allocation requests are alternate things that can be passed to placement to claim resources if the original destination host fails to fully launch the instance.
21:04:14	jaypipes	edleafe: or are you asking to just have a list where the first element of the list is a WeighedHost and the other elements of the list are allocation request blobs?
21:05:16	edleafe	jaypipes: the distinction seems unnecessary. Why not send a list of (host, alloc) for each instance, where the first item in the list is the selected host? This way, if the build fails, the cell conductor can easily unclaim the resources already claimed
21:06:05	jaypipes	dansmith: thoughts? ^
21:06:19	dansmith	jaypipes: we're in the cells meeting right now
21:06:26	jaypipes	oh, sorry.
21:06:44	edleafe	jaypipes: passing a heterogenous structure seems like it will lead to a lot of conditional blocks to handle the different cases
21:07:19	jaypipes	edleafe: I don't disagree with you on that
21:22:48	dansmith	jaypipes: I would expect the compute to un-claim before passing the reschedule attempt to conductor, since it will un-claim always on failed boot, and then punt to conductor if it thinks it should be retried
21:23:02	mriedem	dansmith: dtp: melwitt: https://etherpad.openstack.org/p/nova-pike-cells-v2-todos
21:23:07	mriedem	i'll start dumping
21:23:23	melwitt	cool, thanks
21:23:45	dansmith	jaypipes: so I'm not sure that we need anything too complicated, and was kinda expecting the conductor to do the next claim in order, and then take the would-be-versioned _response_ from placement as an indication of which compute node to use
21:24:05	dansmith	jaypipes: however, if passing (hostname, allocation) around is more convenient or symmetric then that's fine
21:24:18	dansmith	jaypipes: your select-destinations can't return a HostState outside of the scheduler though
21:24:27	jaypipes	dansmith: yeah, I think edleafe's suggestion is a good one.
21:25:15	dansmith	jaypipes: I'm not sure what he's suggesting, other than just making it a list where the implication is that we try the first one as the "selected host"
21:25:44	dansmith	I assumed your example of the weighedhost as the selected one was for efficiency of the next step in the process
21:25:49	edleafe	dansmith: yeah, that's pretty much it :)
21:26:07	jaypipes	dansmith: he's suggesting returning a list of (host, allocation_request) tuples for each instance_uuid in the request
21:26:40	dansmith	obviously that's the most you can return from the rpc call, if that's what you're talking about, but you said WeighedHost() above, so I figured you were talking internal still
21:27:31	dansmith	but yeah, I think I said before that just returning a list of destinations per instance, and processing them in order in each phase is the way I think of it
21:27:41	dansmith	like, I would expect each round to just pop the first one off the list,
21:27:47	dansmith	try it, and move on
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	you can set that to ACTIVE twice and it's fine
22:58:03	mriedem	the problem is task_state
22:58:03	mriedem	instance.save(expected_task_state=[task_states.SOFT_DELETING])
22:58:13	mriedem	and setting vm_state=ACTIVE while task_state is SOFT_DELETING is a problem,
22:58:13	melwitt	oh, guh sorry getting it backward. restore sets to active
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

Earlier   Later