| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-11-21 | |||
| 14:50:38 | stephenfin | Depends on how quickly I work through the backlog, heh | |
| 14:50:56 | moshele | stephenfin: :) | |
| 14:51:03 | openstackgerrit | Balazs Gibizer proposed openstack/nova stable/pike: Document the real behavior of notify_on_state_change https://review.openstack.org/521870 | |
| 14:51:52 | openstackgerrit | Dan Smith proposed openstack/nova master: Alternate fix for validating image on rebuild https://review.openstack.org/521186 | |
| 14:51:53 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix doubling allocations on rebuild https://review.openstack.org/521662 | |
| 14:51:53 | openstackgerrit | Dan Smith proposed openstack/nova master: Add regression test for rebuild with new image doubling allocations https://review.openstack.org/521153 | |
| 14:52:15 | dansmith | mriedem: you wanna write the reno for ^? I'm struggling to formulate english sentences at the moment | |
| 14:52:22 | bauzas | mriedem: ralonsoh: stephenfin: https://review.openstack.org/#/c/521869/ | |
| 14:52:34 | bauzas | mriedem: need your lovely +1 | |
| 14:53:01 | mriedem | dansmith: sure | |
| 14:53:26 | stephenfin | bauzas: Do we want to merge the requirements updates first | |
| 14:53:30 | stephenfin | https://review.openstack.org/#/q/project:openstack/os-traits+status:open | |
| 14:53:30 | stephenfin | ? | |
| 14:53:44 | stephenfin | The rest are WIP | |
| 14:53:46 | bauzas | stephenfin: I looked at those | |
| 14:53:53 | openstackgerrit | Balazs Gibizer proposed openstack/nova stable/ocata: Document the real behavior of notify_on_state_change https://review.openstack.org/521871 | |
| 14:54:03 | bauzas | stephenfin: and honestly, I don't see why we need to block a new release because of those | |
| 14:54:14 | stephenfin | Sweet. Then you have your +1 | |
| 14:54:25 | bauzas | stephenfin: most of cores will be on vacations by the next days | |
| 14:54:51 | bauzas | so if we wanna merge a new release and modify g-r, then it needs to be soon | |
| 14:56:04 | bauzas | mriedem: FWIW, I'm not sure we need to bump g-r given nova reqs.txt is os-traits>=0.3.2 # Apache-2.0 | |
| 14:56:08 | bauzas | you okay with that ? | |
| 14:56:25 | bauzas | or do you think it would be better to just amend g-r ? | |
| 14:56:52 | mriedem | if we're doing a release for new code in os-traits to use in nova, then we need to bump the minimum required version in g-r | |
| 14:57:06 | stephenfin | bauzas: What he said. If someone's on 0.3.2, it would break | |
| 14:57:18 | bauzas | k | |
| 14:57:24 | bauzas | I'll provide a new g-r then | |
| 14:58:03 | stephenfin | While we're here, these two should probably go in at some point too (or be abandoned) https://review.openstack.org/#/q/project:openstack/os-traits+status:open+branch:stable/pike | |
| 15:00:23 | bauzas | that's correct | |
| 15:00:35 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: placement: GET /RPs with aggs, inventories, traits https://review.openstack.org/521875 | |
| 15:00:45 | mriedem | bauzas: -1 | |
| 15:00:58 | efried | jaypipes cdent ^ Along the lines of what we discussed briefly yesterday. | |
| 15:01:04 | bauzas | mriedem: oh man, good point | |
| 15:01:16 | bauzas | mriedem: It's just a long day that I haven't provided a new release | |
| 15:01:20 | bauzas | meh | |
| 15:01:25 | mriedem | psh | |
| 15:01:25 | jaypipes | efried: k. it's gonna be low priority though :) | |
| 15:01:28 | mriedem | excuses excuses | |
| 15:01:52 | cdent | efried: noted | |
| 15:02:05 | bauzas | mriedem: no excuses, I'm a d-bag, that's it | |
| 15:02:21 | stephenfin | bauzas: Well we didn't want to say anything, but... | |
| 15:02:53 | efried | cdent jaypipes Would I be correct in assuming that we get a significant performance improvement by reducing the number of placement API calls, assuming everything else (amount of db work, etc.) remains the same? | |
| 15:03:14 | bauzas | there, fixed | |
| 15:03:19 | jaypipes | efried: for the nova-computes running Ironic virt driver, yeah. | |
| 15:03:41 | jaypipes | efried: for the KVM ones, probably won't notice much of a difference. | |
| 15:03:42 | cdent | efried: performance _where_ ? | |
| 15:04:38 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Transform missing delete notifications https://review.openstack.org/410297 | |
| 15:04:42 | efried | jaypipes cdent I'm talking about just placement calls. The above change set allows us to reduce 3N+1 calls down to one, to get the same data. (N is the number of providers in the tree.) | |
| 15:04:46 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Send soft_delete from context manager https://review.openstack.org/476459 | |
| 15:05:18 | efried | jaypipes cdent The data as a whole is identical, the work being done on the placement side is identical, etc. - just being done in one API call instead of many. | |
| 15:05:30 | cdent | efried: you’re limiting the opportunities for the placement service to parallize across multiple instances of placement itself, which may or may not be a concern. I tend to prefer lots of smaller calls on many servers | |
| 15:05:39 | jaypipes | efried: understood. and what I'm saying is that N is not a large number for KVM. For Ironic (and maybe powervm) N can be a large number. | |
| 15:05:49 | openstackgerrit | Merged openstack/os-traits master: Update the documentation urls https://review.openstack.org/501122 | |
| 15:06:16 | efried | jaypipes Ah, I understand. Nevertheless, even for a one-node tree, it would be taking 4 calls down to 1. | |
| 15:06:29 | efried | jaypipes I guess I'm asking if the overhead of the API calls is significant. | |
| 15:06:29 | jaypipes | efried: understood. :) | |
| 15:07:31 | cdent | efried: if it is possible to measure instead of speculate that would be great, but my speculation would be that http overhead is not the limiting factor in the big picture in these situations | |
| 15:08:05 | cdent | certainly as a micro optimization, at webscale™, http request overhead matters | |
| 15:08:25 | efried | cdent Okay, interesting. Well, if for no other reason than it makes the client code easier... | |
| 15:08:32 | cdent | but only if it cannot be ameliorated by parallelization | |
| 15:09:30 | cdent | my gut reaction is that your prematurely optimizing, and that your efforts would be better served by trying to break stuff | |
| 15:09:41 | cdent | so that when we fix stuff, we’re fixing the right stuff | |
| 15:09:55 | cdent | but I’m sure telling grandma how to suck eggs | |
| 15:10:12 | cdent | efried: Are you familiar with that phrase? I didn’t know it when I first heard it and it totally confused me. | |
| 15:10:39 | efried | cdent No, but google helped me out. | |
| 15:10:58 | efried | cdent In the context of the SchedulerReportClient, parallelization would entail some nontrivial rework, likely including spawing extra threads, which doesn't seem likely. | |
| 15:11:26 | cdent | no, I mean parallization at the global level, not the per “query” level | |
| 15:11:58 | cdent | we are more concerned with the performance of placement being good and predictable for everyone, at large | |
| 15:12:15 | cdent | and given N is almost always quite small... | |
| 15:13:06 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: Implement new attach Cinder flow https://review.openstack.org/330285 | |
| 15:13:07 | openstackgerrit | Ildiko Vancsa proposed openstack/nova master: libvirt: Allow multiple volume attachments https://review.openstack.org/267587 | |
| 15:13:08 | efried | cdent Though even at its smallest, we would be reducing the number of calls by 4x | |
| 15:13:35 | cdent | I hear you, but as we have no evidence, yet, that it matters, it’s early optimization, right? | |
| 15:14:01 | efried | cdent If I was doing it just for performance, yeah. But I have *hard* evidence that it's gonna make my coding easier :) | |
| 15:14:23 | cdent | while making the server more complex | |
| 15:15:00 | efried | cdent See this series: https://review.openstack.org/#/c/521605/ | |
| 15:16:28 | efried | cdent But also https://review.openstack.org/#/c/521098/4/nova/scheduler/client/report.py as we discussed yesterday. | |
| 15:17:20 | efried | Anyway, I did happen to notice a dearth of functional tests in test_report_client, so maybe I will do as you suggest and go back to breaking things. | |
| 15:18:51 | cdent | efried: I’m not against your idea at all, I just don’t think it is urgent in the face of the usual constraints about review bandwidth, getting other stuff merged in this cycle, etc etc. I’d love it if we operated on a “this is a good idea, let’s just do it” mode but we’re not quite there. | |
| 15:20:09 | efried | cdent I'm with you. Though TBH I'm not sure the amount of code will be more one way or the other. We either collate on the server side or the client side. | |
| 15:21:06 | cdent | I’m biased to doing collation on the client side because I want the server to be as simple as possible, and will consistently resist adding new stuff, until overwhelmed and then blithely acquiese | |
| 15:25:49 | openstackgerrit | Merged openstack/nova master: placement: Document request headers in api-ref https://review.openstack.org/521563 | |
| 15:26:00 | openstackgerrit | Merged openstack/nova master: placement: Document `in:` prefix for ?member_of= https://review.openstack.org/521216 | |
| 15:26:10 | artom | dansmith, mriedem, is it worth it to "argue" about which filters should be policy at this point? | |
| 15:26:34 | dansmith | artom: if you see something glaringly wrong, point it out | |
| 15:26:40 | artom | I see there are many comments to that effect already, and I haven't read all of them yet | |
| 15:27:18 | artom | dansmith, I wouldn't say glaringly. To me it's "does policy == things can change through a rebuild"? | |
| 15:27:26 | artom | Or rather, is that what we want? | |
| 15:27:51 | artom | Or are we going with the "real" semantic definition of policy, even it it means running some filters needlessly? | |
| 15:28:11 | efried | cdent Are you aware of a bug complaining that GET / of the placement API should be unauthenticated? | |
| 15:28:43 | dansmith | artom: well, as we discussed on that review, it's hard to draw a neat box around that | |
| 15:28:53 | efried | cdent AFAIK, we want every service to permit version discovery (which is what's at that URI) for free. <== mordred True? | |
| 15:29:19 | artom | dansmith, yeah, which is why I'm asking here before leaving a bunch of somments | |
| 15:29:21 | artom | *comments | |
| 15:29:21 | dansmith | artom: I want it to be "affects _where_ a thing should land" vs. "affects whether a host is "allowed" or "can" land" or something | |
| 15:29:34 | dansmith | artom: but clearly for the purposes of this backport, it's mostly just "does this break rebuild" | |
| 15:29:46 | artom | For instance, the AZ filter... the AZ can't change with a rebuild, can it? Do we need to run it? | |
| 15:30:03 | dansmith | artom: and I'm not running it on rebuild right? | |
| 15:30:18 | dansmith | oh I am | |
| 15:30:46 | dansmith | that might be a mistake.. I was thinking that certain aggregate lines could have changed such that the operator wants to block the rebuild, | |