Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-21
16:12:21 mriedem just thinking about fallout scenarios
16:12:24 gibi parent_finame is not a consumable, it is like a trait
16:12:58 gibi it helps selecting the proper pool of VFs during the pci_claom
16:13:45 mriedem efried: alex_xu: looks like we have a fun TypeError for the resources stuff added late in Train https://zuul.opendev.org/t/openstack/build/2b10b4a240b84245bcee3366db93951d/log/logs/screen-n-cpu.txt.gz?severity=4#2675
16:15:01 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Fix policy doc for host_status and extended servers attribute https://review.opendev.org/689833
16:15:07 gibi mriedem: made a TODO to look into how hard to roll back the parent_ifname during rollback or revert
16:17:25 mriedem gibi: ack, not a huge deal
16:17:35 mriedem i know you like to pull these types of threads in your spare time :)
16:17:44 gibi :)
16:18:09 mriedem efried: alex_xu: https://bugs.launchpad.net/nova/+bug/1849165 seems there is a race in that resources code in the RT during migrations
16:18:09 openstack Launchpad bug 1849165 in OpenStack Compute (nova) "_populate_assigned_resources raises TypeError: argument of type 'NoneType' is not iterable" [High,New]
16:22:08 mriedem this is where i say even things that touch the RT which aren't used still have side effects...
16:30:19 efried mriedem: is that happening because mig.instance.migration_context isn't set at that point?
16:33:15 mriedem yeah
16:33:33 mriedem i haven't traced everything here, but in this case i think the RT is running the periodic on the dest host before the instance gets there
16:33:44 mriedem so the Migration record exists pointing at the source and dest host, but the instance hasn't moved yet
16:34:14 mriedem and given the migration record is usually created in the control plane but the migration context doesn't exist until we do a claim in the compute, there is a window
16:34:56 mriedem dansmith: remember https://review.opendev.org/#/c/274870/ ? i'm seeing where we hit the KeyError in _pair_instances_to_migrations and end up still lazy-loading the migration.instance.migration_context/flavor - what do you think about changing Migration.instance to load the Instance with migration_context and flavor fields always?
16:35:28 openstackgerrit Eric Fried proposed openstack/nova master: Always trait the compute node RP with COMPUTE_NODE https://review.opendev.org/688979
16:35:34 efried bauzas: added reno ^
16:37:13 efried mriedem: so `if not mig_ctx: continue` ?
16:39:35 mriedem efried: yeah i think so
16:39:46 efried mriedem: I'll throw that out
16:40:19 mriedem _update_usage_from_migration is what populates self.tracked_migrations and you can see the "elif incoming and not tracked:" logic
16:40:23 mriedem that's the case we're hitting here i think
16:40:58 mriedem [None req-dd5ddbad-4234-4288-bbab-2c3d20b7f4ad None None] [instance: cd4148a2-4550-4e83-b6f7-c91752eaf779] Starting to track incoming migration 407fd025-e8ba-4012-ade7-d0255d2a1837
16:41:03 mriedem [None req-dd5ddbad-4234-4288-bbab-2c3d20b7f4ad None None] Error updating resources for node ubuntu-bionic-rax-iad-0012404623.
16:43:32 efried mriedem: but it looks like we should be under COMPUTE_RESOURCE_SEMAPHORE in all those code paths
16:43:40 gmann mriedem: melwitt https://review.opendev.org/#/c/689833/
16:43:41 mriedem doesn't matter
16:43:55 mriedem efried: that's per-worker, not global
16:44:02 mriedem efried: so:
16:44:14 mriedem 1. conductor creates migration record with source and dest host set,
16:44:24 mriedem 2. update_available_resource runs on dest host to start tracking incoming migration
16:44:29 mriedem kaboom
16:44:52 mriedem 3. claim happens on dest host to create migration_context (on a different request thread)
16:45:50 dansmith mriedem: yeah, makes sense.. if we're looking for an instance involved with a migration, it's likely we care about the migration context
16:47:04 mriedem dansmith: ok will push something
16:47:38 efried mriedem: are you working up a func test?
16:47:43 mriedem nope
16:48:11 efried is a func test possible?
16:48:41 mriedem yeah probably
16:48:52 mriedem i mean, i *could* write one up
16:49:10 mriedem if $$$properly$$$ motivated
16:52:07 openstackgerrit Dan Smith proposed openstack/nova master: Add image precaching docs for aggregates https://review.opendev.org/687348
16:52:24 efried I just can't seem to stop seeing that ^ as "preaching"
16:52:29 dansmith mriedem: ^ includes some generic image caching doc stuff in addition to precaching
16:52:56 dansmith as we discussed
16:53:56 mriedem is it worth marking "Partial-Bug: #1847302" or at least related?
16:53:56 openstack bug 1847302 in OpenStack Compute (nova) "doc: need admin guide for the image cache" [Undecided,New] https://launchpad.net/bugs/1847302
16:54:09 dansmith if you want
16:54:21 mriedem i think that woudl be good (Related-Bug)
16:54:28 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Don't populate resources for not-yet-migrated instance https://review.opendev.org/689842
16:54:31 efried mriedem: ^
16:54:33 openstackgerrit Dan Smith proposed openstack/nova master: Add image precaching docs for aggregates https://review.opendev.org/687348
16:54:54 dansmith mriedem: already did partial-, I can change to related- if you want, as I'm sure this will need some respins
16:55:10 mriedem up to you, only difference is with partial the bug is assigned to you
16:55:21 mriedem which might imply signing up for more work than you want
16:56:36 dansmith drat! :)
17:01:22 efried still trying to understand the race.
17:01:22 efried - create instance
17:01:22 efried - migrate instance, which has the following steps:
17:01:22 efried - x create migration context -- this creates the migration record and populates the instance in it, but the instance doesn't have a migration_context yet
17:01:22 efried - y schedule
17:01:22 efried - z claim on destination (_move_claim) -- this is the thing that sets the instance's migration_context
17:01:22 efried So we need update_available_resource on the dest compute to run after x and before z, yah?
17:03:52 openstackgerrit Matt Riedemann proposed openstack/nova master: Join migration_context and flavor in Migration.instance https://review.opendev.org/689846
17:03:53 mriedem your steps are wrong
17:04:32 mriedem you don't control when update_available_resource runs, it's on a timer in a thread pool
17:04:58 efried well, we can trigger it manually in a test
17:05:34 mriedem 1. create instance,
17:06:02 mriedem 2. wrap _prep_resize on the dest compute service to trigger the periodic on the dest when calling _prep_resize
17:06:08 mriedem 3. initiate a migration
17:06:11 mriedem check logs for TypeError
17:06:54 dansmith this is happening in CI yeah?
17:07:04 dansmith so more soak time and we would have found it I guess
17:07:07 mriedem yeah, multinode migration tests
17:09:42 mriedem efried: i can whip up a func recreate test quick if you want
17:09:43 efried can we please merge https://review.opendev.org/#/c/686207/ which will be useful for this?
17:09:54 mriedem no
17:09:55 efried mriedem: I'm working on it, wouldn't mind the experience.
17:10:01 mriedem because we don't want to backport that spy stuff
17:10:05 mriedem and this has to go to train
17:10:12 efried ight
17:10:34 mriedem i mean, i'd rather not backport the spy stuff
17:10:44 mriedem if enough stuff builds on it i guess it will eventually happen
17:11:38 efried slight catch-22 there
17:12:00 mriedem well, it would be nice if the spy stuff does land, that it soaks a bit before we have to backport it for something that otherwise wouldn't need it
17:12:35 mriedem cuz if there is a bug in spy then you have to fix and backport that later as well
17:12:37 efried okay. Its successor has soakage. Let's merge both
17:13:02 mriedem i don't think you can claim "soakage" on anything that isn't merged
17:13:06 dansmith to me, confidence in soakage comes from being in the firehose, not a few rechecks on a single patch
17:13:10 dansmith yeah, that ^
17:16:06 efried no, my point is, if we merge both of those things in master now, they're in the firehose because the second patch puts them in the way of I think three functional tests. Then we can soak them in master until you're comfortable before doing anything further.
17:26:22 openstackgerrit Merged openstack/nova stable/rocky: Stop sending bad values from libosinfo to libvirt https://review.opendev.org/688068
17:26:27 openstackgerrit Merged openstack/nova stable/stein: Fix unit of hw_rng:rate_period https://review.opendev.org/689153
17:42:35 artom sean-k-mooney, btw, do we care about https://review.opendev.org/#/c/465783/ not being in OSP10 Neutron?
17:42:56 artom (Sorry to make you context-switch - it's a dependency of https://code.engineering.redhat.com/gerrit/#/c/183568/1)
17:43:08 artom Doh, was meant for the internal channel
17:48:15 efried mriedem: I'm close-ish.

Earlier   Later