| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-02-19 | |||
| 14:48:33 | sean-k-mooney | the resouce tracker calls the driver | |
| 14:48:47 | sean-k-mooney | so you can just add it to get_available_resource | |
| 14:48:50 | sean-k-mooney | in the driver | |
| 14:49:07 | sean-k-mooney | which is what you linked too | |
| 14:50:12 | sean-k-mooney | the alternitive which i assume you wanted to dicuss | |
| 14:50:24 | sean-k-mooney | was adding a new virt driver api function | |
| 14:50:33 | sean-k-mooney | and then calling that directly | |
| 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 | |