Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-11-09
19:45:30 sean-k-mooney i think that would work in this case but its not ideal
19:47:04 sean-k-mooney mriedem: anyway thanks. i was sraching my head for the last day or so trying to parse what was going on from incomplete logs but im 98% sure this is it.
19:47:49 sean-k-mooney mriedem: have a safe trip.
20:48:18 mriedem thanks
20:58:12 dansmith mriedem: so you found a test that confirmed the behavior of that thing?
20:58:19 dansmith mriedem: that deletes the consumer?
21:02:45 mriedem DeleteConsumerIfNoAllocsTestCase is the functional test that covers that case,
21:02:50 mriedem and it looks like a correct test to me,
21:03:02 mriedem creates 2 consumers each with 2 allocations on different resource classes,
21:03:09 mriedem clears the allocations for one of them and asserts the consumer is gone
21:03:29 mriedem i think we're just hitting a race with the shelve offloaded status change before we cleanup the allocations
21:03:40 mriedem but i've posted a couple of patches to add debug logs to help determine if that's the case
21:03:51 mriedem https://review.openstack.org/617016
21:04:55 dansmith okay I'm not sure how we could race and see no allocations but a consumer and get that generation conflict
21:05:13 dansmith it'd be one thing if we thought the consumer was there and then disappeared out from under us
21:17:11 mriedem during unshelve the scheduler does see allocations
21:17:31 mriedem and it thinks we're doing a move
21:17:56 dansmith okay I thought you pasted a line showing that there was only one allocation going back to placement
21:18:07 mriedem there are 3 PUTs for allocations
21:18:11 mriedem 1. create the server - initial
21:18:23 mriedem 2. shelve offload - wipe the allocations to {} - which should delete the consumer
21:18:33 mriedem 3. unshelve - scheduler claims resources with the wrong consumer generation
21:18:45 mriedem and when 3 happens, the scheduler gets allocations for hte consumer and they are there,
21:18:47 dansmith ...right
21:18:55 mriedem so it uses the consumer generation (1) from those allocations
21:19:03 mriedem then i think what happens is,
21:19:05 dansmith oh, so it passes generation=1 instead of generation=0, meaning new consumer?
21:19:12 mriedem placement recreates the consumer which will have generation null
21:19:15 mriedem yes
21:19:22 dansmith okay I see
21:19:46 dansmith I thought you were seeing consumer generation was null or zero or whatever in the third put, but still getting a conflict
21:19:49 dansmith but that makes sense now
21:20:03 mriedem Nov 06 19:48:37.013780 ubuntu-xenial-inap-mtl01-0000379614 nova-scheduler[12154]: WARNING nova.scheduler.client.report [None req-f266a0ff-2840-413d-9877-4500e61512f5 tempest-ServersNegativeTestJSON-477704048 tempest-ServersNegativeTestJSON-477704048] Failed to save allocation for 6665f00a-dcf1-4286-b075-d7dcd7c37487. Got HTTP 409: {"errors": [{"status": 409, "request_id": "req-c9ba6cbd-3b6e-4e5d-b550-9588be8a49d2", "code": "p
21:20:03 mriedem ment.concurrent_update", "detail": "There was a conflict when trying to complete your request.\n\n consumer generation conflict - expected null but got 1 ", "title": "Conflict"}]}
21:20:12 mriedem consumer generation conflict - expected null but got 1
21:20:20 mriedem yup - so new consumer but we're passing a generation of 1
21:20:25 mriedem from the old, now deleted consumer
21:21:07 dansmith cool
21:21:26 mriedem so,
21:21:42 dansmith I wish there was something better to communicate that, but any time we get "expected null" in that case, we should be able to re-try but as a non-move sort of thing
21:21:43 mriedem we can paper over this by deleting the allocations before marking the instance as shelved offloaded, but that's whack-a-moley
21:21:49 dansmith yeah
21:22:00 mriedem right we need to retry from claim_resources but i'm not sure what's the best way to do that
21:22:03 dansmith and like I said, I think it's not really any better, it just changes the problem
21:22:09 dansmith yeah
21:22:22 mriedem if we do retry that method, the next get for allocations will see there are none and we should be good
21:22:31 dansmith right
21:22:40 mriedem b/c we'll pass consumer_generation=None
21:22:59 mriedem i think i know what we can do
21:23:07 mriedem if we hit
21:23:07 mriedem if 'consumer generation conflict' in err['detail']:
21:23:13 mriedem we get the allocs again, and if empty,
21:23:15 mriedem we retry
21:23:17 mriedem easy peasy
21:23:22 mriedem it's a double get but meh?
21:23:29 dansmith yeah, it's just that it takes us an extra op,
21:23:34 dansmith when "expected null" should be enough
21:23:45 dansmith yeah
21:23:45 mriedem i can parse that out of the message if we want..
21:23:57 dansmith I know we can, it's just icky and unfortunate
21:23:59 mriedem with a TODO for more granular error codes later
21:24:02 dansmith like all the other cases in there
21:24:04 mriedem right
21:24:15 mriedem i haven't even started packing yet
21:24:24 mriedem laura is starting to check in on me every 30 minutes
21:24:31 mriedem "this is what i'm wearing all week! god!"
21:24:52 dansmith hah
21:25:35 mriedem plus my mother in law is here,
21:25:42 mriedem so lots of teenage angst memories coming back right now
21:25:51 mriedem the coffee and metallica doesn't hel[p
21:25:58 dansmith isn't that a good reason to pack and get out?
21:26:09 mriedem i've just holed up in my office
21:26:26 mriedem i'll crank out a patch for this bug and be off
21:45:39 openstackgerrit Jack Ding proposed openstack/nova master: Use virt.images.convert_image for qemu-img convert https://review.openstack.org/616692
21:52:35 fried_rice mriedem: Sanity check, please. The compute manager has a report client via the scheduler client, that's *not* the same as the report client the resource tracker has.
21:53:19 fried_rice which means my current SIGHUP doesn't do shit to the RT's cache
21:53:26 fried_rice I need to make the report client a singleton.
21:53:42 mriedem correct
21:53:46 mriedem we have report clients all over the place
21:53:50 mriedem api, conductor, scheduler
21:53:52 mriedem etc
21:54:14 fried_rice that's a scroo
21:54:55 fried_rice mriedem: So - make the report client a singleton (per process), or just diddle the compute manager's reset to hit the rt's reportclient instead.
21:55:59 fried_rice f, without knowing what the various ones in the compute manager are used for, is it really safe to make them a singleton?
21:56:04 mriedem the latter would be a smaller blast area
21:57:42 fried_rice I think I may have actually done this to myself, by removing that LazyLoader
21:57:50 fried_rice I suspect that guy was incidentally singleton-ing.
21:58:33 fried_rice nah, that should still have been creating separate instances per scheduler client.
22:03:56 dansmith fried_rice: yeah I thought that was making it a singleton a month ago when I was looking at it
22:04:18 dansmith a lot of stuff in nova used to be lazy loaded because.. um, terrible reasons
22:04:26 dansmith lazy loaded or pluggable
22:04:52 fried_rice dansmith: In this case it was supposedly because of a circular import. Whether that was ever really an issue, it isn't now, so I ripped it out. But having just looked, I still don't think it was making the report client a singleton. Care to confirm?
22:05:22 dansmith I think I confirmed that a month ago when I was looking into a seemingly recent memory leak
22:05:51 dansmith so I think it's fine that it's gone
22:06:07 dansmith I agree that randomly making it a singleton now should be done with care
22:06:23 dansmith but I don't really know how to convince myself that it's okay once its done, tbh
22:06:45 openstackgerrit Jack Ding proposed openstack/nova master: Use virt.images.convert_image for qemu-img convert https://review.openstack.org/616692
22:07:25 fried_rice well, using the RT's report client fixed the problem I was having. So maybe I pretend singleton was never suggested.
22:10:15 fried_rice oh, f, this is gonna break all over the place. I can't see a reason why the compute manager would possibly want or need to use separate report clients. I'd really like to put 'em together. If not making it a singleton, at least using only one of them from the compute manager.
22:10:46 fried_rice o/

Earlier   Later