Earlier  
Posted Nick Remark
#openstack-nova - 2017-10-06
17:09:43 openstackgerrit Merged openstack/nova master: Add error notification for instance.interface_attach https://review.openstack.org/506643
17:13:20 jgwentworth uh oh, I'm seeing on some gate runs of the py27 unit test job it's not running all the unit tests, only the os profiler test http://logs.openstack.org/66/509766/1/check/gate-nova-python27-ubuntu-xenial/794e1a9/testr_results.html.gz
17:13:57 jgwentworth this is bad
17:19:36 jgwentworth I think maybe it's only happening on stable
17:21:16 jgwentworth I see it on stable/pike and stable/ocata
17:22:22 jgwentworth stable/newton looks okay
17:24:25 sean-k-mooney jgwentworth: is it on master
17:24:40 jgwentworth sean-k-mooney: I'm not seeing it on master
17:25:11 sean-k-mooney jgwentworth: thats not so bad then because master should prevent anything getting backported if the unit tests fail
17:25:28 sean-k-mooney though i guess it would not catch dependecy issues
17:25:37 jgwentworth yeah, definitely not as bad as on master
17:26:18 superdan sean-k-mooney: help, but not prevent.. a backport could assert something that is true on master and not on stable and we'll think it's okay to merge
17:26:21 sean-k-mooney i guess that has something to do with the zuul v2->v3->v2 changes in the last few weeks
17:26:42 jgwentworth ronlund: I noticed on stable/pike and stable/ocata there's something wrong with our unit test jobs and they're running the os test profiler test instead of all of the unit tests ^
17:27:30 ronlund jgwentworth: sounds like an issue for mtreinish
17:28:03 ronlund did some stestr stuff get mixed up in stable?
17:28:35 sean-k-mooney jgwentworth: that should be defiend by this job spec correct https://github.com/openstack-infra/project-config/blob/master/jenkins/jobs/python-jobs.yaml#L109-L131
17:29:37 jgwentworth I dunno, I'm not familiar with how this works but that looks like probably
17:30:18 ronlund the zuulv3 jobs were defined elsewhere
17:30:21 ronlund in openstack-zuul-jobs
17:30:29 ronlund but that's for zuulv3, the zuulv2 stuff should be as before
17:30:30 openstackgerrit Matt Riedemann proposed openstack/nova master: Deprecate allowed_direct_url_schemes and nova.image.download.modules https://review.openstack.org/510195
17:30:32 ronlund but i'm no expert
17:30:52 ronlund i would be suspect of something with stestr but we shouldn't be using that in stable
17:31:49 ronlund cdent: btw i got a recreate of the scheduling 409 failure on https://review.openstack.org/#/c/507918/
17:32:01 ronlund http://logs.openstack.org/18/507918/6/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/d4f175d/logs/screen-n-sch.txt.gz#_Oct_04_18_32_08_753794
17:32:15 ronlund it doesn't have a logging patch in that run which logs which instance failed and prompted retries
17:32:24 ronlund but maybe not necessary to debug?
17:32:32 jgwentworth newton: https://review.openstack.org/#/c/509441/ ocata: https://review.openstack.org/#/c/509440/ pike: https://review.openstack.org/#/c/509439/
17:32:53 sean-k-mooney looking at https://review.openstack.org/#/c/509766/ its reporting as jenkins not zuul so i guess the python job is running as zuul v2.5 not v3
17:33:01 jgwentworth newton job is fine, ocata and pike are messed up
17:33:02 ronlund well i guess we should know because the instance uuid is logged right before it
17:33:38 jgwentworth sean-k-mooney: yeah, this is the old jenkins stuff that I'm looking at
17:35:55 jgwentworth mtreinish we need you
17:37:09 ronlund hmm and we only ever put RP inventory once - which is what i expected since it's the fake driver and inventory doesn't change
17:37:09 ronlund http://logs.openstack.org/18/507918/6/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/d4f175d/logs/screen-placement-api.txt.gz#_Oct_04_18_27_24_496283
17:37:30 jgwentworth I know that ostestr uses stestr underneath starting in a specific version. so if our ostestr version is sufficiently new, we could be getting stestr behavior
17:37:45 ronlund so what else can cause "Inventory changed while attempting to allocate: Another thread concurrently updated the data." if not updating the RP inventory?
17:37:54 ronlund leakypipes: ^ any ideas?
17:38:52 ronlund other allocations on the same resource provider at the same time i suppose
17:39:59 sean-k-mooney ronlund: should there not be a db lock on the inventory while a transaction is in flight that would prevent multiple concurent updates
17:40:39 ronlund we're not actually updating the inventory
17:40:45 ronlund besides the first time when the RP is created
17:40:58 ronlund we are making allocations in a loop in the scheduler
17:41:07 ronlund one by one, this isn't concurrent as far as i know
17:41:27 sean-k-mooney unless you have 2+ schduers doing this at the same time
17:42:53 sean-k-mooney just looking at http://logs.openstack.org/18/507918/6/check/gate-tempest-dsvm-neutron-full-ubuntu-xenial/d4f175d/logs/screen-placement-api.txt.gz#_Oct_04_18_27_24_496283 so there we are creating the inventory right?
17:43:12 sean-k-mooney or is the payload of the put an update
17:43:48 jgwentworth okay, so it's running all the tests, just showing the results of the os profiler run only, so it looks like this is just a display problem
17:44:13 jgwentworth like, it's picking up the wrong results to show in the testr_results html
17:44:38 sean-k-mooney jgwentworth: thats good because looking at https://raw.githubusercontent.com/openstack-infra/project-config/master/zuul.d/projects.yaml and the job definition everything looks correct
17:44:44 ronlund sean-k-mooney: it's only 1 scheduler
17:44:53 openstackgerrit Merged openstack/nova master: api-ref: note that project_id filter only works with all_tenants https://review.openstack.org/509650
17:45:40 ronlund sean-k-mooney: and yes, PUT /placement/resource_providers/c8d3d366-c0a0-481d-b7e7-b3e31b8b73e8/inventories is updating the inventory for the compute node resource provider with uuid c8d3d366-c0a0-481d-b7e7-b3e31b8b73e8
17:45:48 ronlund that happens when nova-compute starts up and creates the compute node
17:46:09 ronlund gotta run to get my license renewed, bbiab
17:46:13 sean-k-mooney ronlund: my point was its not safe to decorment an inventory by doing a put with the new value if you have 2 schduler that will create a race
17:46:39 ronlund sean-k-mooney: the PUT has a generatoin id in it
17:46:41 ronlund like an etag
17:47:06 ronlund and we're only doing it once anyway
17:47:14 ronlund so i don't think that's the issue
17:47:15 sean-k-mooney oh ok so if it does not match the current generation then the scecond put will fail and retry
17:47:32 ronlund the 2nd put would fail and the client would have to fetch the latest generation and update their requet
17:47:40 ronlund the server doesn't do it automatically
17:47:47 ronlund the consumer allocation is what's failing with the 409
17:47:53 ronlund there is no generation id on the consumer allocation
17:47:56 andreykurilin superdan: as expected, your fix works :)
17:48:03 jgwentworth I'm guessing it has something to do with the newer ostestr being stestr underneath and somehow it's messing up the result gathering
17:48:13 sean-k-mooney ya that workflow is fine
17:48:20 ronlund sean-k-mooney: this https://developer.openstack.org/api-ref/placement/#update-allocations
17:48:25 superdan andreykurilin: okay it won't actually work with multiple cells, but I'm polishing off the full fix
17:48:31 superdan andreykurilin: will certainly want you to test that as well
17:49:41 andreykurilin superdan: I do not have multiple cells installation, so will able to test only on regular one
17:49:47 superdan andreykurilin: yep that's fine
17:50:16 jgwentworth .tox/py27/bin/testr last --subunit vs .tox/py27/bin/stestr last --subunit
17:52:50 sean-k-mooney jgwentworth: so on master the tox config for py27 is https://github.com/openstack/nova/blob/master/tox.ini#L33 with stestr and on newton we delegate to pretty tox script
17:53:26 leakypipes bauwser, superdan, ronlund: reading back... just got back in.
17:53:33 sean-k-mooney on pike we use ostestr
17:56:14 jgwentworth sean-k-mooney: yeah, I think we're gonna need mtreinish to look at this. especially now that we know it's not urgent, it's running all the tests, just showing the wrong results in the html page
17:57:48 jgwentworth and if the run fails the unit tests, the os profiler thing won't even run, so I think in a fail case it would show the fail results
17:57:58 sean-k-mooney jgwentworth: looking at stable ocata i think we are overriding the result because we do this https://github.com/openstack/nova/blob/c2aa30b102808882c85d3d3f53d531c4510218cd/tox.ini#L28-L32
17:58:39 leakypipes bauwser, superdan: https://blueprints.launchpad.net/nova/+spec/de-orm-resource-providers
17:59:01 sean-k-mooney i think we are running all the test first then running the osprofiler tests
17:59:31 jgwentworth sean-k-mooney: yes, it's been like that for a long time. it's like that on master too. it's just I don't know how the results are picked out of that
17:59:51 jgwentworth out of the fact that we have two separate runs I mean
17:59:51 leakypipes finucannot: we are indeed going to be transferring these objects over the wire in short order. Probably good to get the obj_make_compatible() stuff done sooner or later.
18:00:12 sean-k-mooney yes but on master we are not using pretty tox anymore we are using stestr
18:01:27 leakypipes cdent: sorry, I had to leave before answering your question. yes, I'd like to add the nested resource providers series on to the end of that de-orm series because it makes handling superdan's request to make root_provider_uuid and parent_provider_uuid into root_provider_id and parent_provider_id.
18:01:47 leakypipes ronlund: ok, now looking into your issue
18:04:33 leakypipes ronlund: technically, any change to either inventory or traits of a resource provider would cause that concurrent update error. that said, I don't believe we are yet setting traits on a resource provider (other than in functional DB tests...)
18:04:50 sean-k-mooney jgwentworth: basically my assertion is that ostestr(pike) and python setup.py testr(ocata) probably overrite the results stestr(master) is appending?
18:06:18 leakypipes ronlund: grasping at straws, but maybe cdent's patch that reduced the number of times we update aggregates might be playing into this.. cdent, does updating aggs change the generation? /me goes to check
18:06:24 jgwentworth sean-k-mooney: maybe. I know almost nothing about how the unit test jobs work so I couldn't tell you :)
18:07:14 sean-k-mooney jgwentworth: https://github.com/openstack/nova/blob/353db2d1932965b6502e002b8be510440ff529c0/tox.ini#L33 yes just checked stestr docs thats what the --combine does http://stestr.readthedocs.io/en/latest/MANUAL.html#combining-test-results
18:08:03 sean-k-mooney jgwentworth: on newton we doe not run os profiler which is why that works
18:08:10 jgwentworth ahhh, nice sleuthing
18:09:59 leakypipes ronlund: that's a negative on the set aggregates changing the resource provider generation.
18:11:06 openstackgerrit Dan Smith proposed openstack/nova master: Always put 'uuid' into sort_keys for stable instance lists https://review.openstack.org/510140
18:11:07 openstackgerrit Dan Smith proposed openstack/nova master: Fix instance_get_by_sort_filters() for multiple sort keys https://review.openstack.org/510203
18:11:12 superdan ronlund: andreykurilin ^

Earlier   Later