| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-02-02 | |||
| 15:40:02 | mriedem | https://review.openstack.org/#/c/428120/ | |
| 15:40:10 | edleafe | jaypipes: there is no HTTP method that will remove a RP and all of its associated invetory and allocations | |
| 15:40:10 | openstack | Launchpad bug 1659266 in OpenStack Compute (nova) "Disk allocation for instance is not good with swap" [High,Confirmed] - Assigned to Tanvir Talukder (tanvirt16) | |
| 15:40:10 | mriedem | https://bugs.launchpad.net/nova/+bug/1659266 | |
| 15:40:28 | edleafe | jaypipes: so adding one will mean a new microversion :( | |
| 15:40:29 | jaypipes | edleafe: I know, and I think that's appropriate. | |
| 15:41:05 | edleafe | I would certainly prefer to add it that way, but with RC1... | |
| 15:41:16 | jaypipes | edleafe: meaning DELETE /resource-providers/<UUID> should return a 409 if there are existing allocations and inventories. The user should be required to DELETE /allocations|inventories in order to perform the DELETE /r-p | |
| 15:41:35 | edleafe | jaypipes: of course | |
| 15:41:44 | jaypipes | edleafe: I don't care for implementing DELETE CASCADE in a single HTTP call. | |
| 15:42:00 | mriedem | let us rathole on that in ATL | |
| 15:42:04 | jaypipes | yup | |
| 15:42:07 | edleafe | jaypipes: well, that's why I said several HTTP calls | |
| 15:42:27 | edleafe | jaypipes: one to delete allocation, one to delete inventory, and the final to delete the RP | |
| 15:42:45 | jaypipes | edleafe: right, which is how things currently work. | |
| 15:43:09 | mriedem | think if we actually had hard constraints on the instances table and nova didn't auto delete all related things in code... | |
| 15:43:21 | mriedem | *auto soft-delete... | |
| 15:43:41 | dims_ | mriedem : dansmith : melwitt : should ServersNegativeTestJSON have been fixed? (with https://review.openstack.org/#/c/427782)? | |
| 15:43:53 | dansmith | dims_: yes, but we have another patch in the queue for it | |
| 15:43:57 | mriedem | dims_: yes, but https://review.openstack.org/#/c/427775/ | |
| 15:44:51 | mriedem | sfinucan: bauzas: want to send it on https://review.openstack.org/#/c/428198/ ? | |
| 15:44:55 | dims_ | mriedem : dansmith : melwitt : gotcha, saw it pop back up. will track 427775. thanks (http://logs.openstack.org/43/408143/9/gate/gate-tempest-dsvm-neutron-full-ubuntu-xenial/eb7a75d/logs/testr_results.html.gz) | |
| 15:45:47 | jroll | jaypipes: I'm putting this on a plaque "in the future I will fix the performance issues with the Ironic RT once and for all." | |
| 15:46:17 | dims_ | oh boy! | |
| 15:46:27 | jaypipes | jroll: heh. | |
| 15:49:02 | openstackgerrit | Pushkar Umaranikar proposed openstack/nova master: [WIP] Placement api: Add json_error_formatter to defaults https://review.openstack.org/395194 | |
| 15:54:47 | johnthetubaguy | mriedem: jaypipes: I can take a look at that in a few mins | |
| 15:55:54 | jaypipes | johnthetubaguy: actually, just reviewing you patch to fix the MB -> GB thing. running some tests on it locally, but overall the code looks fine (but also bauzas' identified issue with the allocation side also is a good find) | |
| 15:56:37 | openstackgerrit | Lance Bragstad proposed openstack/nova-specs master: Cleanup pike policy spec https://review.openstack.org/428222 | |
| 15:57:08 | bauzas | jaypipes: mriedem: FWIW I'm about to add a PTG bullet about discussing on modifying the disk resources so it's bytes | |
| 15:57:16 | lbragstad | johnthetubaguy ^ feel free to squash that into your original version (it's just cleanup to get your draft passing) | |
| 15:57:56 | johnthetubaguy | lbragstad: oh, cool, thanks | |
| 15:58:05 | jaypipes | bauzas: I support the general idea. but would be a pain in the ass to migrate all the MEMORY_MB and DISK_GB inventory and allocation records to bytes... | |
| 15:58:07 | johnthetubaguy | lbragstad: feel free to just push to my change if you want, thats cool | |
| 15:58:12 | johnthetubaguy | jaypipes: great, thank you | |
| 15:58:20 | lbragstad | johnthetubaguy yep! also - you have a todo in there for the keystone model | |
| 15:58:22 | jaypipes | bauzas: in addition to the fact that MEMORY_MB and DISK_GB are standard resource classes. | |
| 15:58:51 | lbragstad | johnthetubaguy I can take that action item - but where do you want that documented? I felt weird putting keystone things in a nova specific spec | |
| 15:59:16 | bauzas | jaypipes: exactly | |
| 15:59:29 | bauzas | jaypipes: that's why I think we should be discussing about it during the PTG | |
| 15:59:38 | bauzas | jaypipes: or during a hangout | |
| 15:59:51 | bauzas | because yes, changing that could be a PITA | |
| 15:59:56 | jaypipes | cool with me. | |
| 16:00:03 | jaypipes | (the discussions) :) | |
| 16:02:06 | edleafe | jaypipes: cdent just found a flaw in this approach | |
| 16:02:27 | edleafe | jaypipes: we can't delete allocations by RP or ID | |
| 16:02:39 | edleafe | jaypipes: only by consumer_id | |
| 16:02:55 | edleafe | jaypipes: and a consumer might have allocations across multiple RPs | |
| 16:03:36 | jaypipes | edleafe: yes, and? | |
| 16:04:01 | dansmith | so after an evacuate, | |
| 16:04:20 | dansmith | we wouldn't want to delete allocations for not-our-instance-anymore | |
| 16:04:58 | johnthetubaguy | lbragstad: ah, sorry, I missundertood, got you now | |
| 16:05:14 | jaypipes | edleafe: should still be able to delete the resource provider if the deletion of the consumers on that resource provider (identified with an InstanceList.get_all_by_host()) is called to DELETE /allocations/<instance_uuid> | |
| 16:05:48 | lbragstad | johnthetubaguy yep - line 102 here https://review.openstack.org/#/c/427872/3/specs/pike/approved/policy-cleanup-pike.rst | |
| 16:06:01 | jaypipes | dansmith: yeah, we would. the evacuate would result in a *new* allocation on a new resource provider. we would definitely want to delete the allocation on the old one. | |
| 16:06:08 | edleafe | jaypipes: yeah, that could work. | |
| 16:06:28 | dansmith | jaypipes: edleafe just said you can only delete by consumer id, which is instance uuid right? | |
| 16:07:17 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Improve assertJsonEqual error reporting https://review.openstack.org/428199 | |
| 16:08:07 | edleafe | dansmith: yeah, so to delete by compute node, you'd have to get all the instances for that node, and delete their allocations first | |
| 16:08:20 | jaypipes | dansmith: yes, what edleafe said. | |
| 16:08:50 | dansmith | edleafe: right, but after an evacuation we have allocations on two compute nodes for that instance, right? | |
| 16:09:14 | edleafe | dansmith: wouldn't the evacuation delete allocations? | |
| 16:09:17 | jaypipes | dansmith: I think what we're saying is that the evacuation process would delete the allocs on the source node before adding them in the dest node. | |
| 16:09:46 | dansmith | edleafe: how? allocations are only managed from the compute node's RT AFAIK | |
| 16:10:00 | dansmith | if we add that, then maybe that's okay | |
| 16:10:12 | dansmith | but I don't think it happens today | |
| 16:10:24 | sfinucan | diana_clarke: https://review.openstack.org/#/c/411441/2/nova/tests/unit/api/openstack/compute/test_cells.py | |
| 16:10:58 | jaypipes | dansmith: no, does not happen today. | |
| 16:11:19 | dansmith | jaypipes: so today we probably already corrupt the allocations after an evacuate anyway I guess | |
| 16:11:25 | edleafe | so let me summarize to make sure I'm clear on the evacuation scenario | |
| 16:11:30 | jaypipes | dansmith: sounds like it. | |
| 16:11:38 | dansmith | jaypipes: we just allocate again on the new node, and then maybe when the old node comes up, we delete the new and old ones | |
| 16:12:02 | edleafe | when we evacuate an instance to a new host, the instance ID stays the same, so the allocations for that instance remain | |
| 16:12:02 | jaypipes | dansmith: sounds right. | |
| 16:12:13 | jaypipes | dansmith: I mean, not right, but sounds like what currently would happen. | |
| 16:12:13 | dansmith | jaypipes: sounds wrong.. but accurate, unfortunately. | |
| 16:12:16 | dansmith | heh | |
| 16:12:19 | jaypipes | jinx :) | |
| 16:12:33 | edleafe | the new host for that instance would periodically update, see the new instance, and then re-allocate against the new RP | |
| 16:12:36 | jaypipes | dansmith: want to add a bug? or should I? | |
| 16:12:52 | dansmith | jaypipes: I have done nothing useful so far today, so let me | |
| 16:13:00 | jaypipes | edleafe: yes, eventually it would resolve itself... but still not ideal | |
| 16:13:10 | edleafe | If we then delete all allocations for that instance / consumer, there will be a window where there are no allocations for that instance | |
| 16:13:33 | dansmith | jaypipes: oh yeah I guess it will heal, although not after cdent's proposed lessening of the healing action I think | |
| 16:13:37 | dansmith | or at least, heal slower | |
| 16:13:57 | dansmith | jaypipes: but opens us up for a poor decision because we think that host is less loaded than it is | |
| 16:14:00 | cdent | dansmith: i've punted on that until we figure out how that stuff even works | |
| 16:14:11 | dansmith | cdent: yeah, I know, I'm just saying | |
| 16:14:26 | dansmith | cdent: either (a) this is a good reason to take the hit on the healing or (b) need to consider this kind of thing if/when we proceed with that | |
| 16:14:35 | cdent | I'd vote for b | |
| 16:14:55 | cdent | so we can get rid of some of the implictness and confusion when we talk about this stuff :) | |
| 16:15:20 | dansmith | well, things like this being papered over because we heal aggressively is problematic I think, | |
| 16:15:32 | dansmith | because before we heal, we're making bad decisions | |
| 16:15:48 | dansmith | which I think is an argument for (b), but.. | |
| 16:16:35 | cdent | ( )|( ) | |
| 16:17:31 | dansmith | um | |
| 16:17:43 | dansmith | a butt? | |
| 16:17:58 | cdent | a big ol butt | |
| 16:18:30 | dansmith | classy, cdent, classy | |
| 16:19:42 | openstackgerrit | Maciej Szankin proposed openstack/nova master: conf: mark glance.api_servers opt as required https://review.openstack.org/428241 | |