14:00:03 <efried> #startmeeting nova-scheduler
14:00:03 <openstack> Meeting started Mon Aug 27 14:00:03 2018 UTC and is due to finish in 60 minutes.  The chair is efried. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:00:04 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
14:00:06 <openstack> The meeting name has been set to 'nova_scheduler'
14:00:10 <cdent> o/
14:00:11 <takashin> o/
14:01:10 <edleafe> \o
14:01:18 <alex_xu> o/
14:01:48 <efried> #link agenda https://wiki.openstack.org/wiki/Meetings/NovaScheduler#Agenda_for_next_meeting
14:01:51 <tetsuro> o/
14:02:07 <efried> #topic specs and review
14:02:07 <efried> #link latest pupdate: http://lists.openstack.org/pipermail/openstack-dev/2018-August/132852.html
14:02:07 <efried> cdent to pick this up again near the PTG \o/
14:02:36 <mriedem> o/
14:02:49 <efried> #link reshaper series: https://review.openstack.org/#/q/topic:bp/reshape-provider-tree+status:open
14:02:49 <efried> Half is +A (still blocked by -2 at the bottom). Let's get it merged early this week.
14:03:29 <efried> There are a few additional barnacle patches fixing nits
14:03:33 <edleafe> efried: How much of that is placement-side?
14:03:54 <efried> edleafe: There are four placement-side patches, I believe.
14:04:03 <efried> The main one, two bug fixes, and a doc fix.
14:04:11 <edleafe> So we should be good to freeze after they merge?
14:04:34 <edleafe> I'm asking because we are just about ready to do the extraction to a new repo
14:05:21 <efried> https://review.openstack.org/#/c/576927/ https://review.openstack.org/#/c/585033/ (both +A), https://review.openstack.org/#/c/596497/ and https://review.openstack.org/#/c/596494/ respectively.
14:05:59 <efried> The latter two still need reviews
14:06:36 <efried> edleafe: Got a topic for extraction later on: remind me to ask more about the freeze thing.
14:06:56 <efried> Any other questions/comments on reshaper series?
14:07:07 <edleafe> ack
14:07:28 <efried> #link Gigantor SQL split and debug logging: https://review.openstack.org/#/c/590041/
14:07:28 <efried> This is merging
14:07:52 <efried> Planning/Doing support in nova/report client for:
14:07:52 <efried> #link consumer generation handling (gibi): https://review.openstack.org/#/q/topic:consumer_gen+(status:open+OR+status:merged)
14:07:52 <efried> #link ML thread on consumer gen conflict handling: http://lists.openstack.org/pipermail/openstack-dev/2018-August/133373.html
14:07:52 <efried> #link nested and shared providers for initial & migration (and other?) allocations: https://review.openstack.org/#/q/topic:use-nested-allocation-candidates+(status:open+OR+status:merged)
14:08:18 <efried> #link Spec: Placement modeling of PCI devices ("generic device management") https://review.openstack.org/#/c/591037/
14:09:02 <efried> Anyone have other specs or reviews to bring up?
14:09:43 * cdent enqueues
14:10:00 <efried> #topic bugs
14:10:00 <efried> #link placement bugs: https://bugs.launchpad.net/nova/+bugs?field.tag=placement&orderby=-id
14:10:13 <efried> anyone have bugs to highlight?
14:10:48 <mriedem> i need to write a bug at some point,
14:10:58 <mriedem> for resize to same host doubling allocations against the same compute RP
14:11:02 <mriedem> even though it's different consumers
14:11:11 <mriedem> that's going to be a tricky one to fix most likely,
14:11:15 <mriedem> at least in a non-racy way
14:11:33 <efried> mm
14:11:53 <mriedem> b/c we swap the instance allocation to the migration record before hitting the scheduler,
14:11:58 <efried> "greatest common denominator" allocation candidate request
14:11:59 <mriedem> to find out if the scheduler has selected the same host
14:12:20 <efried> or "least superset"?
14:12:33 <mriedem> something like that yes,
14:12:39 <mriedem> 1. am i resizing on the same host
14:12:44 <mriedem> 2. figure out superset of allocations
14:12:45 <efried> but different depending on same vs different host, yah.
14:12:47 <mriedem> 3. put them back on the instance
14:13:07 <efried> can we tell whether resizing to the same host beforehand?
14:13:21 <mriedem> don't think so
14:13:29 <efried> fun times
14:13:57 <mriedem> anyway, just thinking out loud
14:13:58 <mriedem> carry on
14:14:04 <efried> Do we have a way to ask for allocation candidates limited to a specific host yet?
14:14:12 <mriedem> nope
14:14:21 <mriedem> that was part of that force_hosts bug
14:14:25 <efried> Seems like that's going to be necessary at some point.
14:14:26 <mriedem> we talked about as an option
14:14:39 <mriedem> right for the force_hosts bug, we just remove the limit to GET /allocation_candidates
14:14:53 <mriedem> but what we really want is GET /allocation_candidates?in_tree=rp_uuid
14:14:55 <mriedem> or something like that
14:15:00 <efried> Cause then not only can you solve the force_hosts thing, you can ask for resize candidates in two batches: one with the superset-difference for same host, one for fresh new-size for other hosts.
14:15:23 <efried> but in the meantime, you have to do two separate full requests and then filter them manually.
14:15:44 <efried> Lemme know when you have that bug written.
14:15:49 <mriedem> ok
14:16:09 <efried> moving on, any other bug-like stuff to talk about?
14:16:25 <efried> #topic opens
14:16:25 <efried> Extraction
14:16:25 <efried> #link extraction etherpad: https://etherpad.openstack.org/p/placement-extract-stein
14:16:25 <efried> #link extraction ML thread (when should it happen? what should the status/position of the project be?) http://lists.openstack.org/pipermail/openstack-dev/2018-August/133445.html
14:16:25 <efried> cdent/edleafe: what's the latest?
14:16:33 <efried> (I hear the etherpad may be temporarily busted?
14:16:34 <efried> )
14:16:55 <edleafe> We have a working repo
14:17:13 <edleafe> Once you apply cdent's PR
14:17:18 <edleafe> #link https://github.com/EdLeafe/placement/pull/3
14:17:39 <edleafe> IOW, tests passing
14:18:07 <mriedem> so,
14:18:12 <cdent> #link de-placement-ed nova https://review.openstack.org/#/c/596291/
14:18:23 <cdent> has been updated to use the latest pull
14:18:23 <mriedem> is the proposal to seed the repo with a big pull request rather than a series of changes in gerrit?
14:18:33 <mriedem> like,
14:18:48 <mriedem> couldn't we extract to a repo and have non-voting CI jobs until the changes that make it work?
14:18:52 <mriedem> and then make them voting?
14:19:03 <edleafe> mriedem: that would be.... tedious
14:19:13 <edleafe> Most of the changes I made are from a script
14:19:14 <dansmith> we *have* to do that I think
14:19:14 <mriedem> but it would be correctly using our code review system in openstack
14:19:33 <cdent> we're not losing any history by doing it the way the experiments have been done thus far
14:19:38 <edleafe> The plan was to get tests passing first, and then push the extracted repo
14:19:39 <cdent> we just aren't creating that history in gerrit
14:19:39 <dansmith> IMHO, we have to extract to a repo, have changes against CI jobs that use the alternate repo instead of the nova stuff,
14:19:45 <dansmith> and then delete the nova stuff once we're not using it anymore
14:19:57 <edleafe> dansmith: that's exactly what this is
14:20:33 <mriedem> i'm confused
14:20:38 <mriedem> https://github.com/EdLeafe/placement/pull/3/commits/a7805ca13a98e3a25876285c37d603aee9c65c3f is a github-only change right?
14:20:39 <dansmith> edleafe: okay I thought you were implying that making the CI jobs work against the split was "tedious"
14:21:08 <edleafe> dansmith: No, I meant doing a small change, pushing to gerrit, doing another small change, etc.
14:21:26 <mriedem> but that's totally not following our community dev policy
14:21:29 <dansmith> oh okay, yeah I don't think that can happen
14:21:29 <mriedem> which is reviews in gerrit
14:21:49 <edleafe> mriedem: there isn't a repo to review yet
14:21:54 <mriedem> it looks to me like this is (1) fork nova repo, (2) remove nova stuff (3) pull request in fixes to make placement tests pass
14:22:20 <mriedem> with the eventual plan to (4) seed the openstack/placement repo based on that repo from github
14:22:22 <edleafe> 2) and 3) are intertwined
14:22:39 <edleafe> but yeah
14:22:58 <dansmith> I don't think I understand why 2 and 3 are intertwined
14:22:58 <cdent> mriedem: yes, that's what's been discussed for a few weeks on the (now broken etherpad) and in the several emails and blog posts associated with this stuff. What is your preference?
14:23:03 <mriedem> so i guess my point is i think the core team should be involved in reviewing these changes
14:23:15 <mriedem> in gerrit
14:23:18 <mriedem> not github pull requests
14:23:33 <edleafe> dansmith: a lot of the test failures come from imports that reference the nova directory structure
14:23:46 <cdent> mriedem: we can do that if you want, but as most of it is very mechanical it seemed... wasteful?
14:24:12 <cdent> mriedem: but it sounds like you want (correct me if I'm wrong please):
14:24:12 <edleafe> Isn't that the point of having tests?
14:24:27 <cdent> 1. use the filter script
14:24:31 <cdent> 2. seed a broken repo
14:24:38 <cdent> 3. incremental review and fix things
14:24:59 <mriedem> is there more?
14:25:00 <cdent> 4. add some tests at the end
14:25:05 <cdent> eof
14:25:22 <cdent> s/add/automate/
14:25:25 <mriedem> 4) is also getting tempest-full / devstack working with the placement repo?
14:25:32 <cdent> yes
14:25:34 <mriedem> 5) drop code from nova?
14:25:51 <mriedem> that's more inline with what i was expecting yes,
14:25:55 <cdent> I was leaving 5 out of the picture entirely for now as it seemed like a "we'll visit that when we get there"
14:26:03 <mriedem> edleafe's repo was an experimental is my understanding
14:26:11 <mriedem> *experiment
14:26:23 <edleafe> mriedem: It was more like a work area that we knew would be messy
14:26:26 <cdent> so the reason we didn't 3 was because of what I said before about mechanical
14:26:32 <cdent> and messy
14:26:46 <mriedem> i think messy is ok
14:27:11 <mriedem> i don't really think "fork into ed's github repo, do stuff, voila here is a thing" is the right way to do this per community norms
14:27:21 <edleafe> mriedem: what exactly do you think a review will entail?
14:27:25 <mriedem> it goes against (1) our code review system of using gerrit and (2) our core review team
14:27:44 <cdent> I guess I wasn't really expecting that people would think of it that way, mriedem: the expectation was that people like you would come and look at ed's repo too
14:27:46 <mriedem> looking at https://github.com/EdLeafe/placement/pull/3 these are individual commits
14:27:51 <mriedem> so why wouldn't the same be pushed to gerrit?
14:28:32 <cdent> they _can_ be. But I thought we were saving some hassle by doing it "outside". Apparently not.
14:28:54 <mriedem> i was out for a week and don't read all the personal blog posts from everyone, so i'm coming in late,
14:28:58 <mriedem> but this is my 2 cents
14:29:08 <edleafe> mriedem: There are a couple dozen commits there. The initial commit I pushed has about that many steps. So are you saying that you want to commit at least 2 nova cores to review around 50 patches?
14:29:14 <mriedem> it looks like the wrinkles have been ironed out in the github repo,
14:29:25 <mriedem> and could easily be transitioned into a working repo with gerrit
14:29:53 <mriedem> look, i'm one person
14:30:22 <mriedem> i don't like the idea of doing this in github, outside gerrit and our core team, and then using it to seed the official openstack namespaced repo
14:30:49 <mriedem> but if majority rules and doesn't want to deal with the incremental changes in gerrit, whatever
14:31:04 <cdent> efried, jaypipes, alex_xu : thoughts?
14:31:07 <edleafe> I would think that the core team's time could be better spent
14:31:43 <jaypipes> sorry, I need to read back.
14:32:36 <mriedem> melwitt should probably also at least be aware of the direction here and alternatives
14:33:12 <edleafe> mriedem: just to be clear: there are no logical changes being made. Just pathing updates for the updated directory structure
14:33:31 <efried> Well, I said a lot of the same things last week, but phrased as questions, and was satisfied with the answers.
14:33:48 <edleafe> And changing some docs to only keep the placement-specific part
14:33:52 <edleafe> *pars
14:34:01 <efried> My understanding was that the commit history would still be available for anyone to look at.
14:34:11 <edleafe> efried: yep
14:35:05 <edleafe> What you won't see is the gyrations needed to restructure placement from the way it is in Nova
14:35:15 <efried> so
14:35:23 <mriedem> well we have 2 +2s for a reason, so i figure this is no different if it's openstack
14:35:47 <mriedem> should i just air out my concerns in the ML thread on this so we can move on or...?
14:35:59 <efried> what I expressed before was that being able to see, in a gerrit review, the steps from raw to working, would be useful.
14:36:24 <cdent> so, we can do it in gerrit, just the existing patches
14:36:29 <edleafe> mriedem: I haven't gotten an answer from you on the question of whether you are committing 2 nova cores to review 50 or so patches in a timely fashion?
14:37:02 <mriedem> edleafe: i can help, it's not going to be my sole focus day in and out, but i'm sure efried is also going to be on board to help with that
14:37:05 <mriedem> i can't speak for others really
14:37:08 <cdent> but in order to do that we're going to need more concerted help from the entire nova group with regard to the repo setup, understanding zuul, etc. The current style was a shortcut to step into the future without needing to recruit as many people
14:37:11 <efried> Like, as a placement core, I would like to be able to approve that initial seed patch by comparing base with PS<N> to see that e.g. 'import nova.api.openstack.placement.foo' was replaced with 'import placement.foo' but the guts of that file weren't changed.
14:37:47 <edleafe> efried: so the initial commit to openstack/placement would be the full nova code base?
14:37:53 <efried> I don't necessarily think we need to deviate much from the current process to make that happen.
14:38:12 <efried> edleafe: Nah, I think it would be adequate if it was just the selected files, but in their form as they exist in nova initially.
14:38:35 <mriedem> like i said,
14:38:44 <mriedem> non-voting test jobs until it's passing
14:38:46 <efried> i.e. I don't need to see fifty thousand file deletes between base and PS1. But I also don't want to see a couple hundred file adds with no delta context.
14:38:53 <mriedem> with the series of incremental changes to get there
14:39:04 <edleafe> I do feel like this is going to be a major time suck on the cores
14:39:15 <efried> well, I don't think we need to span multiple change sets to go from seed to working.
14:39:22 <efried> I think we should do that in one change set.
14:39:39 <edleafe> wait.. what? You want a huge patch?
14:39:48 <efried> Heck, I think we could even do it in one patch set.
14:39:49 <mriedem> that's gonna be hard to review
14:39:53 <edleafe> talk about going against the way we do things in openstack
14:39:55 <efried> um
14:40:07 <efried> sorry, maybe I've been misunderstanding how the repo would get seeded.
14:40:14 <efried> We need an initial change set with all the file adds, no?
14:40:54 <efried> it wasn't just going to... spring into existence already populated, with nobody reviewing or approving, was it?
14:40:59 <jaypipes> FTR, I agree with mriedem and would prefer to review Gerrit patches for this.
14:41:28 <edleafe> efried: a quick and dirty check showed that the single patch would be 23,000 lines
14:41:49 <edleafe> jaypipes: and will you also commit to reviewing them?
14:42:17 <jaypipes> edleafe: yes, but as I have repeatedly said on this topic, they will take lower priority to other nova items.
14:42:22 <efried> edleafe: how many files?
14:43:01 <edleafe> efried: 410
14:43:39 <edleafe> jaypipes: This feels like a huge roadblock
14:43:45 <jaypipes> I was under the impression that cdent and edleafe were experimenting with extraction automation/scripts in the GH repo and that we weren't actually going to be importing said GH repo as "the new placement".
14:44:17 <edleafe> jaypipes: the GH repo was so that we could see what each other was doing
14:44:17 <cdent> I'm a little confused. Do we have confidence that the placement tests test placement well and represent "the placement functionality"?
14:44:25 <jaypipes> edleafe: I've been 100% upfront about that since cdent originally began discussing extraction.
14:44:29 <edleafe> cdent: obviously not
14:44:43 <jaypipes> edleafe: ack. and I was fine with the GH collaboration/experimentation.
14:44:58 <jaypipes> edleafe: I just didn't realize the plan was to make that GH repo the seed for a new placement.
14:44:58 <efried> So here's what I would like to see:
14:44:58 <efried> - One change set.
14:44:59 <efried> - The initial patch set of that change should be the 410 files from nova in their raw form. I should be able to do a diff -r with nova (with whatever the flaggyflag is to ignore missing files), and get effectively no output.
14:44:59 <efried> - Then the second patch set of that same change should be the resolved thingy that edleafe and cdent have been working out in the github repo. Now I can diff base with PS2 and see e.g. file renames and changes in import lines but effectively nothing else.
14:45:11 <edleafe> ok, then I'll start working on a step-by-step patch series, starting with the results of the history-preserving extraction script
14:45:36 <mriedem> efried: i don't love that
14:45:46 <edleafe> And I agree with Jay that this will all be lower priority, and will most likely languish
14:45:48 <efried> perhaps I'm not understanding what you're suggesting, then.
14:46:01 <mriedem> "The initial patch set of that change should be the 410 files from nova in their raw form. I should be able to do a diff -r with nova (with whatever the flaggyflag is to ignore missing files), and get effectively no output."
14:46:05 <jaypipes> edleafe: it's not my priority, sorry.
14:46:06 <mriedem> cool, agree, change 1
14:46:20 <mriedem> then separate changes to make things work, as i'm seeing in this pull request in githu
14:46:22 <mriedem> *github
14:46:43 <mriedem> otherwise there isn't really a point in doing it in gerrit if it's so big i can't review it coherently
14:46:59 <cdent> I'm a little confused on whether efried and mriedem are using the same definition of terms for 'patch set' and 'change set'
14:47:03 <edleafe> mriedem: agree that huge patches suck
14:47:09 <mriedem> before anyone goes off and doing anything,
14:47:20 <cdent> I think what we are talking about is a stack of 25-50ish individual reviews
14:47:30 <mriedem> i would like to first reply on the latest extraction ML thread to clarify understanding and raise the issue
14:47:54 <efried> Yeah, see, I don't see the point in multiple *change sets* for the initial seed repo.
14:47:58 <cdent> mriedem: if you're inclined, use the thread with "(technical)" in the subject?
14:48:15 <mriedem> yes that's the one i was going to use
14:48:29 <cdent> efried: so you're disagreeing with mriedem, or no, still not clear
14:48:40 <efried> I.e. I don't get why we need to merge patently broken code initially.
14:49:04 <mriedem> to sanely review the differences to get to working
14:49:11 <mriedem> clearly https://github.com/EdLeafe/placement/pull/3 is made of individual commits
14:49:13 <mriedem> for similar reasons
14:49:35 <mriedem> not just "here is 1 patch with 50 or some logical changes"
14:50:27 <mriedem> looks like it's more around ~30 changes
14:50:41 <mriedem> given ed's 2 here https://github.com/EdLeafe/placement/commits/master
14:50:52 <edleafe> mriedem: you've also got to take into account my initial push
14:51:03 <mriedem> isn't that https://github.com/EdLeafe/placement/commit/e3173faf59bd1453c3800b2bf57c2af8cfde1697 ?
14:51:23 <edleafe> mriedem: Like I said, there were at least a couple dozen steps from the hsotory filter to that push
14:52:19 <mriedem> so you would also post those in gerrit or not?
14:52:49 <edleafe> If the idea is that you want to see what changed from the original files to the placement repo, then yes
14:53:06 <edleafe> To clarify, I don't *want* to post them to gerrit
14:53:11 <mriedem> https://github.com/EdLeafe/placement/commit/e3173faf59bd1453c3800b2bf57c2af8cfde1697 itself might be ok
14:53:13 <edleafe> I think this is a huge waste of time
14:53:18 <edleafe> But I'm being outvoted
14:53:27 <mriedem> i was thinking more https://github.com/EdLeafe/placement/commit/e3173faf59bd1453c3800b2bf57c2af8cfde1697 + https://github.com/EdLeafe/placement/commit/e984bef8587009378ea430dd1c12ca3e40a3c901 + what's in https://github.com/EdLeafe/placement/pull/3
14:53:39 <efried> Being able to "see what changed from the original files to the placement repo" is all I care about. But I don't think we get benefit from splitting it into a couple dozen change sets.
14:53:59 <edleafe> My first commit was abandoned, and I force pushed the second. Like I said, this was supposed to be a messy work area
14:54:19 <cdent> mriedem: you understand the change in pull/3 are basically the same as the changes in pull/2 except broken apart so people can have insight into what's going on, yeah?
14:54:22 <efried> IMO there's no reason for gerrit to contain "show your work" info.
14:54:34 <edleafe> efried: So you'd be able to review something like https://github.com/EdLeafe/placement/commit/e3173faf59bd1453c3800b2bf57c2af8cfde1697 ?
14:54:39 <mriedem> sure, an experimental repo to mess around and see what works is fine, and that's what i thought this was doing; i didn't expect this to eventually be "ok here we go - official placement"
14:54:42 <cdent> (the main different is that we chose to have some different paths for modules)
14:55:05 <cdent> it's still a seed
14:55:13 <cdent> because until it is gating it isn't anything
14:55:13 <mriedem> edleafe: that change is mostly file moves and deleting nova stuff right?
14:55:21 <edleafe> mriedem: we learn as we go. Once we have it figured out, then we re-run the steps and push clean to the openstack repo
14:55:41 <edleafe> mriedem: and some import pathing changes
14:55:56 <mriedem> sure that's all pretty trivial to review
14:56:18 <edleafe> that's the point - it *is* trivial. What needs to be reviewed is the final product
14:56:20 <efried> right. So no need to do it in a zillion change sets.
14:56:53 <mriedem> edleafe: so how would you expect me to review the final product? pull it down and diff it against the nova repo?
14:57:11 <mriedem> or just not review it at all and assume that if tests are passing it's good enough?
14:57:14 <edleafe> Oh, I dunno - see that the tests are all there, and that they are passing?
14:57:26 <efried> edleafe: I would be reviewing such a thing by diffing against nova, yes. And then the next delta I would be able to mostly skim, because it would comprise mostly renames and import changes, and the evidence that it's fully mechanically correct is that it passes tests.
14:57:30 <mriedem> by that logic i don't need to review any changes ever right?
14:57:30 <edleafe> Unless you don't have confidence in the tests, of course
14:57:33 <mriedem> as long as tests pass
14:57:34 <cdent> i have to leave very soon to get physio on my shoulder. jaypipes was internet contagious. it is also a holiday today for me, so i'm not really coming back today. Can people please be sure that whatever is decided today it gets reflected on the email thread?
14:57:47 <efried> yes
14:58:13 <edleafe> Yeah, I think I'll find something else to occupy myself today
14:58:15 <efried> mriedem: Skimming to assure myself that what changed was imports not content, plus tests passing, equals approval in this case, yes.
14:58:40 <mriedem> "what changed was imports not content" is what i mostly care about
14:58:44 <efried> extending that to "any change ever" is obviously spurious
14:58:50 <mriedem> and why we do reviews
14:59:16 <efried> And I don't see the point in splitting that across multiple change sets.
14:59:29 <mriedem> so with <1 minute,
14:59:31 <efried> Is that the only thing we're discussing here?
14:59:34 <mriedem> i can reply with the options to the ML thread
14:59:56 <edleafe> Be sure to include the load on the nova cores in that post
15:00:14 <efried> #endmeeting