| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-10-18 | |||
| 14:43:45 | openstackgerrit | Eric Fried proposed openstack/nova master: ItemsMatcher: mock call list arg in any order https://review.opendev.org/689487 | |
| 14:44:14 | efried | gibi: You'll like ^ :) | |
| 14:52:46 | bauzas | melwitt: sorry was afk, I meant to not import any module by relative | |
| 14:53:34 | bauzas | any import should be absolute | |
| 14:53:46 | bauzas | that's it | |
| 14:53:48 | melwitt | bauzas: np. I know, I'm saying how do you do the "from |
|
| 14:54:15 | bauzas | melwitt: "import fixtures" is absolute for it :) | |
| 14:54:24 | bauzas | I meant for packages | |
| 14:54:44 | bauzas | fixtures is both the package and the module you wanna import | |
| 14:54:53 | bauzas | nova.tests.unit.fixtures isn't | |
| 14:55:15 | bauzas | http://sametmax.com/les-imports-en-python/ | |
| 14:56:16 | melwitt | right, but if your test is in nova/tests/functional/my_test.py and you need 'import fixtures' and there is a nova/tests/functional/fixtures.py it would pick up the latter, we now know | |
| 14:56:50 | melwitt | anyway, just saying I don't see a way to avoid the problem other than using 'from __future__ import absolute_import' as has been mentioned earlier | |
| 14:58:33 | mdbooth | Thanks, all! I'll update the test. | |
| 14:59:06 | mdbooth | import fixtures as fucking_fixtures | |
| 15:00:19 | mdbooth | __future__ works :) | |
| 15:02:03 | openstackgerrit | Matthew Booth proposed openstack/nova master: Unplug VIFs as part of cleanup of networks https://review.opendev.org/663382 | |
| 15:03:53 | mdbooth | efried: I split out the regression test from the new bugfix, btw. Turned it into a double regression test with 2 separate fixes: https://review.opendev.org/#/c/689278/ | |
| 15:04:57 | efried | mdbooth: I was thinking that might be appropriate, but I was too fried to make a stink about it yesterday. Thanks. | |
| 15:05:14 | mdbooth | efried: Yeah, np. Thanks for looking. | |
| 15:06:31 | efried | cdent, gibi, bauzas, mriedem: I think all the ducks are lined up for: Always trait the compute node RP with COMPUTE_NODE https://review.opendev.org/688979 | |
| 15:06:39 | mdbooth | efried: I was also fried, and extremely annoyed at finding a new bug I wasn't looking for. | |
| 15:06:55 | efried | and btw, "if you build it, they will come" -- I came across another use case for it yesterday https://review.opendev.org/#/c/676522/21/nova/compute/resource_tracker.py@1823 | |
| 15:08:10 | bauzas | efried: cool, I'll just honestly leave it for monday | |
| 15:08:39 | bauzas | (and with no kids at home, my productivity will dramatically increase) | |
| 15:09:48 | efried | bauzas: are they "not at home" like moved out, or just on vacation? | |
| 15:10:05 | bauzas | both, grand-parents care | |
| 15:10:13 | bauzas | for the week \o/ | |
| 15:10:21 | efried | woot | |
| 15:10:25 | bauzas | indeed | |
| 15:10:28 | mdbooth | bauzas: Party at your house :) | |
| 15:11:02 | bauzas | technically, our friends and us have no kids at same time | |
| 15:11:18 | bauzas | I don't wanna rush into details about the fact we already planned the whole week | |
| 15:11:58 | mdbooth | bauzas: Why haven't you taken the week off? | |
| 15:12:36 | bauzas | because I still love my job and despite the fact it looks like, I'm not on perpetual PTO (c) mriedem | |
| 15:13:12 | mdbooth | He's just jealous | |
| 15:14:21 | bauzas | I'm pretty sure France still provides work permits for people who'd like to :p | |
| 15:14:37 | bauzas | there is one drawback tho | |
| 15:14:50 | bauzas | you have to leave in France | |
| 15:17:21 | cdent | "normal tea" | |
| 15:17:26 | cdent | such a british thing to say :P | |
| 15:17:33 | bauzas | mdbooth: it exists, it's called Courchevel | |
| 15:17:44 | bauzas | well, you also have to speak russian too | |
| 15:19:13 | mdbooth | cdent: You know, tea that's just normal. Like PG Tips or Yorkshire Tea. Not fancy. Just... normal. | |
| 15:19:23 | bauzas | mdbooth: do you think British MPs would be more inclined to accept the deal if we would add a clause for selling Courchevel ? Honestly, it's not like we miss anything | |
| 15:19:54 | bauzas | (sorry, Friday here) | |
| 15:20:23 | mdbooth | bauzas: I tried to put myself in the mind of a British MP once. I awoke from a coma 2 months later. Haven't tried again. | |
| 15:23:19 | cdent | only 2 months? | |
| 15:24:26 | sean-k-mooney | cdent: my mother refers to barrys or lyons tea as normal tea | |
| 15:24:35 | sean-k-mooney | cdent: so it translate to ireland too | |
| 15:46:11 | mriedem | efried: i'm going to have to take a different approach on https://review.opendev.org/#/c/689049/ where the caller has the context, like this: http://paste.openstack.org/show/784726/ | |
| 15:46:20 | mriedem | and pretty much leave move_allocations as-is | |
| 15:46:53 | mriedem | if we had consumer types i could determine from move_allocations if the instance still exists or not... | |
| 15:47:13 | mriedem | or just pass the types into move_allocations but that kind of sucks | |
| 15:48:12 | efried | mriedem: don't we still have a race in that situation? | |
| 15:48:47 | mriedem | as in when i refresh the instance is still exists but is gone by the time move_allocations does the POST? | |
| 15:49:03 | efried | yeah | |
| 15:49:18 | efried | Small window -- but the window you're closing is pretty durn small too. | |
| 15:49:34 | mriedem | i guess that's possible, but i i'm not sure how to detect that when consumers in placement are ephemeral | |
| 15:50:39 | mriedem | meaning when we get the consumer allocs we pass the generation if we have one otherwise we just pass None if it's a new consumer | |
| 15:50:47 | efried | iow you need to know *before* you decide what to do in placement? | |
| 15:51:15 | efried | Instead of refreshing the instance, you could GET its allocations from placement. | |
| 15:51:39 | efried | then the window is n/a because when you push back if the generation doesn't match you'll blow up. | |
| 15:51:44 | mriedem | which is what move_allocations does | |
| 15:52:48 | efried | mriedem: let me stare at it a bit after my meeting here. | |
| 15:52:59 | efried | (by which time you'll have probably figured it out anyway) | |
| 15:53:11 | mriedem | dealing with a head cold so my brain isn't really functioning | |
| 16:16:53 | openstackgerrit | Dan Smith proposed openstack/python-novaclient master: Add aggregate-cache-images command and client routines https://review.opendev.org/687141 | |
| 16:16:54 | openstackgerrit | Dan Smith proposed openstack/python-novaclient master: Add images.GlanceManager.find_images() bulk query https://review.opendev.org/689537 | |
| 17:09:21 | efried | mriedem: when an instance is in CONFIRM_RESIZE, is the migration allocation still on the dest? | |
| 17:15:07 | eandersson | Would enabling randomize_allocation_candidates help with race conditions (RescheduleException)? | |
| 17:15:50 | eandersson | We heavily favor stacking, which tends to cause race conditions when deploying a lot of small flavors. | |
| 17:16:16 | eandersson | Especially bad when deploying 1k+ VMs | |
| 17:18:53 | efried | eandersson: are you deploying them with a single command (max_instances=N or whatever that thing is) or multiple concurrent/quick operations? | |
| 17:20:09 | eandersson | multiple concurrent using terraform or ansible | |
| 17:20:16 | eandersson | if I do a single command it works fine | |
| 17:20:49 | dansmith | efried: the migration one is yeah | |
| 17:20:52 | eandersson | we have a lot of custom weighers and filters that probably make things.. slower (more likely to end in a race condition) | |
| 17:20:56 | dansmith | efried: the instance one is on the dest | |
| 17:21:10 | dansmith | efried: sorry, the migration allocation is on the _source_, the instance is on the _dest_ | |
| 17:21:19 | dansmith | assume that's what you meant | |
| 17:21:45 | eandersson | We used host_subset_size and max_attempts to counter it | |
| 17:34:12 | efried | However, just like the delete thing that started you down this rathole, we can't close that window effectively while doing the GET from within move_allocations | |
| 17:34:12 | efried | - If an oob operation updates the instance's allocs (which only applies to the execute case; I assume you can't resize an instance that's in CONFIRM_RESIZE or ERROR, right?) we also want the migration to fail. | |
| 17:34:12 | efried | - In either case, if instance disappears, we want the operation fail, but that's something the caller needs to be aware of, not move_allocations itself. | |
| 17:34:12 | efried | - revert, including from a failed migration: source is migration, target is instance. | |
| 17:34:12 | efried | - (live/cold) migrate execute: source is instance, target is migration. | |
| 17:34:12 | efried | mriedem: Correct me if I'm wrong, move_allocations needs to deal with these scenarios: | |
| 17:34:13 | efried | - move_allocations should *not retry on a consumer 409*; it should raise AllocationMoveFailed. It *should* retry on a *provider* 409 though. (This is a little complicated, and we have seen this issue elsewhere -- gibi L@@K.) | |
| 17:34:13 | efried | - move_allocations should always use 'allocations': {} to delete, never DELETE. | |
| 17:34:13 | efried | So here's what I think needs to happen: | |
| 17:34:14 | efried | - Eventually (probably not now) the calling flows should be responsible for pulling the instance consumer (at least the generation, if not the whole allocation record) *early*, before entering the "critical path", and funneling that information into move_allocations. This is the only way we're going to effectively detect the race. | |
| 17:34:15 | efried | TL;DR the only thing that should change right now is: don't retry on consumer 409. | |
| 17:35:38 | efried | eandersson: randomize sounds like it would help there, yes. | |
| 17:36:41 | efried | eandersson: when you said you favor "stacking", did you mean "packing" (as opposed to "spread")? | |
| 17:37:47 | efried | If you're doing that ^ via your weighers/filters, then randomize probably won't do you a lot of good. | |
| 17:38:16 | efried | because your weigher will trawl down the list, random or not, and pick the fullest host anyway. | |
| 17:38:42 | melwitt | eandersson: what version are you running again? as of claims in placement, the rescheduling due to racing parallel requests shouldn't be a thing | |
| 17:39:08 | efried | melwitt: he's using separate parallel processes | |
| 17:39:15 | melwitt | I know | |