Earlier  
Posted Nick Remark
#openstack-nova - 2018-07-03
19:19:05 karimull efried: failing at the same place...looks like libvirt (mock?) is not setup correctly for this test case... compared to other test cases similar to this one ..
19:19:15 jmlowe Basically all you need to do is give the name of an erasure coded pool when you do create or clone operations. I saw some soft of inferred pool name for cloning back into glance and I have no idea how to infer the data pool option.
19:19:33 mriedem dansmith: see what you think about what yikun is doing here during this refactor https://review.openstack.org/#/c/579113/
19:19:55 jmlowe Any ideas or am I just talking nonsense here?
19:20:04 efried karimull: Oh, yeah, I missed that, you're using SmallFakeDriver, you should be using FakeLibvirtDriver.
19:20:06 cdent efried: for now, yes
19:20:07 mriedem i understand why he's removing the ability to save changes to InstanceGroup.policies (it saves him some work for a thing that isn't used today and isn't supported anyway), but still
19:20:38 openstack Launchpad bug 1779931 in OpenStack Compute (nova) "Provider update race between host aggregate sync and resource tracker" [Undecided,New]
19:20:38 efried mriedem, dansmith: FYI https://bugs.launchpad.net/nova/+bug/1779931
19:21:42 karimull efried:good catch..let me try with FakeLibvirtDriver..thanks ..
19:22:26 mriedem jmlowe: no idea, jbernard or mnaser or cfriesen might have ideas
19:22:46 openstackgerrit Andreas Karis proposed openstack/nova master: Add documentation for emulator threads policy https://review.openstack.org/579956
19:23:03 efried karimull: Actually, I don't think you want to use FakeLibvirtDriver, sorry.
19:23:32 efried I think you want to use the *actual* LibvirtDriver, with the FakeLibvirtFixture.
19:24:01 efried karimull: I think you should rebase on top of 579201 and set up your test case like https://review.openstack.org/#/c/579201/1/nova/tests/functional/libvirt/test_report_cpu_traits.py
19:24:12 dansmith mriedem: what does the uselist=False do on the join?
19:24:24 openstackgerrit Andreas Karis proposed openstack/nova master: Add documentation for emulator threads policy https://review.openstack.org/579956
19:24:45 mriedem dansmith: i think enforces the one-to-one mapping
19:24:50 karimull efried:sure will do that..
19:24:54 efried dansmith, mriedem: Can I please get your eyes on https://review.openstack.org/#/c/579201/ ?
19:24:58 openstackgerrit Andreas Karis proposed openstack/nova master: Add documentation for emulator threads policy https://review.openstack.org/579956
19:25:07 dansmith mriedem: what happens if there's actually one to many in the DB?
19:25:47 mriedem dansmith: http://docs.sqlalchemy.org/en/latest/orm/relationship_api.html#sqlalchemy.orm.relationship.params.uselist
19:26:29 dansmith mriedem: that doesn't really tell me
19:26:36 mriedem http://docs.sqlalchemy.org/en/latest/orm/basic_relationships.html#relationships-one-to-one
19:26:48 mriedem that was the answer to your 'what does uselist=False do'
19:26:54 dansmith unless.. is there a unique constraint on policy now?
19:27:11 dansmith nope
19:27:23 mriedem no
19:27:55 dansmith I just don't see anything about what happens if there are more than one in the DB currently
19:28:06 mriedem would have to have a unique constraint in instance_group_policy over group_id and policy columns
19:28:12 dansmith because the old code _will_ let you set multiple policies at the db/object level right?
19:28:15 mriedem but even then that doesn't fix the uselist worry
19:28:24 mriedem dansmith: yes
19:28:37 mriedem even though that's not possible from our rest api
19:28:40 dansmith seems like it would be safer to just keep the list, and pick off the first one
19:28:41 dansmith right
19:29:48 mriedem i think he's trying to avoid having to make this work with the new policy field in the model https://review.openstack.org/#/c/579113/4/nova/objects/instance_group.py@a72
19:30:52 mriedem he also needs to add the policy field to the InstanceGroup model in the next change in the series (the one we told him to split this out of)
19:31:02 mriedem because that's the backref to the InstanceGroupPolicy table
19:31:09 dansmith you mean he needs to add it to this one right?
19:31:51 mriedem sorry yeah he's replacing _policies with policy
19:33:10 mriedem i'm ok with his db model changes, i just wasn't sure about dropping support for saving InstanceGroup.policies in InstanceGroup.save() w/o a version bump - even though nothing does that today
19:33:50 mnaser jmlowe: isnt an erasure coded pool just a normal ceph pool?
19:33:58 mriedem i'd also rather not continue humping around the 1:M data model that is really 1:1
19:34:11 dansmith I guess I'm confused here
19:36:32 efried cdent: This one: https://github.com/openstack/nova/blob/master/nova/conductor/tasks/migrate.py#L56
19:36:55 cdent efried: that's the one
19:37:01 mriedem we don't hit that on rebuild
19:37:03 mriedem fellas
19:37:43 cdent mriedem: do we have a conceptually similar thing?
19:38:06 mriedem no, rebuild is on the same host, no new resource allocations / claims
19:38:12 mriedem no new taxes
19:38:16 mriedem read my lips
19:38:41 mriedem zzzeek: what happens with an orm.relationship(..., uselist=False) if the backref has >1 related record? kablammo? https://review.openstack.org/#/c/579113/4/nova/db/sqlalchemy/api_models.py
19:38:54 cdent in my recent experience, there will always be new taxes^wmethods every time we do a placement thing
19:39:48 cdent mriedem: is there never a doubling of resource use during a rebuild?
19:39:53 cdent what magic is this?
19:39:57 mriedem no
19:39:59 dansmith cdent: rebuild is in place
19:40:30 mriedem rebuild = destroy the guest and re-spawn it with optionally a new image, but re-use the same volumes and ports and uuid and all that jazz
19:40:42 zzzeek mriedem: that will raise yse
19:40:58 zzzeek mriedem: means you did a mistake
19:41:11 cdent thanks dansmith, mriedem
19:41:15 mriedem cdent: don't forget unshelve
19:41:21 mriedem oh you said shelf
19:41:21 cdent see!?
19:41:40 zzzeek mriedem: e.g. one-to-one is not always simple to enforce needs to have correct unique constraints to make sure u dont blow it
19:41:41 cdent efried: so what's going on with your thing?
19:41:47 mriedem dansmith: so are you worried that we're going to blow up someone's db if they have >1 policy record per group https://review.openstack.org/#/c/579113/4/nova/db/sqlalchemy/api_models.py ?
19:41:55 efried cdent: I have several things.
19:42:20 zzzeek mriedem: oh the backref....umm, backref is an independent relationship. depends on if it has uselist=False or not
19:42:50 dansmith mriedem: I think if we don't have the schema to protect us, and we've had interfaces to let us create those, we might as well just do the safe thing and use list[0] everywhere, until we can check for and enforce 1:1
19:43:06 mriedem zzzeek: tl;dr is we have 2 tables in a 1:1 relationship from a REST API standpoint, but modeled them as 1:M years ago for whatever reason, idk - future requirements or something
19:43:06 dansmith mriedem: just like that instance uuid dupe thing.. shouldn't have ever happened, but there were dupes in people's dbs
19:43:13 mriedem this change is trying to reconcile the 1:1ness
19:43:16 zzzeek mriedem: yeah i am looking
19:43:51 mriedem dansmith: instance uuid dupe thing? you mean when i added that unique constraint?
19:43:58 mriedem those were the days....
19:44:00 dansmith yeah
19:44:07 dansmith and remember, we added a check, cleanup, and then the enforce patch
19:45:13 zzzeek mriedem: commented
19:45:21 zzzeek mriedem: w/ workaroun
19:45:22 mriedem ok i'm not sure what that does for the rest of his series and the object interaction, maybe his policies property on the InstanceGroup db model just needs to change to return the backref list rather than a list of policy names
19:45:29 mriedem and the object code would have to handle that accordingly
19:45:44 openstackgerrit Andreas Karis proposed openstack/nova master: Add documentation for emulator threads policy https://review.openstack.org/579956
19:45:46 dansmith just always use _policies[0]
19:45:57 dansmith he already has that on create
19:46:07 dansmith and an @property on the model for policy makes sense
19:46:39 mriedem yeah i think you and zzzeek said the same thing :)
19:46:41 mriedem works for me
19:46:47 dansmith okay
19:50:03 mriedem dansmith: i left a summary comment, can you confirm?
19:51:30 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Tighten up ReportClient use of generation https://review.openstack.org/556669
19:51:49 efried mriedem, cdent, dansmith, jroll, sambetts|afk: Let's try ^ that
19:52:28 efried There wound up being no reasonable way to do the retry in its own patch - because we weren't raising the exception to retry on.
19:54:30 jmlowe mnaser: an erasure coded pool is normal'ish, cannot do OMAP so it cannot hold the metadata of an rbd device, only need to reference an erasure coded pool for rbd during create or clone operations
19:56:41 jmlowe my concern is this, you use an erasure coded pool for glance, everything works ok until you go to clone back into glance during image-create then you need to know what pool to put the data in, currently infers the meta data pool just fine
19:57:24 mriedem efried: questions in https://review.openstack.org/#/c/579201/
19:58:07 jmlowe if you don't specify a data pool all the image data will go back into the metadata pool which may not be sized properly and will probably be a nasty surprise for the operator

Earlier   Later