14:00:16 <edleafe> #startmeeting nova_scheduler
14:00:17 <openstack> Meeting started Mon Oct  2 14:00:16 2017 UTC and is due to finish in 60 minutes.  The chair is edleafe. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:18 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:20 <openstack> The meeting name has been set to 'nova_scheduler'
14:00:26 <edleafe> Good UGT morning! Who's here?
14:00:27 <mriedem> o/
14:00:35 <takashin> o/
14:00:38 <cdent> o/
14:00:41 <bauzas> \o
14:00:46 <bauzas> (at least for 25 mins)
14:02:22 <edleafe> Let's get started
14:02:23 <edleafe> #topic Specs
14:02:36 <edleafe> #link Return Alternate Hosts https://review.openstack.org/504275/
14:02:36 <edleafe> #link Return Selection objects https://review.openstack.org/498830/
14:02:44 <mriedem> i had comments on the first
14:02:46 <edleafe> These two are mine
14:02:51 <edleafe> mriedem: yeah, saw them
14:03:11 <edleafe> was waiting until the caffeine fully kicked in before jumping into that
14:03:21 <mriedem> the two main things i had left where,
14:03:47 <mriedem> 1. you said you don't expect changes to the compute rpc, but then if we're not persisting the alternate hosts, how do you propose to pass the alternate hosts through for the reschedule loop?
14:04:04 <mriedem> superconductor -> scheduler -> superconductor -> compute -> cell conductor -> compute2 -> cell conductor -> compute3
14:04:18 <mriedem> the cell conductor / compute dance is going to need the alternate hosts passed through
14:04:29 <bauzas> yup, that was my point in the 2nd spec
14:04:36 <bauzas> I need to review then the first one
14:04:55 <mriedem> i haven't read the 2nd, i thought it was just about the object modeling
14:05:05 <edleafe> mriedem: yeah, I got that. Like I said, I'll get to it later
14:05:05 <bauzas> there are 2 possibilities, but AFAIR we said to pass by a parameter
14:05:15 <bauzas> the alternative was to use the ReqSpec object
14:05:16 <edleafe> Yes, the second is just the object.
14:05:29 <mriedem> i don't want to use the request spec
14:05:40 <mriedem> so yeah pass as an rpc parameter
14:05:50 <mriedem> if the compute is queens, it will get it and send it to conductor,
14:06:02 <bauzas> mriedem: yeah that was the consensus
14:06:04 <mriedem> if conductor gets called from a pike compute, it won't have alternate hosts and you're done
14:06:34 <bauzas> (even if I think we should still pass the ReqSpec record anyway :)
14:06:36 <mriedem> ok the only other thing on the first was i disagreed with the assertion that notifications will change
14:06:44 <mriedem> but that's pretty minor
14:07:39 <edleafe> Next up
14:07:39 <edleafe> #link nova-spec add traits to /allocation_candidates https://review.openstack.org/497713
14:07:41 <bauzas> edleafe: any reason why we have 2 specs ?
14:07:42 <edleafe> Still trying to get best name for the required traits query param
14:08:11 <mriedem> i have comments in ^ too,
14:08:13 <edleafe> bauzas: We really only needed the alternate hosts as a spec
14:08:24 <bauzas> agreed
14:08:28 <mriedem> i've given up on the name of the parameter, 'required' is fine i guess
14:08:34 <mriedem> it's been discussed at length
14:08:43 <mriedem> my -1 was on the behavior of passing ?required=
14:08:44 <edleafe> bauzas: but as there was some disagreement about what the selection object should look like, I wrote a spec so that we could get agreement
14:09:13 <mriedem> i did some testing and if you pass GET /servers?status='' to the compute api, we take that literally
14:09:13 <edleafe> mriedem: my objection to 'requires' was wearing my API-SIG hat
14:09:19 <mriedem> meaning give me all instances with status=''
14:09:32 <mriedem> and i don't think ^ is what we want
14:09:42 <edleafe> computers do tend to take things literally :)
14:10:08 <mriedem> so i was saying, we either do the same and ?required='' means give me all allocatoin candidates that specifically don't have any traits
14:10:11 <mriedem> which is weird,
14:10:25 <mriedem> or we just require that if you specify "required" it must be length > 0
14:10:55 <edleafe> since 'required' is a list of traits, sending '' means an empty list, no?
14:11:24 <mriedem> as a user, how do you expect that to be interpreted?
14:11:27 <edleafe> whereas 'status' is a single string
14:11:48 <mriedem> the spec says required='' would be ignored
14:12:01 <edleafe> "required=''" means "I don't require any traits"
14:12:29 <mriedem> then don't specify the parameter...
14:12:41 <mriedem> it just seems like very confusing API behavior
14:12:42 <edleafe> that would be preferable, sure
14:13:01 <cdent> I’m confused on how this ever became an issue?
14:13:18 <cdent> why was empty param ever proposed in the first place?
14:13:22 <mriedem> i said from early in the spec i didn't like treaing ?required='' as ignoring it
14:13:22 <edleafe> 'status' is saying: match this status. 'required' is saying: "only give me results with these traits"
14:13:32 <mriedem> because alex said it would be easier to code client-side that way
14:14:06 <mriedem> edleafe: sure, and i think it could be reasonable for a user to think '' means give me RPs with *no* traits whatsoever
14:14:07 <cdent> I don’t think that’s worth it
14:14:14 <mriedem> cdent: i don't either, it's ambiguous behavior
14:14:18 <mriedem> and inconsistent with the compute api
14:14:46 <mriedem> https://review.openstack.org/#/c/497713/2/specs/queens/approved/add-trait-support-in-allocation-candidates.rst@67
14:14:49 <cdent> Since alex is away I can fix it up tonight or tomorrow morn
14:14:51 <mriedem> was alex's response to me about it
14:15:15 <mriedem> well i'd like to make sure we all either agree, or at least don't care
14:15:25 <mriedem> including jaypipes and johnthetubaguy since they are +2 on the spec
14:15:51 * cdent looks around
14:16:08 <mriedem> we can follow up after the meeting
14:16:13 <mriedem> i'll just leave the -1 for now
14:16:17 <johnthetubaguy> yeah, lets catch up after
14:16:22 <edleafe> next up are 2 from cdent
14:16:23 <edleafe> #link POST /allocations https://review.openstack.org/#/c/499259/
14:16:23 <edleafe> Has 1 +2
14:16:23 <edleafe> #link Minimal cache headers in Placement https://review.openstack.org/#/c/496853/
14:16:26 <edleafe> Also has 1 +2
14:17:00 <cdent> first one requires https://review.openstack.org/#/c/508164/
14:17:35 <edleafe> You beat me to it :)
14:17:37 <edleafe> #link Add spec for symmetric GET and PUT of allocations https://review.openstack.org/#/c/508164/
14:18:25 <cdent> I have a 4th spec in progress which I think is getting in the weeds about details without agreement on the goal: (limiting /allocation_candidates) https://review.openstack.org/#/c/504540/
14:18:26 <jaypipes> mriedem: I'd prefer it be an error to provide required=<empty>
14:18:35 <mriedem> jaypipes: ok same here
14:18:43 <jaypipes> mriedem: but I don't feel it enough to hold up the spec.
14:18:46 <cdent> as I say on there I’m going to rewrite the spec to be more limited unless there are objections
14:18:55 <edleafe> cdent: no, that sounds good
14:19:32 <cdent> jaypipes, mriedem: give me a signal if you want me to update alex’s spec
14:19:43 <mriedem> let's confirm with john after the meeting
14:20:25 <edleafe> ok then - last spec that I have:
14:20:26 <edleafe> #link Re-propose nested resource providers spec https://review.openstack.org/#/c/505209/
14:20:29 <edleafe> This is going down the rabbit hole of NUMA and NFV permutations
14:20:44 <mriedem> yeah i saw that....
14:20:51 * cdent passes edleafe the semphore flags
14:20:52 <mriedem> i was happyish when it was just a re-proposal,
14:20:58 <jaypipes> cdent: go for it if it'll make mriedem happy.
14:21:01 <mriedem> and then it turned into the generic device mgmt thing
14:21:12 * cdent is all about making mriedem happy
14:21:35 <mriedem> would i be correct in saying the nested rp's spec is less of a re-proposal now and requires a new read through?
14:21:48 <jaypipes> mriedem: no.
14:21:49 <johnthetubaguy> didn't we decide a simple first step on nested, I can't remember what that was, bandwith monitoring?
14:21:50 <bauzas> yeah
14:21:53 <bauzas> mriedem: +1
14:21:54 <jaypipes> mriedem: it's a re-proposal.
14:22:10 <mriedem> we said no numa use cases to start at the ptg
14:22:18 <jaypipes> johnthetubaguy: no, PCI devices.
14:22:24 <edleafe> I added a little clarifying language to placate some comments, but nothing substantial changed
14:22:26 <jaypipes> johnthetubaguy: without nUma
14:22:36 <johnthetubaguy> jaypipes: ++
14:22:47 <mriedem> edleafe: ok. the amount of commenting back and forth made me think it was changing quite a bit
14:22:54 <mriedem> will re-review
14:22:57 <jaypipes> mriedem: no, it has not.
14:23:01 <jaypipes> (changed much)
14:23:09 <bauzas> it's more about the usage of what we could do
14:24:18 <edleafe> Anyone else have any specs to discuss that I missed?
14:24:29 <johnthetubaguy> I have that ironic one (finds link)
14:24:38 * bauzas needs to disappear for 20 mins
14:24:55 <johnthetubaguy> https://review.openstack.org/#/c/507052/
14:25:05 <johnthetubaguy> basically using traits with ironic
14:25:28 <johnthetubaguy> I think the deeper question, is who should set the traits? second, how should they do that?
14:25:35 <johnthetubaguy> I mean generally
14:25:43 <edleafe> #link Support traits in the Ironic driver #link Re-propose nested resource providers spec https://review.openstack.org/#/c/505209/
14:25:50 <johnthetubaguy> is the answer: Operator + PlacementClient
14:25:55 <johnthetubaguy> edleafe: thanks
14:25:57 <edleafe> #undo
14:25:58 <openstack> Removing item from minutes: #link https://review.openstack.org/#/c/505209/
14:26:25 <edleafe> #link Support traits in the Ironic driver https://review.openstack.org/#/c/507052/
14:26:32 * edleafe can't copy/paste
14:27:25 <edleafe> Moving on then...
14:27:26 <edleafe> #topic Reviews
14:27:29 <edleafe> #link Add traits to GET /allocation_candidates https://review.openstack.org/479776
14:27:30 <johnthetubaguy> so at the PTG we were talking more about copying what we do with ResourceClasses, which is the set them in Ironic (using inspector, etc) then push up to placement in the ironic virt driver
14:27:58 <johnthetubaguy> OK, no response on that one then, for the moment?
14:28:30 <mriedem> in general,
14:28:35 <edleafe> johnthetubaguy: I'll take a look at the spec later
14:28:38 <mriedem> i'd like nova to not be a proxy for ironic
14:28:57 <johnthetubaguy> mriedem: yeah, I am ++ that
14:29:02 <mriedem> i haven't reviewed either spec, but it sounds like you might be proposing that ironic-inspector could create the traits?
14:29:36 <johnthetubaguy> so that is how the resource class is set, but its only talking ot the ironic API today
14:29:37 <mriedem> which could get us into some split brain scenario, but maybe not terrible if the rp generation changes when traits are added/removed
14:29:40 <mriedem> do we know if ^ is true?
14:30:27 <edleafe> AFAIK, no, traits don't change generation. cdent?
14:30:32 <johnthetubaguy> thinking about this, inspector can't talk to placement, as it will race nova-compute creating the RP
14:30:34 <cdent> i need to look, one sec
14:30:59 <cdent> generation is updated on set trait
14:31:26 <mriedem> johnthetubaguy: well, one would just get a 409
14:31:28 <mriedem> which could be ignored
14:31:49 <johnthetubaguy> mriedem: inspector only really runs once when you are commissioning the node
14:31:54 <edleafe> cdent: ah, you're right. I was looking at the trait creation code
14:31:58 <mriedem> but that's why i wondered about the generation, because we don't want to the user to chnage traits on the node via ironic api, and then have inspector and nova-compute racing to update the traits
14:31:59 <jaypipes> johnthetubaguy: well the ironic virt driver would need to do the same thing with traits as it currently does with node.resource_class
14:32:03 <johnthetubaguy> mriedem: it can go in a retry loop, but thats nasty
14:32:23 <mriedem> why can't ironic itself just call placement?
14:32:25 <johnthetubaguy> jaypipes: thats the current proposal, I think its the safest, but nasty approach
14:32:37 <jaypipes> johnthetubaguy: whatever method Ironic wants to use to expose those traits, fine by us. The Ironic virt driver will just need to ask Ironic for that information the way it needs to.
14:32:53 <jaypipes> johnthetubaguy: in other words, decouple any expectations between Nova and Ironic :)
14:33:17 <mriedem> don't forget about mogan :)
14:33:17 <johnthetubaguy> jaypipes: that makes the virt driver a bit of a proxy though, which is what we don't really want
14:33:26 <mriedem> anything you do as a proxy in the nova ironic driver, they will copy in mogan
14:33:30 <cdent> It sounds like the question is in part about whether ironic will talk to placement itself, or nova will talk to ironic to talk to placement
14:33:39 <cdent> I think we should encourage the former
14:33:41 <jaypipes> johnthetubaguy: that's exactly how we handle libvirt, hyper-v, etc, though
14:34:01 <johnthetubaguy> jaypipes: they report traits already?
14:34:05 <mriedem> no
14:34:14 <mriedem> but they will for some standard types i think
14:34:17 <jaypipes> johnthetubaguy: libvirt does a get_devices('pci') and looks at capabilities crap and jams it into the resources dict returned from get_avilable_resource(). hyperv and xen do similar but different underlying calls.
14:34:20 <mriedem> like vgpus
14:34:34 <jaypipes> johnthetubaguy: they put them in extra specs now, but yes.
14:34:55 <jaypipes> johnthetubaguy: and I mean cpu_info
14:35:08 <johnthetubaguy> I mean its like ironic and capabilities now, which they want to drop once traits is working
14:35:09 <jaypipes> johnthetubaguy: which is matched with the extra specs for compute capabilities filter.
14:35:12 <mriedem> the stuff an operator would put on an ironic node for traits would be mostly custom traits, yes?
14:35:29 <johnthetubaguy> mriedem: not all, but most
14:35:39 <cdent> gotta dash, will catch up on log later
14:35:39 <jaypipes> mriedem: could be. or could be standardized if the hardware folks decide on some standard traits.
14:35:40 * cdent waves
14:36:01 <mriedem> i'm just trying to compare what we would be doing for libvirt/hyperv wrt custom traits, and i guess the answer is flavor extra specs?
14:36:27 <johnthetubaguy> well, so ironic will use the same flavor extra specs for the request
14:36:44 <jaypipes> mriedem: well, right now all the virt drivers "inspect" the host in their own way and jam capabilities into the cpu_info key of the resource dict returned by get_avaialable_resource()
14:37:37 <jaypipes> mriedem: we'd like to change that to have the virt drivers passed a ProviderTree and have them associate traits with the providers in that tree and then have the RT and scheduler report client do the needful as far as communicating with placement.
14:37:41 <jaypipes> mriedem: make sense?
14:37:45 <mriedem> and we're going to translate those capabilities to traits on the compute node resource provider?
14:38:03 <jaypipes> mriedem: no, the virt drivers would tag a provider with a set of traits
14:38:33 <johnthetubaguy> jaypipes: you mean in get_inventory() ?
14:38:35 <jaypipes> mriedem: which is what they do today only instead of tagging a provider with traits, they just return a JSON blob of randomness in the resources dict returned from get_available_resource()
14:38:41 <jaypipes> johnthetubaguy: yep.
14:38:59 <mriedem> jaypipes: right and then ComputeCapabilitiesFilter processes that json bloc
14:39:00 <mriedem> *blob
14:39:06 <jaypipes> johnthetubaguy: though with the nested resource providers series, we're changing that to update_inventory() but yeah, same conecepot
14:39:07 <mriedem> or whatever other scheduler filter cares
14:39:12 <jaypipes> mriedem: yup
14:39:20 <jaypipes> mriedem: and we'd love to get rid of that awfulness.
14:39:37 <johnthetubaguy> so I think jaypipes might (mostly) like the spec, I think mriedem might (mostly) hate the spec, in its current state
14:39:40 <jaypipes> mriedem: nope, it's just the cOmputecapabilitiesfilter.
14:39:53 <mriedem> jaypipes: and whatever out of tree filters i mean :)
14:39:57 <johnthetubaguy> jaypipes: that is good to know, these ironic patches would sit behind that I guess
14:39:59 <jaypipes> mriedem: ah, yeah
14:40:01 <mriedem> johnthetubaguy: well, given ^ i might hate it less
14:40:06 <jaypipes> johnthetubaguy: sorry, which spec are we referringh to?
14:40:18 <johnthetubaguy> https://review.openstack.org/#/c/507052
14:40:19 <mriedem> johnthetubaguy: the ironic driver being consistent with the other drivers is ok by me
14:40:19 <jaypipes> I've lost track of which spec... :(
14:40:26 <jaypipes> ah!
14:40:38 <johnthetubaguy> mriedem: it sounds more consistent than I expected, which is nice
14:40:42 <jaypipes> johnthetubaguy: well I had not even seen that spec yet! :)
14:40:50 <jaypipes> johnthetubaguy: so I don't know if I like it or not ;)
14:40:58 * johnthetubaguy mission accomplished.
14:41:00 <jaypipes> :)
14:41:04 * jaypipes reviews now
14:41:32 <johnthetubaguy> cool, thanks, I think that helps me feel better about plan A for the spec
14:43:48 <edleafe> Well now... we're running a bit long today, so rather than go through the reviews one-by-one, let me dump them all and if any tickle your interest, we can discuss
14:43:51 <edleafe> #link Add traits to get RPs with shared https://review.openstack.org/478464/
14:43:54 <edleafe> #link Allow _set_allocations to delete allocations https://review.openstack.org/#/c/501051/
14:43:57 <edleafe> #link WIP - POST /allocations for >1 consumer https://review.openstack.org/#/c/500073/
14:44:00 <edleafe> #link Use ksa adapter for placement https://review.openstack.org/#/c/492247/
14:44:03 <edleafe> #link Migration allocation fixes https://review.openstack.org/#/q/topic:bp/migration-allocations+status:open
14:44:06 <edleafe> #link Add Alternate Hosts https://review.openstack.org/#/c/486215/
14:44:09 <edleafe> #link Add Selection objects https://review.openstack.org/#/c/499239/
14:44:12 <edleafe> #link Return Selection objects from the scheduler driver https://review.openstack.org/#/c/495854/
14:44:24 <jaypipes> well, since it's spec sprint today, should we be primarily focused on those?
14:44:53 <mriedem> jaypipes: tomorrow is the spec review sprint
14:44:59 <jaypipes> oh...
14:45:04 <mriedem> posted late to the ML this morning, sorry
14:45:09 * jaypipes steps back, suitably reprimanded.
14:45:17 <mriedem> feel free to review specs all day :)
14:47:50 <edleafe> No interest in discussing any of these reviews?
14:48:15 <edleafe> Moving on then...
14:48:15 <edleafe> #topic Bugs
14:48:17 <edleafe> #link placement server needs to retry allocations, server-side https://bugs.launchpad.net/nova/+bug/1719933
14:48:18 <openstack> Launchpad bug 1719933 in OpenStack Compute (nova) "placement server needs to retry allocations, server-side" [Medium,Triaged]
14:48:43 <edleafe> Just the one new one that arose from mriedem's tests with large numbers of builds
14:49:05 <mriedem> which i'm trying to recreate here https://review.openstack.org/#/c/507918/
14:49:43 <jaypipes> edleafe: that TODO is not really relevant any more.
14:50:01 <edleafe> jaypipes: why?
14:50:12 <jaypipes> edleafe: we now *do* attempt 3 retries when claiming resources.
14:50:21 <mriedem> jaypipes: client side,
14:50:24 <mriedem> and we've seen that failing
14:50:25 <jaypipes> edleafe: handling automatically the concurrent update detected.
14:50:27 <mriedem> hence the bug
14:50:38 <jaypipes> ah, sorry...
14:50:52 * jaypipes steps back, suitably reprimanded.
14:50:56 <mriedem> dansmith and i were doing large(r) scale multi-create testing last week
14:51:10 <mriedem> if you try to create >100 instances at once, you start hitting the limits of that client side retry
14:51:27 <jaypipes> mriedem: gotcha.
14:51:31 <mriedem> https://review.openstack.org/#/c/507918/ is trying to recreate in devstack so we can get logs to see where the conflict is coming from,
14:51:39 <mriedem> because it's using the fake driver which shouldn't be updating inventory
14:51:39 <jaypipes> mriedem: if OK with you, I'd like to take that bug then.
14:51:41 * bauzas is bacvk
14:51:43 <mriedem> go nuts
14:51:55 <jaypipes> mriedem: since I have a series that has cleaned up DB stuff in the resource_provider.py module
14:53:03 <edleafe> Well, we have 7 minutes left for:
14:53:06 <edleafe> #topic Open Discussion
14:53:17 <edleafe> Anything that we haven't covered?
14:53:17 <mriedem> i have another bug fix
14:53:21 <mriedem> https://review.openstack.org/#/c/507687/
14:53:23 <edleafe> #undo
14:53:24 <openstack> Removing item from minutes: #link https://review.openstack.org/#/c/507687/
14:53:25 <mriedem> ^ needs to go to pike
14:53:39 <mriedem> fixes 2 bugs
14:53:56 * edleafe is curious that #undo doesn't undo topics
14:54:51 <edleafe> #link Remove dest node allocations during live migration rollback https://review.openstack.org/#/c/507687/
14:55:25 <edleafe> Anything else to discuss?
14:56:03 <edleafe> OK, thanks everyone!
14:56:05 <edleafe> #endmeeting