| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-07-03 | |||
| 17:32:19 | efried | I'm suggesting that as the fix. | |
| 17:32:36 | efried | Oh, the generation checking is in the method that's common to both the compute side and the host agg sync. | |
| 17:32:52 | efried | sorry that wasn't clear | |
| 17:33:08 | dansmith | oh set_aggregates_for_provider() | |
| 17:33:09 | dansmith | ? | |
| 17:33:37 | efried | https://review.openstack.org/#/c/556669/11/nova/scheduler/client/report.py@1250 yeah | |
| 17:33:41 | dansmith | api is using that for the mirroring, which never would fail before because it didn't check, but now is failing because it does, in the patch | |
| 17:33:57 | dansmith | okay, I got it.. I thought you were saying lots of things other than this | |
| 17:34:14 | efried | :) | |
| 17:34:23 | efried | You were too late. I already killed your hit man. | |
| 17:34:36 | dansmith | I requested NFO | |
| 17:34:42 | efried | while typing with the other hand | |
| 17:34:42 | dansmith | so that must have been jaypipes' hit man | |
| 17:35:46 | efried | dansmith: So do you agree adding a retry on both sides of this thing would be a viable solution? | |
| 17:36:14 | dansmith | well, I agree that (a) mirroring should be checking generation and (b) that it should retry if generation conflicts, so .. yes? | |
| 17:36:25 | efried | dig | |
| 17:38:28 | efried | I'll retract what I said earlier about there not being a bug, mriedem. There is, we were just hiding it before. I'll open it and *try* to explain it. Then fix it via ^ | |
| 17:38:40 | dansmith | agree | |
| 17:39:21 | efried | Does the Big Lock in the rt mutex the _update periodic with the instance_claim path? | |
| 17:39:39 | efried | I guess it would have to. | |
| 17:41:04 | dansmith | that's the point of it I think | |
| 17:41:55 | efried | confirmed. | |
| 18:11:50 | openstackgerrit | Merged openstack/nova master: Merge server create schema for multiple create extension https://review.openstack.org/578338 | |
| 18:11:57 | openstackgerrit | Merged openstack/nova master: Merge server create schema for scheduler hint extension https://review.openstack.org/578343 | |
| 18:42:49 | karimull | Eric: I have one issue with a functional test case in nova on shared RP, can you help me understand where the issue is? here is the link | |
| 18:43:07 | karimull | Eric: http://paste.openstack.org/show/i9a6MRtJ1IUxF9xlI8to/ | |
| 18:45:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add action initiator attribute to the instance payload https://review.openstack.org/536243 | |
| 18:52:11 | karimull | efried: oops! pasted with wrong handle | |
| 18:52:31 | efried | karimull: looking... | |
| 18:52:48 | karimull | efried:thank you.. | |
| 18:53:05 | efried | karimull: What patch are you working on this test for? | |
| 18:57:01 | karimull | efried:https://review.openstack.org/#/c/569498/9 | |
| 18:57:56 | efried | karimull: Is the rebuild succeeding | |
| 18:57:57 | efried | ? | |
| 18:58:21 | efried | karimull: This is trying to rebuild to the same host? | |
| 18:59:27 | openstackgerrit | Merged openstack/nova master: conf: Correct documentation for '[pci] passthrough_whitelist' https://review.openstack.org/552874 | |
| 18:59:33 | openstackgerrit | Merged openstack/nova master: [doc] enhance admin/configuration/api.rst https://review.openstack.org/566008 | |
| 18:59:40 | openstackgerrit | Merged openstack/nova master: Remove mox in sec group test and functional tests https://review.openstack.org/576751 | |
| 19:00:21 | karimull | Efried:server state is Active after rebuild, but the DISK_GB usage is not accounted? | |
| 19:00:40 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Time how long pre_live_migration() takes https://review.openstack.org/574481 | |
| 19:00:50 | efried | karimull: or it got the disk from the compute node. | |
| 19:00:56 | efried | karimull: Where is the flavor defined? | |
| 19:02:53 | openstackgerrit | Merged openstack/nova master: conf: Resolve Sphinx errors https://review.openstack.org/579545 | |
| 19:02:55 | karimull | efried:I commented out "self.assertEqual({'DISK_GB': 1}, shared_rp_usages)" and the test passed | |
| 19:03:02 | karimull | efried: one min | |
| 19:03:39 | efried | karimull: I'm not completely sure how rebuild works, but I can't see where you're checking that the rebuild happened at all. | |
| 19:04:34 | efried | karimull: The other thing I'm confused about is how the rebuild could be succeeding - assuming the flavor asks for MEMORY_MB - because afaict you've *replaced* the inventory with just VCPU:1 (L92 of your paste). That's why I want to see the flavor... | |
| 19:04:50 | efried | oh, I guess it doesn't reallocate. | |
| 19:05:04 | efried | okay, I think I understand what's going on. | |
| 19:06:45 | efried | When you initially create the VM, it's getting its DISK_GB from the compute node. If you want to force it to get the DISK_GB from the sharing provider, you'll have to remove the DISK_GB inventory from the compute node. I think the libvirt driver does this for you at this point; try copying L76 to after L80 and see if it blows up. | |
| 19:07:18 | karimull | efried:ok..let me check | |
| 19:08:25 | efried | I think the other thing to know here is that I kinda doubt we're reallocating properly on rebuild with sharing providers. I don't think we've gone through and scrubbed the code for this. | |
| 19:15:24 | cdent | efried: there's a fixme at that part of the code for any kind of migration-like thing, if I remember right | |
| 19:16:21 | efried | cdent: Yes, I believe you are correct. | |
| 19:16:56 | jmlowe | I've been chipping away at getting erasure coding support for ceph, patches in review for glance and cinder. I hit a wall with nova. | |
| 19:17:50 | efried | cdent, karimull: It's not clear where we're messing up in this flow, though. Is the initial build allocating from the sharing provider or the compute node provider? Is the rebuild changing allocations at all? No combination of answers fully makes sense with what karimull is seeing, though. | |
| 19:18:27 | openstackgerrit | Andreas Karis proposed openstack/nova master: Add documentation for emulator threads policy https://review.openstack.org/579956 | |
| 19:18:34 | efried | cdent: Do I tag a bug `placement` if it's a bug in nova's use of placement? | |
| 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 | |