Earlier  
Posted Nick Remark
#openstack-nova - 2024-02-19
14:50:47 sean-k-mooney alternitivly you could decorate a method in the libvirt driver as a perodic
14:51:03 sean-k-mooney and have it run seperatly form those run by the compute manager
14:52:08 sean-k-mooney both are workable but the minimal change is just make get_avialable_resoucces call a new function to reconsile the dict.
14:53:25 sean-k-mooney bauzas: looking at the code i wonder if there is a better approch
14:53:52 sean-k-mooney bauzas: have you coniserd doing it in update_provider_tree
14:54:05 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L8869C9-L8869C29
14:54:25 sean-k-mooney that has the advantage of being passed in the allocations
14:54:50 sean-k-mooney so if you see there is not allocation against the provider but there is a reservation in the dict for an instance
14:55:03 sean-k-mooney that would be a stale reservation correct?
14:55:55 sean-k-mooney hum perhaps not
14:56:09 sean-k-mooney the allocation may be owned by the migration uuid
14:56:47 sean-k-mooney if you were to do it in _update_provider_tree i would do it in the existing _update_provider_tree_for_vgpu fucntion where you do reshapes
14:56:52 bauzas sean-k-mooney: sorry got a phonecall meanwhile
14:57:00 sean-k-mooney no worries
14:57:08 sean-k-mooney readback and we can chat later
14:58:09 bauzas sean-k-mooney: okay I see your point
14:58:49 bauzas so, we would need to inject the db to get the migrations in _update_provider_tree_for_vgpu()
14:59:04 bauzas sean-k-mooney: would you be okay ?
15:00:04 bauzas I mean calling objects.MigrationList.get_in_progress_and_error() in _update_provider_tree_for_vgpu()
15:01:49 dansmith I've lost track of what we're trying to solve here
15:02:14 dansmith I thought the periodic was to clean up reservations that are now stale for migrations that never happened?
15:02:40 dansmith in the driver memory-resiident dict
15:03:22 sean-k-mooney yes
15:03:32 sean-k-mooney so i think we can do that in _update_provider_tree_for_vgpu
15:03:34 dansmith so what injecting into the DB needs to happen?
15:03:50 sean-k-mooney nothing has to happen in the db
15:04:05 dansmith ack, okay, I'm confused by bauzas' statement then
15:04:08 sean-k-mooney dansmith: is the destionation allcotion held by the instance uuid or the migration uuid
15:04:18 sean-k-mooney i think the instance_uuid?
15:04:27 dansmith instance IIRC yeah
15:04:43 sean-k-mooney ack, assuming that is correct all _update_provider_tree_for_vgpu need to do
15:05:05 sean-k-mooney is loop over the in memory dict
15:05:12 dansmith maybe bauzas is talking about _querying_ for canceled migration objects to know what to clean from the dict?
15:05:15 sean-k-mooney and remove any key that does not have an allocation
15:05:17 bauzas dansmith: btw. I added a new check by https://review.opendev.org/c/openstack/nova/+/904209/16/nova/virt/libvirt/driver.py#1740
15:05:41 dansmith sean-k-mooney: you can't do that I think because you might be racing with the start of the migration?
15:05:55 bauzas dansmith: my question was about where we could verify the dict
15:06:02 dansmith sean-k-mooney: or are you thinking the instance already has an allocation for that host, until it's canceled and we can go on the presence of that allocation?
15:06:08 sean-k-mooney we should have claimed the placment allcotion before calling migrate correct
15:06:19 bauzas and I just don't want to create a new virt driver API method just for this
15:06:21 dansmith right, okay, yeah that makes sense
15:06:45 dansmith I'm just wondering what bauzas meant by "so, we would need to inject the db to get the migrations in _update_provider_tree_for_vgpu() "
15:07:13 bauzas sean-k-mooney: I wouldn't check the allocations, I would just verify that all the dict keys have a current migration
15:07:28 sean-k-mooney well that required addtional db lookups
15:07:36 sean-k-mooney which can only be done in the compute manager or resouce tracker
15:07:38 sean-k-mooney not the driver
15:07:44 dansmith bauzas: but "has an allocation for gpu on this host" is good enough right?
15:07:44 bauzas dansmith: I'm saying that I want to get the current migrations
15:07:51 sean-k-mooney so that why im trying to use the allcotions instead
15:08:15 dansmith it also means the reservation mirrors the allocation in placement, which seems right
15:08:16 bauzas dansmith: no, because you could have an allocation for an instance while it's not because of a migration
15:08:36 bauzas if we want to check the dict, we need to look at the *migrations*
15:08:41 dansmith bauzas: okay, but that would be for an instance that migrated in successfully in which case it doesn't hurt right?
15:08:45 bauzas dansmith: that's what you asked me
15:09:00 bauzas dansmith: no, it would only be for current migrations
15:09:10 bauzas once the migration is done, then the dict key is removed
15:09:19 bauzas (or aborted)
15:09:24 dansmith bauzas: actually, what I asked is for it to not to get out of sync silently so that things stop working and a restart is the only solution :)
15:09:25 bauzas so we need to compare
15:09:43 sean-k-mooney bauzas: not what dansmith is saying is if we have an entry in the dict and the instance ahs an allcoation agaisnt the host and there is no migration
15:09:50 dansmith bauzas: if that happens, then we're fine, this is just for cleanup if the migration does *not* get rolled back successfully right?
15:09:59 sean-k-mooney then its can only happy because the migration succeeded
15:10:07 dansmith sean-k-mooney: well, not exactly
15:10:10 sean-k-mooney and we failed to remove it form the dict for some bug
15:10:23 bauzas dansmith: that's what I'm saying, we need to compare migrations with the dict
15:10:38 dansmith sean-k-mooney: what I'm saying is that if there's a reservation in the dict and there's an allocation, then it's either still in progress, has yet to be rolled back, or it succeeded in which case keeping the reservation in the dict doesn't prevent other stuff from working
15:10:38 bauzas if a dict key is not related to a current migration, then it's stale
15:11:01 dansmith once the instance is deleted or migrated away, healing based on the allocation would work itself out too
15:11:02 sean-k-mooney dansmith: ya i agree with that
15:11:20 sean-k-mooney and yes to the healing too
15:11:28 sean-k-mooney also bauzas if we ever use consumer_types
15:11:39 dansmith bauzas: a dict key for a completed migration where the instance is on our host is not incorrect, even if it is stale, and won't harm anything yeah?
15:11:40 sean-k-mooney you can actully tell if the allocation is a migration if you really want too
15:12:24 sean-k-mooney so if we start using consumer types for that and we do this based on the allcoation if you want to filter you will be able to in the future
15:12:30 bauzas dansmith: I wouldn't be super happy, as we should have removed it by https://review.opendev.org/c/openstack/nova/+/904209/16/nova/virt/libvirt/driver.py#11355
15:12:43 dansmith can we gmeet?
15:12:49 dansmith I feel like we're going in circles here
15:12:56 bauzas sure, I can
15:12:58 bauzas sec
15:13:18 bauzas https://meet.google.com/cyg-ucsv-xri
15:14:08 dansmith sean-k-mooney: are you able to join?
15:14:50 opendevreview Amit Uniyal proposed openstack/nova master: enforce remote console shutdown https://review.opendev.org/c/openstack/nova/+/901824
15:17:34 opendevreview Amit Uniyal proposed openstack/nova master: enforce remote console shutdown https://review.opendev.org/c/openstack/nova/+/901824
15:28:41 sean-k-mooney dansmith: sorry no i was in/am in anohter call
15:29:05 sean-k-mooney dansmith:i can join quickly now since this call is just ending
16:01:51 opendevreview Mateusz Klejn proposed openstack/nova master: Fixed: unit tests for force delete instance https://review.opendev.org/c/openstack/nova/+/909464
16:01:52 opendevreview Mateusz Klejn proposed openstack/nova master: Changed: adjust shelving for shelved image protection https://review.opendev.org/c/openstack/nova/+/909465
16:04:17 mnaser Good morning! https://review.opendev.org/c/openstack/nova/+/909228 + https://review.opendev.org/c/openstack/nova/+/909229 are ready for review today :)
16:42:42 melwitt when O
16:43:49 melwitt *when I've looked into nova-lvm job failures, what I have seen is that they fail due to the glance apache server responding to nova with a HTTP 502 bad gateway. meanwhile no errors in glance logs https://etherpad.opendev.org/p/nova-ci-failures-minimal#L11
16:44:22 melwitt not really sure what to do next to debug
16:45:17 sean-k-mooney melwitt: this maybe be related to glance-api being OOM killed
16:45:45 sean-k-mooney melwitt: artom was debuging failure where that happend so it would be good to confirm that is not whats happening fro the nova-lvm case
16:46:12 artom "debugging" is a strong word
16:46:22 artom I chased a red herring only to figure out that glanceclient does chunking for us
16:46:27 melwitt hm ok. I didn't think there was OOM killer in these but let me check
16:46:35 artom So I still no have no idea why glance-api gets oom-killed
16:47:04 melwitt oh, yeah there is some OOM kill in here
16:47:05 sean-k-mooney melwitt: there may not be but if there is no error in the glance api log i would first check for an OOM event
16:47:36 sean-k-mooney so we chatted about turning on zswap on nova-next

Earlier   Later