Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-15
13:29:42 mriedem did someone report a bug for it?
13:30:19 mriedem looks like artom already opened one for the same thing https://bugs.launchpad.net/nova/+bug/1836754
13:30:19 openstack Launchpad bug 1836754 in OpenStack Compute (nova) "Conflict when deleting allocations for an instance that hasn't finished building" [Medium,Confirmed]
13:30:25 gibi mriedem: yes, that one
13:31:53 mriedem last time i looked we just need a retry in the local delete case
13:32:10 gibi mriedem: yeah, that was my understanding above as well
13:33:02 mriedem "what if the delete happens first, and then the create comes in? We would have leaked allocations."
13:33:04 mriedem yeah that is a problem
13:33:12 mriedem since the create will just re-create the consumer and allocations in placement
13:33:27 mriedem hear me out, SOFT DELETE-ABLE CONSUMERS!
13:35:06 mriedem efried: i'd think we could trap that case in conductor once the response comes from the scheduler,
13:35:42 mriedem b/c we'll check if the build request has been deleted in the interim during scheduling and if so halt the build process - at that point we could cleanup allocations (do we not already?)
13:38:56 dansmith gibi: got it
13:39:07 gibi dansmith: thanks!
13:41:25 efried mriedem: we might already, yeah.
13:42:55 mriedem gibi: efried: i might work up a functional test to recreate it since unit tests aren't going to cut it for that kind of interaction
13:43:32 efried cool
13:44:08 efried brinzhang: yt?
13:44:13 gibi mriedem: cool, sorry I'm busy today - tomorrow so I did not bite
13:44:28 mriedem np
13:59:49 dansmith mriedem: I think you oughta drop your -W on that (now) base patch and I'll +2.. I know you're going to add the test later and it sounds like you don't think it's likely to break as it is
14:09:19 mriedem dansmith: ok
14:09:38 dansmith I already +2d
14:38:27 stephenfin mriedem: Ugh, that's pip's total lack of a dependency resolution biting us in the ass. I've no idea how to fix that
14:38:35 stephenfin Is that the log from my patch or something else?
14:42:20 dansmith so, there's a cinder tempest test that is running assertEqual() and failing because an updated_at stamp isn't exactly what it expects...
14:42:35 dansmith I don't see that up on e-r
14:44:56 dansmith it's the minimum basic scenario, where it expects the post-volume-create list to exactly match a show a couple statements later, but seems like something else has touched that volume
14:45:10 dansmith mriedem: know anything about such a thing?
14:51:09 mriedem updated_at or something is different right?
14:51:16 mriedem i've seen that before, maybe a new regression
14:51:38 mriedem stephenfin: it was a log from my devstack patch to default USE_PYTHON3=True which your patch depends on
14:51:53 mriedem note i brought it up in -tc since they were talking about this upgrade testing yesterday
14:51:56 dansmith mriedem: yep.
14:53:38 mriedem dansmith: got a link to a job failure?
14:53:56 dansmith mriedem: https://365c4224c221ec730c2d-019bc8f0795daf4dab730f80e83974fa.ssl.cf1.rackcdn.com/627891/62/check/nova-next/58f7f91/testr_results.html.gz
14:53:56 stephenfin mriedem: I've a minimal reproducer here: http://paste.openstack.org/show/783995/
14:53:59 mriedem oh https://bugs.launchpad.net/tempest/+bug/1838202
14:53:59 openstack Launchpad bug 1838202 in tempest "TestMinimumBasicScenario.test_minimum_basic_scenario race fail comparing volume to expected values with updated_at diff" [Undecided,New]
14:54:00 mriedem dansmith: ^
14:54:00 openstackgerrit Merged openstack/nova master: VMware: Update flavor-related metadata on resize https://review.opendev.org/681004
14:54:08 stephenfin Stupid pip
14:54:14 dansmith mriedem: ah nice, I didn't see that on e-r
14:54:38 mriedem because it's not....let me look
14:55:35 dansmith if my logstashing is right, looks like it started around 10/7 although not sure how much history we have, that's about a week ago
14:56:13 mriedem yeah the pain in the ass is the mismatch is singleline indexing
14:56:23 dansmith yeah
14:56:24 mriedem logstash only goes back 10 days
14:56:30 mriedem i reported that bug 78 days ago
14:56:30 openstack bug 78 in Baz (deprecated) "When asking you to sign something; baz should tell you what" [Medium,Won't fix] https://launchpad.net/bugs/78
14:56:49 mriedem heh bug 666
14:56:49 openstack bug 666 in Launchpad itself "can't file a bug on Ubuntu" [Medium,Invalid] https://launchpad.net/bugs/666
14:56:49 dansmith yeah
15:05:08 mriedem it looks like the dict keys are at least sorted so i can do:
15:05:11 mriedem http://logstash.openstack.org/#dashboard/file/logstash.json?query=message%3A%5C%22actual%20%20%20%20%3D%20%7B'attachments'%3A%20%5B%5D%2C%5C%22%20AND%20tags%3A%5C%22console%5C%22&from=7d
15:06:03 dansmith hmm, that only shows two hits/
15:06:14 dansmith maybe those are the two I looked at
15:07:57 stephenfin mriedem: https://review.opendev.org/688731
15:08:31 mriedem dansmith: yeah it's on 627891
15:08:36 mriedem it's just rare
15:08:37 dansmith yeah
15:10:21 mriedem stephenfin: ack i'll make my devstack change depend on that
15:10:59 mriedem oh i lost all of my beautiful meaningless +1s......what a world what a world
15:11:15 stephenfin Cool. Looks like a fix in pip _is_ underway but it could be months/years before that lands https://pradyunsg.me/blog/2019/06/23/oss-update-1/
15:12:12 mriedem cool like what? write a dep resolver for pip?
15:12:40 mriedem fwiw i think lifeless shed many years from his life working on a dep resolver for pip....
15:13:01 mriedem before or during just saying f it and doing the constraints stuff in openstack
15:13:48 dansmith ah the good old days of openstack
15:14:26 sean-k-mooney mdbooth: is this https://review.opendev.org/#/c/663382/ the patch form stephenfin you planned to add functional test too? if not i might add one in a few days
15:20:38 sean-k-mooney stephenfin: by the way we still need to land this https://review.opendev.org/#/c/675776/ and backport it to train
15:21:42 sean-k-mooney although it looks like it has lots of other change mixed in
15:24:00 sean-k-mooney so ya we shoudl merge v5 https://review.opendev.org/#/c/675776/5 not v6 ill -1 the patch and ask for the patch to be fixed.
15:49:56 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove compute compat checks for aborting queued live migrations https://review.opendev.org/688409
17:54:20 mriedem i'm finding that writing a functional test for bug 1836754 is so hacky to order the events that it's probably not worth it and we should just throw a retry decorator on delete_allocation_for_instance when we hit a consumer generation conflict
17:54:20 openstack bug 1836754 in OpenStack Compute (nova) "Conflict when deleting allocations for an instance that hasn't finished building" [Low,Confirmed] https://launchpad.net/bugs/1836754
17:55:27 mriedem efried: also conductor deletes the allocations created by the scheduler if the server is gone by the time we get back from scheduling https://github.com/openstack/nova/blob/149327a3abb12418cdf65316e7c1d4924767bfdf/nova/conductor/manager.py#L1402
17:55:30 mriedem so that's covered
17:59:54 artom mriedem, we never hit it that often, did we?
17:59:56 artom And only in CI
18:00:00 artom So I think no func test is fine
18:00:17 artom We can use the e-r query to see if merging the commit makes the hits disappear
18:01:25 openstackgerrit Dan Smith proposed openstack/python-novaclient master: Add aggregate-cache-images command and client routines https://review.opendev.org/687141
18:15:56 mriedem artom: yeah it's very rare
18:34:35 openstackgerrit Merged openstack/nova master: Filter migrations by user_id/project_id https://review.opendev.org/674243
18:55:29 efried mriedem: are you working on the forced alloc delete rn?
19:01:19 mriedem yeah, need to write a test
19:01:56 efried mriedem: imo we should use DELETE when we're serious rather than using a retry.
19:02:08 efried and indicate seriousness explicitly from the caller
19:02:41 mriedem why was it using PUT with an empty allocations dict to begin with then?
19:03:41 efried mriedem: exactly so we can capture 409s.
19:03:50 efried point being, we only want to do that some of the time.
19:04:04 efried IIRC we used to use DELETE until I made a stink about it.
19:05:09 efried mriedem: I was about to dig in; if you want to write a test, I can take a swing at the patch.
19:05:18 efried unless you're doing both already
19:05:27 mriedem so you must have gotten stinky here https://review.opendev.org/#/c/591597/
19:06:52 efried yeah, that was a step on the path. I think I remember complaining about the fact that that wasn't really helping us, because of the tininess of the window we were leaving.
19:07:02 efried though clearly we're hitting that window, hence the bug.
19:08:21 efried mriedem: is this something you think we should backport? Because I'd like to do a series where we split the existing one into forced-or-normal, also getting rid of @safe_connect. But can do a "tactical" version for backport if that's a consideration.
19:09:00 efried artom: did you see this bug in real life or just in CI?
19:09:04 mriedem reading some of the comments on that patch it sounds like you were pushing for keeping DELETE rather than going with PUT and consumer generation handling:

Earlier   Later