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