Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-15
12:53:31 efried We don't care if we get a provider conflict here, but if we get a consumer conflict that's bad.
12:53:47 gibi efried: noted. I still have a TODO to make progress on that
12:54:29 efried we could solve this with a retry, but without making that ^ distinction, I'm not sure it's the right thing to do.
12:54:47 efried ...or that it's better than just using an old (generation-less) microversion to drop the allocs.
12:55:03 efried ...or using the DELETE route which IIRC doesn't do generations at all.
12:55:13 efried (because no payload).
12:55:28 efried (and we don't put that stuff in headers)
12:57:22 gibi I think we need to retry on consumer conflict as well. This bug basically means that the server delete codepatch racing with the server create codepath. The end user want's the server to be deleted so even if the create codepath updated the server allocation we need to delete the updated allocation
12:58:06 gibi so if we want to delete that allocation in every case, then we can even call DELETE without the generation checking
12:58:31 efried If we can be sure we're in the instance delete flow, I agree with you.
12:59:14 efried Meaning we can't just go hard at the report.py level; we have to {call a different method | send a specific flag} indicating we want to force it.
12:59:56 efried because in the general case if we're deleting allocations and something changes the consumer, it doesn't necessarily mean we want to proceed.
13:00:06 gibi efried: you are right
13:00:26 efried Though I'm not sure how we could race e.g. a resize and a migrate
13:00:29 gibi efried: we need to be careful and only force the delete from the server delete codepath
13:01:23 efried btw I checked the placement logs in slaweq's repro and they don't include the message for the 409, so I can't tell whether it was indeed a provider conflict.
13:08:53 gibi efried: in the placement log here is the conflict Oct 14 14:45:26.823202
13:09:36 gibi efried: around that there is multiple PUT request for consumer 5b5b12dc
13:10:26 gibi efried: nova-api doing a local delete for server 5b5b12dc
13:12:38 gibi efried: so at least slaweq's repor is a race between a server create and a server delete
13:14:37 efried mm. So we could fix this with a hard delete -- but what actually worries me is the reverse problem.
13:15:01 efried what if the delete happens first, and then the create comes in? We would have leaked allocations.
13:17:29 efried we should have some way for the delete to abort the create...
13:17:53 efried But I guess that's another problem for another day.
13:28:39 mriedem stephenfin: since you care about the py2 droppage, grenade jobs are failing on some weird package things
13:28:39 mriedem https://zuul.opendev.org/t/openstack/build/4da3c44dcbcd4ed7aa04a8dcaa19c011/log/logs/grenade.sh.txt.gz#35366
13:28:51 mriedem that's with py2 on the old (train) side and py3 on the new (ussuri) side
13:29:04 mriedem ERROR: Cannot uninstall 'PyYAML'. It is a distutils installed project and thus we cannot accurately determine which files belong to it which would lead to only a partial uninstall.
13:29:35 mriedem gibi: efried: i'm pretty sure melwitt brought up that bug the other day (the local delete conflict),
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

Earlier   Later