| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-24 | |||
| 17:19:29 | cdent | is "broken if we want a cache" or "broken"? | |
| 17:19:51 | cdent | What I mean is, if later we decided "screw it, let's strip out the cache", would that be a thing? | |
| 17:20:06 | cdent | (just trying to make sure I'm grokking) | |
| 17:20:15 | mriedem | alex_xu: some comments in https://review.openstack.org/#/c/536085/ which can be addressed in the follow up nit cleanup patch - and also a request for a functional test, which would be done in a follow up patch | |
| 17:20:58 | efried | cdent If we get rid of the cache, and never DELETE allocations/inventories unless we know we're the sole owner thereof, then I think we would be okay. | |
| 17:21:47 | efried | cdent I don't think the placement API is broken. I just think it has some holes for consumers wishing to do good caching. | |
| 17:22:01 | efried | cdent And certainly some holes in documentation. | |
| 17:22:20 | cdent | for allocations that's (sole owner thereof, when deleting by consumer id) supposed to be true, but for inventories there's probably work to do | |
| 17:22:58 | mriedem | "And certainly some holes in documentation." - i will say, all things aside to this discussion, the placement stuff has a lot of good documentation compared to what we've historically had for stuff in nova | |
| 17:23:05 | cdent | From my perspective placement wasn't designed for caching. It was designed to be capable of tell you the truth, right now. | |
| 17:23:37 | cdent | efried: if it can't do that fast enough, that's the bug | |
| 17:23:56 | cdent | fast -> effectively, efficiently, low latency, reliably | |
| 17:24:03 | efried | cdent I continue to be vehemently not on board with the theory that it's okay to do lots of calls to placement if that can be avoided. | |
| 17:24:33 | cdent | we can agree to disagree on that and I respect your opinion. | |
| 17:24:34 | efried | cdent Because I don't care how fast we get it, it's still a "call over the wire" (even if localhost). | |
| 17:24:56 | efried | ...which is always going to be an order of magnitude higher overhead than a local cache index. | |
| 17:25:06 | cdent | efried: if you want to avoid calls over the wire I'd suggest looking at RPC :) | |
| 17:25:12 | cdent | which is ripe for adjustments | |
| 17:25:14 | efried | Even RPC | |
| 17:25:20 | efried | oh, that's not what you meant. | |
| 17:25:26 | edleafe | and what good is a local cache if you aren't sure it's current? | |
| 17:25:48 | efried | edleafe Or more precisely, you can't detect reliably when it's not. | |
| 17:26:07 | edleafe | that's what the 409 handling is about | |
| 17:26:08 | efried | which is what started me off on this whole thing. | |
| 17:26:08 | cdent | efried: we can (and perhaps should) put a placement web service (with a memcached) near the compute nodes | |
| 17:26:28 | efried | I don't know what that means. | |
| 17:26:35 | cdent | have hundreds of them | |
| 17:26:43 | efried | But it's still ultimately socket(), nah? | |
| 17:27:12 | efried | Hell, even if that socket() is to a fifo, it's still an order of magnitude heavier than talking to the cache. | |
| 17:27:25 | cdent | sure, but a) that's such the example of early optimization, b) if talking to placement turns out to be the consistent high expense in spawning a server, I'd be surprised. | |
| 17:27:35 | cdent | Since we don't know that b is or is not a problem | |
| 17:27:36 | cdent | it is | |
| 17:27:37 | cdent | too soon | |
| 17:27:42 | cdent | to be optimizig | |
| 17:27:50 | efried | yet here we are. | |
| 17:27:57 | cdent | indeed, wtf? | |
| 17:28:06 | edleafe | I was just typing the same thing - optimizing when we don't know that that's the problem | |
| 17:28:17 | cdent | but the great thing is: it mostly works, we have a temporary workaround, and we can keep on improving stuff | |
| 17:28:24 | efried | so now on the table is a third option, even broader. | |
| 17:28:28 | efried | for the long term | |
| 17:28:38 | efried | which is: rip out the cache | |
| 17:30:41 | cdent | efried, edleafe: so fairly productive conversation to some extent; more pages shared | |
| 17:30:50 | mriedem | this reminds me, we're GETing aggregates twice per update_available_resource periodic for 0 reason :) | |
| 17:30:57 | cdent | yes _that_ is a bug | |
| 17:31:08 | cdent | but I think efried fixes it in ProviderTree? | |
| 17:31:27 | mriedem | i was going to push a simple backportable patch to remove that unnecessary callin | |
| 17:31:45 | efried | I'm actually not sure any of the stuff I've done touches the legacy update_available_resource code paths. | |
| 17:31:48 | mriedem | like just remove the shit, and leave a comment saying "revert git hash xyz once aggregates are a thing the client cares about" | |
| 17:32:10 | mriedem | the ever growing todo list | |
| 17:32:15 | cdent | mriedem: is it enough of a concern to do anything? I seem to recall someone reporting it as a problem? | |
| 17:32:37 | cdent | oh, I get you, take it all out | |
| 17:32:38 | mriedem | klindgren__ at godaddy was just noticing the number of placement REST API calls in a single periodic run w/o no changes otherwise | |
| 17:32:54 | efried | mriedem If aggregates were only useful for sharing providers, maybe. Also, it won't be as simple as "revert this commit". That's gonna be merge conflict central on the patch series in flight right now. | |
| 17:32:55 | mriedem | i assume to start planning for the scale reqiurements | |
| 17:33:13 | mriedem | efried: i realize, but it would be a breadcrumb to look at what existed before, | |
| 17:33:27 | mriedem | idk, could just be simpler / better to remove it all and when needed, add the stuff back in fresh as needed | |
| 17:33:51 | cdent | I gotta go before getting sucked into another thing, I feel like ass. | |
| 17:33:56 | efried | mriedem On what time frame? Before FF? Or between now and when we cut Q? | |
| 17:34:16 | mriedem | efried: i should be backported, so whenever | |
| 17:34:36 | mriedem | *it | |
| 17:34:38 | mriedem | o/ | |
| 17:34:47 | efried | Bye cdent, thanks for the talk. | |
| 17:35:25 | efried | mriedem So that backport is gonna be very different for Q and pre-Q, just sayin. | |
| 17:35:36 | mriedem | melwitt: want to hit this cleanup patch and the docs one after it? https://review.openstack.org/#/c/536871/ - i've got the osc-placement 1.0.0 release dependent on this series | |
| 17:35:48 | mriedem | efried: because the provider tree stuff changed everything? | |
| 17:35:59 | melwitt | mriedem: sure thing | |
| 17:36:00 | mriedem | i haven't looked at things there in 2 weeks | |
| 17:36:15 | efried | mriedem Yes, changed some things, and added lots of things. | |
| 17:36:26 | mriedem | anyway, whatever, it should be pretty straight-forward | |
| 17:36:47 | mriedem | melwitt: thanks | |
| 17:42:35 | melwitt | mriedem: will this link be working after the change merges or does it need to be corrected now? https://review.openstack.org/#/c/536858/4/releasenotes/notes/commands-v1.0.0-894ea659825b3757.yaml@36 | |
| 17:43:04 | mriedem | melwitt: it'll work as a result of this change | |
| 17:43:07 | mriedem | and once the docs get published | |
| 17:43:21 | melwitt | k | |
| 17:44:47 | mriedem | bauzas: what do you need from me wrt the libvirt gpu series? | |
| 17:47:40 | dansmith | mriedem: : | |
| 17:47:46 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid suspending guest with attached vGPUs https://review.openstack.org/535693 | |
| 17:47:55 | dansmith | mriedem: I just fixed up all the things we commented on | |
| 17:47:59 | mriedem | ack | |
| 17:48:01 | mriedem | reviewing | |
| 17:48:22 | dansmith | mriedem: I can try to find someone else to be the second +2 if my hands are too dirty.. I'll let you make that call | |
| 17:54:17 | openstackgerrit | Dan Smith proposed openstack/nova master: Avoid suspending guest with attached vGPUs https://review.openstack.org/535693 | |
| 17:54:19 | dansmith | mriedem: sorry, I forgot to replace one line in the test ^ | |
| 17:59:46 | dansmith | lbragstad: hey, mriedem told me you fixed the copious warnification about policy deprecativity recently | |
| 17:59:58 | dansmith | but I still experience said pain | |
| 18:00:22 | dansmith | is there something we're doing that blocks us from experiencing intended test euphoria? | |
| 18:01:09 | lbragstad | dansmith: oh - really? | |
| 18:01:20 | dansmith | lbragstad: yah | |
| 18:01:28 | lbragstad | i had a few patches in flight for a couple different issues, let me check to see where they are at | |
| 18:02:40 | openstackgerrit | OpenStack Release Bot proposed openstack/os-traits master: Update reno for stable/queens https://review.openstack.org/537512 | |
| 18:02:55 | openstackgerrit | OpenStack Release Bot proposed openstack/os-vif master: Update reno for stable/queens https://review.openstack.org/537514 | |
| 18:03:42 | lbragstad | dansmith: what version of oslo.policy are you experiencing this with? | |
| 18:03:59 | dansmith | lbragstad: I just tox -r'd, but let me look | |
| 18:04:12 | dansmith | (r/s/b/add-suppo)% grep policy requirements.txt | |
| 18:04:12 | dansmith | oslo.policy>=1.30.0 # Apache-2.0 | |
| 18:04:45 | mriedem | dansmith: +2 on https://review.openstack.org/#/c/535693/ - i think you're ok to +W | |
| 18:04:50 | lbragstad | https://review.openstack.org/#/c/531497/ is the patch that should have fixed the issue you're seeing | |
| 18:04:56 | dansmith | mriedem: rr thanks | |
| 18:04:59 | lbragstad | and that *should* be in oslo.policy 1.33.1 | |
| 18:05:50 | lbragstad | which looks good here - https://github.com/openstack/requirements/blob/master/upper-constraints.txt#L446 | |