Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-12-08
13:58:15 leakypipes fried_rice: ack. which is fine, frankly. it may seem like there's a potential for data corruption, but aside from the aggregates, there isn't.
13:58:42 cdent fried_rice: i think the lack of traction is not because it is a fundamentally bad idea, but because there’s insufficient sharing of info on what all that stuff’s doing (as I’ve noted in the rp updated pushed out just now)
14:00:58 fried_rice cdent sorry, noted where?
14:01:43 cdent fried_rice: in the nested providers section of http://lists.openstack.org/pipermail/openstack-dev/2017-December/125318.html
14:03:05 leakypipes fried_rice: cdent is referring to his weekly status email
14:03:10 leakypipes oh, jinx
14:03:54 fried_rice aha
14:04:55 leakypipes cdent, giblet, fried_rice: in any case, to just wrap this conversation up in a tidy little bow, the resource provider generation protects the resource provider's important attributes (inventories, traits, allocations, etc) against concurrent writes. It means that yes, you need to occasionally refresh that generation if you get a 409 Conflict about a concurrent write.
14:05:47 cdent leakypipes: we still haven’t done server side retry of allocations have we? reckon we should get that in sooner than later or does it matter?
14:06:50 fried_rice leakypipes At the moment the only thing that's actually doing RP updates (qua updates - not fresh creates) is inventory. And there's no concurrency issues yet because it's always 1:1 compute node to RP.
14:06:51 leakypipes but also keep in mind that that refresh/concurrent update on the provider generation is exceedingly rare in the ProviderTree (since the compute host is what houses the ProviderTree and all operations against it are currently done while holding a semaphore in the compute manager. The scheduler's claim_resources() block is where the contention is, but it's designed for quick retries when it receives a 409 Conflict for either a concurrent update
14:06:52 leakypipes or a capacity exceeded due to a concurrent claim
14:07:17 leakypipes fried_rice: claim_resources() in the scheduler is the only hot-spot currently.
14:07:23 leakypipes fried_rice: and that is by design.
14:07:50 fried_rice wait, claims don't update a RP generation, do they??
14:07:54 leakypipes cdent: nope, only client-side (in the reportclient.claim_resources(*)
14:08:01 leakypipes fried_rice: they absolutely do.
14:08:21 fried_rice oh, dang, then we definitely need to do more than we're doing.
14:08:56 leakypipes fried_rice: https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2098
14:09:14 cdent leakypipes: Is that “no we shouldn’t bother” or “ack, I agree we aren’t”
14:09:46 figleaf mriedem: heh, you are already rechecking the alternate host series
14:10:01 leakypipes cdent: on the server-side retry thing? that was a "no, we just haven't gotten to that yet and I'm not too worried because we have client-side retries"
14:10:13 leakypipes cdent: i.e. the server-side would be a welcome optimization but isn't unsafe
14:10:15 figleaf mriedem: I had them open, and didn't see your update until I rechecked 'em
14:10:25 cdent ✔
14:10:44 mriedem i have rechecked all things
14:10:57 figleaf mriedem: cool, thanks
14:11:22 leakypipes diga: whatup? :)
14:11:27 diga leakypipes: Hi
14:11:52 leakypipes diga: the thing ew've been discussing -- server-side retries of the claim_resources() logic -- is something I think you might be interested in contributing.
14:12:34 leakypipes diga: it all stems from the following TODO left in the code:
14:12:35 leakypipes https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2148-L2152
14:12:50 diga leakypipes: yep, this can be gr8 opportunity for me to pitch in :)
14:12:53 diga leakypipes: let me go through this
14:13:46 diga leakypipes: How can I test this first
14:13:54 leakypipes diga: currently, we are doing a retry of the "claim resources" transaction from the client side (i.e. in the scheduler). It would be an optimization to be able to retry the transaction from the placement server side (thus saving the round-trip HTTP call)
14:15:07 fried_rice diga The real life scenario would be to have two separate threads doing claims, and make sure they do them in a certain sequence.
14:15:41 openstackgerrit Surya Seetharaman proposed openstack/nova master: update_cell allows more than once cell to have the same db/transport url https://review.openstack.org/518556
14:15:42 fried_rice diga For test purposes, you could simulate it by cloning the local RP data, doing a claim with one copy, and then doing a different claim with the second copy (which will have the old generation).
14:19:48 cdent sigh, I was wrong about deploy.deploy() being a public interface, it’s deploy.loadapp() that I was thinking of. I’ll fix things appropriately.
14:39:04 mriedem fried_rice: do we need to hold https://review.openstack.org/#/c/522112/ for https://review.openstack.org/#/c/508345/ or can yours be stacked on top?
14:40:35 mriedem diga left huh
14:40:42 mriedem was going to point them at how we deal with this client-side today https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L98
14:45:18 openstackgerrit Chris Dent proposed openstack/nova master: [placement] annotate loadapp as public interface https://review.openstack.org/526691
14:47:32 diga leakypipes: Hi
14:47:37 diga sorry got disconnected
14:48:57 diga leakypipes: I will try to test it as per your suggestion, if anything requires, will ping you
14:51:07 openstackgerrit Stephen Finucane proposed openstack/nova master: tests: fixes mock autospec usage https://review.openstack.org/447505
14:52:01 fried_rice mriedem Either way will work. Mine isn't working yet, so do the other first.
14:52:07 diga cdent: I will ping you if need help
14:52:53 mriedem fried_rice: ok - did you see anything wrong with the way they are doing the ksa options in that one?
14:53:35 fried_rice mriedem No, looks fine. Mine will need a manual rebase to accomodate, of course.
14:56:16 fried_rice mriedem I left a +1.
14:56:25 mriedem thanks
15:02:48 openstackgerrit Jackie Truong proposed openstack/python-novaclient master: Microversion 2.57 - Add trusted_image_certificates https://review.openstack.org/500396
15:19:01 openstackgerrit Jackie Truong proposed openstack/python-novaclient master: Microversion 2.57 - Add trusted_image_certificates https://review.openstack.org/500396
15:22:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Deprecate file injection https://review.openstack.org/522027
15:25:20 openstackgerrit Jackie Truong proposed openstack/python-novaclient master: Microversion 2.57 - Add trusted_image_certificates https://review.openstack.org/500396
15:30:14 melwitt mriedem: I utilitized the quotas follow up patch https://review.openstack.org/#/c/524234
15:31:32 mriedem ok, currently re-reviewing superdan's live migration allocations thing
15:34:00 melwitt sahid: would you be able to take a look at this review where there's a bug with disk device addresses being changed during a live migration? https://review.openstack.org/#/c/518022
15:35:15 melwitt it looks like it's a regression caused by a different bug fix from the past https://review.openstack.org/#/c/459741
15:36:24 openstackgerrit Chris Dent proposed openstack/nova master: [placement] add name to resource provider create error https://review.openstack.org/526710
15:36:55 sahid melwitt: i think i did that this morning
15:37:09 sahid is there something not clear you want to discuss?
15:37:15 melwitt mdbooth: it would be good to have your review on this change for fixing the cache value with regard to O_DIRECT support during an image conversion https://review.openstack.org/#/c/523554
15:38:03 mdbooth melwitt: I vaguely recall having looked at something similar, if not this
15:38:09 melwitt sahid: oh geez, sorry. I didn't see that you reviewed it this morning. I had it on my todo to ask you the past few days and just did it blindly. thanks
15:43:04 openstackgerrit Merged openstack/nova master: Avoid stashed connector lookup for new style detach https://review.openstack.org/526182
15:43:46 melwitt mdbooth: most of the patch is moving the supports_direct_io check to nova/utils.py so that it could be used by both virt/images and libvirt/driver. but the main part of the fix is in here https://review.openstack.org/#/c/523554/6/nova/virt/images.py
15:47:06 mriedem superdan: done
15:47:19 mriedem however, it's an easy one
15:47:30 superdan received
15:50:34 leakypipes mriedem: I'm trying to square your code comment here: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4591-L4595 with the following in the functional test case setup here: https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_host_api.py#L76-L79
15:51:12 leakypipes mriedem: the reason is I have some PoC code I'm working on locally that would allow host aggregates to be associated to compute nodes (via UUID) in addition to compute *services* by service hostname
15:51:19 cdent superdan: not sure if you saw my earlier ping on this potentially cells related bug that needs some evaluation: https://bugs.launchpad.net/nova/+bug/1736101
15:51:20 openstack Launchpad bug 1736101 in OpenStack Compute (nova) "nova placement resource_providers DBDuplicateEntry when name repeat" [Undecided,New]
15:51:51 superdan I did not
15:51:53 mriedem leakypipes: https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_host_api.py#L78 is just test data, the hostmapping.host wouldn't be a uuid
15:52:01 mriedem leakypipes: at least, not a uuid that matches the computenode.uuid
15:52:31 leakypipes mriedem: sorry, I'm not following you...
15:52:55 leakypipes mriedem: you're creating a HostMapping with the host attribute of the mapping set to the compute node's UUID.
15:53:02 superdan cdent: so, two hosts with the same name is not a valid thing, but it sounds like they only have that situation because they're moving between cells?
15:53:07 leakypipes mriedem: which is what the code comment in HostsAPI says we're not doing?
15:53:16 mriedem leakypipes: https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4591-L4595 is just saying, we're looking up the compute node (in a cell) by uuid, (microversion >=2.53), if we had the uuid stored in the host mapping (api db), then we wouldn't have to iterate the cells querying each db for a compute node with that uuid
15:53:26 mriedem we could just get the host mapping from the api db which would tell us which cell that compute node is in
15:53:37 cdent superdan: I struggled to parse it and wasn’t entirely clear on the naming constraints
15:53:55 mriedem leakypipes: https://github.com/openstack/nova/blob/master/nova/tests/functional/compute/test_host_api.py#L78 - the hostmapping.host field could be "turd.ferguson"
15:53:56 superdan cdent: the conflict is in the placement db, right?
15:54:07 cdent yes, rp names are supposed to be unique
15:54:17 superdan cdent: the rp name comes from what though?
15:54:18 leakypipes mriedem: ok. so are we adding HostMapping records for compute nodes now or are we not doing that?
15:54:19 mriedem leakypipes: the test is just using something random, which in this case is the uuid that matches the compute node in the cell - confusing the test i guess
15:54:30 leakypipes mriedem: note: compute nodes, not service hosts.
15:54:31 mriedem leakypipes: we are adding host mappings for service hosts
15:54:32 mriedem not compute nodes
15:54:53 leakypipes mriedem: ok, that's what I needed to know, thanks man
15:54:53 cdent superdan: that’s what I’m not clear on. some portion of the host? If it’s not the fqdn, then that’s a clear potential for problems
15:55:11 leakypipes mriedem: I'm writing func tests for this code I'm proposing, thus the question...
15:55:14 mriedem leakypipes: this is what creates the host mappings http://git.openstack.org/cgit/openstack/nova/tree/nova/objects/host_mapping.py#n192

Earlier   Later