Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-24
17:06:14 cdent efried: I thought the point of the provider tree itself was to lock over that kind of operation?
17:06:26 cdent the tree can only be manipulated by one thread at a time
17:06:26 efried dansmith Meanwhile, thread B decides it's time to add some CUSTOM_BAR. He does his GET, sees some CUSTOM_FOO in there, adds his CUSTOM_BAR, and PUTs the whole shebang back. Now on the server, there's CUSTOM_FOO and CUSTOM_BAR.
17:06:40 dansmith efried: but nothing other than the thing that owns the inventory should collapse a PUT to a DELETE because it thinks it's the last thing, right?
17:07:02 dansmith efried: and the PUT in your case would have included a generation which is wrong now because the DELETE bumped it, no?
17:07:22 efried No, because the PUT happens first.
17:07:27 efried That's the point.
17:07:44 dansmith no I meant the second case
17:08:25 dansmith in the first case, the non-owner is collapsing a PUT to a DELETE which isn't legit It hink
17:08:26 cdent efried: is [t 1zzs] not true?
17:08:26 purplerbot <cdent> the tree can only be manipulated by one thread at a time [2018-01-24 17:06:26.123234] [n 1zzs]
17:08:38 dansmith cdent: it doesn't matter,
17:08:44 efried dansmith I don't disagree. But that's what the code is doing today.
17:08:46 dansmith there could be multiple trees on multiple hosts I think is his point
17:09:02 dansmith efried: I'm saying that's probably wrong, for the non-owner
17:09:06 cdent dansmith: I though we were concluding that the compute node owns the inventory
17:09:08 efried dansmith https://github.com/openstack/nova/blob/b214dfc41928d9e05199263301f8e5b23555c170/nova/scheduler/client/report.py#L982-L985
17:09:14 cdent but I guess some things might share nested bits
17:09:16 cdent which is
17:09:17 cdent awkward
17:10:13 dansmith cdent: we do, I'm just saying I think efried is talking about a more general case of multiple external actors needing to interact with one set of inventory or allocations, using generations as the locking mechanism
17:10:21 efried sean-k-mooney has been coming up with all sorts of interesting use cases where it may very well be the case that e.g. nova owns the VFs, neutron owns the VIFs, and some other thing owns the bandwidth - all on the same RP.
17:10:31 dansmith I'm only half paying attention to this conversation so maybe I should keep quiet :)
17:11:00 dansmith efried: I'm not sure I agree that such a case is legit
17:11:00 efried dansmith So far you've only been reaffirming my position, so by all means continue half paying attention.
17:11:03 cdent efried: in which case a cache that is local the compute node is going to fraught with challenges beyond the one you have identified today
17:11:12 efried dansmith Once you start disagreeing, then yeah...
17:11:16 dansmith hah
17:12:07 openstackgerrit Stephen Finucane proposed openstack/nova master: Don't filter out sibling sets with one core https://review.openstack.org/537361
17:12:07 openstackgerrit Stephen Finucane proposed openstack/nova master: trivial: Update tests to set 'NUMACell.siblings' https://review.openstack.org/537362
17:12:08 openstackgerrit Stephen Finucane proposed openstack/nova master: Ensure emulator threads are always calculated https://review.openstack.org/537363
17:12:08 openstackgerrit Stephen Finucane proposed openstack/nova master: Always pass 'NUMACell.siblings' to _pack_instance_onto_cores' https://review.openstack.org/537364
17:17:10 cdent efried: are you blocked in the immediate sense, or "merely" the medium term sense?
17:18:02 efried cdent I'm not blocked, other than the fact that I'm discussing and writing up this issue rather than working on the update_from_provider_tree patch.
17:18:32 efried I think we're broken long-term, but again, it's not going to matter until we have real-world situations that can do concurrent updates to the same RPs.
17:18:36 efried Which won't be Q.
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

Earlier   Later