Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-10-21
15:52:46 gibi mriedem: will do
15:55:30 gmann mriedem: melwitt replied on host_status policy patch. that is only doc bug for 2.75. let me know if ok to fix in same patch otherwise i can push enw one - https://review.opendev.org/#/c/679181/1/nova/policies/servers.py@120
15:55:35 mriedem dansmith: done, removed from queue
15:55:54 dansmith cool
15:56:10 mriedem gmann: maybe push a separate fix and melwitt can rebase on top of it
15:57:41 gmann mriedem: ok, even same for extended-server-attributes. https://github.com/openstack/nova/blob/964d7dc87989b5765fcc60d34f734963ab8e03e7/nova/policies/extended_server_attributes.py
16:00:13 melwitt gmann: yeah if you upload a fix I'll rebase my patch on top of it
16:00:28 gmann melwitt: ok, doing..
16:00:38 mriedem gibi: something occurs to me - what if _update_pci_request_spec_with_allocated_interface_name runs and updates the instance pci_requests, then we fail later in whatever operation (resize, cold migrate, evacuate), we don't roll that back right? so instance.pci_requests would be wrong after that and might cause issues, but i'm not sure what kind - maybe hard reboot would fail?
16:04:20 gibi mriedem: good point. The InstancePciRequest change is not rolled back.
16:04:39 gibi mriedem: it could cause the failure of the pci_claim in the rt
16:04:58 gibi mriedem: do we re-claim resource at hard reboot? I don't think
16:05:01 mriedem no
16:05:16 gibi mriedem: if the instance is migrated forward then the migration will update the pci request again
16:06:04 gibi so the pci_claim at the second migration will see consitent data
16:07:36 gibi mriedem: now I think not rolling back this is not causing problems. but I agree that it is not nice to have wrong data in the PCI request after rollback
16:08:55 mriedem so could we have a case like,
16:09:10 mriedem 1. create server using port request and it gets parent_ifname foo,
16:09:25 mriedem 2. try to migrate the server and we update the instance pci_request to point at parent_ifname bar,
16:09:39 mriedem 3. migrate fails and we don't rollback the instance pci_request
16:09:54 mriedem 4. another server is created - it can't use parent_ifname bar even though nothing is using it right?
16:10:14 mriedem iow it's "claimed" by the failed first server
16:10:25 gibi this is not part of the claim
16:10:41 gibi it drivers the claim to select the good PF
16:10:52 gibi but the whole PF never claimed
16:11:07 mriedem ok, then how about if we hard reboot the failed server, will it try to use parent_ifname bar and fail if another guest on the same host is already using that?
16:11:42 gibi mriedem: if we hard reboot then nobody will check the parent_ifname. It is only the VF pci address that matters
16:12:14 mriedem ok i don't know how any of this stuff is actually used in the driver
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

Earlier   Later