| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-01 | |||
| 09:56:03 | sdague | cdent: what comms channel did you notice this on? | |
| 09:56:19 | cdent | i’m getting a lot of email for reviews that I’m watching | |
| 09:56:25 | cdent | or otherwise participating in | |
| 09:56:36 | cdent | so: from gerrit | |
| 09:57:55 | sdague | gotcha | |
| 10:02:00 | openstackgerrit | Gábor Antal proposed openstack/nova master: Transform instance.resize_prep notification https://review.openstack.org/465081 | |
| 10:18:35 | openstackgerrit | Chris Dent proposed openstack/nova master: Test resize with placement api https://review.openstack.org/487958 | |
| 10:22:20 | gibi | cdent: I will push an update to ^^ soon with some refactoring | |
| 10:22:42 | gibi | cdent: removing duplicated code and better naming variables | |
| 10:23:21 | cdent | gibi: are you up to date on the plan there? make it pass on current master, with expectations commented out, and then adjust it based on all the code jay’s been working. | |
| 10:23:31 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Test resize with placement api https://review.openstack.org/487958 | |
| 10:23:39 | gibi | cdent: yes, I read back | |
| 10:23:45 | cdent | go you | |
| 10:24:01 | gibi | cdent: what I did is that I update ps 14 on master with some refactoring | |
| 10:24:23 | gibi | cdent: also locally I moved top of the bugfix to see that confirm works with the uncommented asserts as well | |
| 10:24:38 | cdent | there was a hangout between dan and jay last night that went on for quite some time, I was only able to particpate for a short while. it was complicated. | |
| 10:24:41 | bauzas | cdent: I still need to understand the consensus now | |
| 10:24:58 | cdent | bauzas: as far as I can tell there isn’t yet a consensus on a fix | |
| 10:25:06 | bauzas | okay | |
| 10:25:10 | bauzas | let's wait for the others | |
| 10:25:19 | cdent | we are aware of the problem and want to make it as visible as possible, thus the functional test going in first | |
| 10:25:26 | gibi | exactly this is why I only did refactoring that does not impact whatwhever will be the fix for the bug | |
| 10:27:24 | cdent | bauzas: in the conversations last night, each time a potential solution was discussed another problem was revealed | |
| 10:27:32 | bauzas | heh | |
| 10:28:09 | cdent | one idea that was well liked was that a migration/resize would have a second allocation with a different consumer id, instead of the doubling | |
| 10:28:36 | cdent | but that discussion exposed problems with migrations happening between ocata computes and pike computes | |
| 10:28:41 | bauzas | shouldn't we buy https://www.amazon.com/Hasbro-40509-Whac-A-Mole-Game/dp/B0001GDP00 ? | |
| 10:28:48 | cdent | probably yes | |
| 10:29:05 | bauzas | I see | |
| 10:29:12 | bauzas | about the 2nd consumer discussion | |
| 10:29:24 | cdent | sdague: and now the launchpad emails are starting | |
| 10:29:31 | bauzas | honestly, looks like a doubling allocation seems difficult | |
| 10:29:54 | bauzas | when I provided my comment, I was just thinking of getting the original allocation and just passing it back in case of an exception | |
| 10:30:12 | bauzas | I didn't thought about all the other problems we could have :( | |
| 10:31:05 | sdague | cdent: yeh, I'm flushing the in progress bugs, it moves at least 25 out of in progress | |
| 10:31:30 | bauzas | sdague: I did that too | |
| 10:32:24 | cdent | bauzas: It’s good that we are talking this stuff out, because we don’t really have sufficient testing to find the bugs that we are creating (we need more gibi ) so applying brains is necessary. Each question, though annoying, is revealing something, and that’s better in the long run. | |
| 10:33:18 | bauzas | sdague: do we have jobs for resizing to a 2nd host ? I know that for live-migration, but I do wonder if we only verify resizes for the same host | |
| 10:39:04 | johnthetubaguy | bauzas: I think XenAPI already shrinks the disks when resizing down | |
| 10:40:10 | bauzas | johnthetubaguy: ack | |
| 10:40:30 | bauzas | johnthetubaguy: anyway, stopping to accept at the API level a resize down would probably need a microversion | |
| 10:40:37 | johnthetubaguy | cdent: when we chatted about this before, I remember we kinda liked the doubling up, are folks finding thats bad? | |
| 10:40:58 | johnthetubaguy | bauzas: I guess it should, although that's kinda removing a feature some folks use | |
| 10:41:15 | cdent | johnthetubaguy: we need the doubling up, the problem is that it is hard to manage the cleaning up afterwards | |
| 10:41:16 | bauzas | johnthetubaguy: exactly my point I wrote in the review | |
| 10:41:34 | johnthetubaguy | cdent: oh, I see what you mean now | |
| 10:41:56 | johnthetubaguy | cdent: would the migration uuid holding an allocation work, or mess things up totally for sync logic? | |
| 10:41:58 | bauzas | johnthetubaguy: either we say nova will stop supporting that for all drivers and then it requires a microversion, or we say it's per-driver and then we don't want to have the API verifying it | |
| 10:42:29 | johnthetubaguy | bauzas: I would rather the API new if the compute host could do it | |
| 10:42:40 | johnthetubaguy | knew | |
| 10:43:02 | bauzas | johnthetubaguy: yeah, agreed, a capability | |
| 10:43:05 | cdent | johnthetubaguy: if there were a migration uuid, that would help, but there’s not, and getting access to it in the confirm_resize is weird (not sure I have all the details of this right, was very tired while listening to dan and jay last night) | |
| 10:43:29 | bauzas | cdent: we have a migration object, couldn't that help ? (well, for all migrations except live-mig :D) | |
| 10:43:32 | johnthetubaguy | cdent: oh, damm, did we never add that | |
| 10:43:45 | cdent | johnthetubaguy: that’s what dan and jay said :) | |
| 10:43:51 | cdent | (the “oh damn”) | |
| 10:43:58 | johnthetubaguy | cdent: oh, thats correct, migration is completed before the confirm/revert phase I think | |
| 10:44:02 | bauzas | cdent: and FWIW we expose those migration objects to the API | |
| 10:44:14 | bauzas | s/FWIW/AFAIK | |
| 10:45:07 | johnthetubaguy | bauzas: in my head that was getting added as part of the cancel resize/migrate work | |
| 10:45:59 | openstackgerrit | Sean Dague proposed openstack/nova master: Show quota detail when inject file quota exceeds https://review.openstack.org/453040 | |
| 10:46:18 | bauzas | johnthetubaguy: you mean os-migrations ? | |
| 10:46:30 | johnthetubaguy | bauzas: I think so | |
| 10:46:56 | bauzas | but whatever, we haven't yet a migration object for live migrations :) | |
| 10:47:21 | johnthetubaguy | bauzas: can't remember if the additions merged now, I thought it was shared for both now | |
| 10:47:31 | bauzas | maybe we could just split nikola's patch in two and just at least add the migration object at first, before trying to claim | |
| 10:47:34 | johnthetubaguy | anyways, not sure any of that helps | |
| 10:47:51 | sdague | bauzas: we run the resize tests in multihost I think | |
| 10:48:01 | bauzas | sdague: cool then | |
| 10:48:06 | sdague | bauzas: but I haven't looked that hard to verify | |
| 10:48:16 | bauzas | sdague: I can check | |
| 10:51:41 | cdent | sdague, bauzas: we talked about that last night, and dan confirmed there are some, but the issue with them is that they don’t run in a constrained environment nor concurrent placements and don’t validate the allocations, so the fact that there isn’t yet doubling of allocations isn’t an issue because there’s spare capacity | |
| 10:52:02 | sdague | cdent: yeh, that makes sense | |
| 10:52:31 | sdague | honestly, that's one of those things where doing the in tree functional testing with a couple of fake computes is probably the best way to flush it out | |
| 10:52:43 | cdent | sdague: that’s what gibi’s new test does | |
| 10:52:46 | cdent | so is very good to have | |
| 10:52:57 | sdague | cdent: does that still need review? | |
| 10:53:14 | bauzas | cdent: dan confirmed there are some what ? | |
| 10:53:39 | cdent | but I suspect that there are lots of edges that won’t get covered and in about 18 months we’ll see a lot of bugs from big deployments | |
| 10:53:46 | cdent | bauzas: there are some resize tests | |
| 10:54:01 | cdent | sdague: yes, it needs review, but I’m not sure how “done” it is: https://review.openstack.org/#/c/487958/ | |
| 10:54:20 | bauzas | cdent: ah | |
| 10:54:32 | bauzas | cdent: yeah, I know that but I do wonder if we test those multihost | |
| 10:54:46 | cdent | yes, multihost | |
| 10:54:51 | bauzas | ok | |
| 10:55:36 | bauzas | cdent: I see your comment now, you mean those test exist but aren't really stress-testing at limits so we can't really verify capacity issues | |
| 10:55:42 | bauzas | tests* | |
| 10:56:09 | bauzas | so, not a big deal for functional testing, but not for placement-specific concerns | |
| 10:56:15 | bauzas | gotcha | |
| 10:58:23 | gibi | cdent, sdague: I'm still intended to understand and remove the last time.sleep(1) left in the setUp() of that tests as that is ugly and might not be necessary | |
| 11:05:32 | openstackgerrit | Merged openstack/nova master: fix test_rebuild_server_exc instability https://review.openstack.org/487382 | |
| 11:08:54 | gibi | OK I think that sleep was only needed before the fake virt driver was updated to keep a local nodes copy and the periodic tasks was running every second | |
| 11:09:21 | gibi | that two things together caused a race on setting the nodes of the fake virt driver | |
| 11:09:25 | sdague | gibi: yeh, I just went through with a fine toothed comb there and provided some import | |
| 11:09:28 | sdague | input | |
| 11:09:57 | gibi | sdague: thanks. checking... | |
| 11:10:13 | sdague | gibi: mostly also bringing fresh eyes of not being familiar with some of this, so some code restructure for clarity that might help as well | |
| 11:11:46 | gibi | sdague: sure, fresh eyes helps a lot | |
| 11:12:39 | openstackgerrit | Merged openstack/nova master: Fix example in _serialize_allocations_for_consumer https://review.openstack.org/487614 | |
| 11:16:22 | openstackgerrit | Merged openstack/nova master: rootwrap.d cleanup mislabeled files https://review.openstack.org/486831 | |