| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-06-04 | |||
| 16:22:55 | bauzas | moving on then | |
| 16:23:00 | bauzas | #topic Open discussion | |
| 16:23:11 | bauzas | erlon: nova handling of virDomainGetJobStats() errors | |
| 16:23:27 | erlon | so, I posted a link on the wiki | |
| 16:23:41 | erlon | https://gist.githubusercontent.com/sombrafam/8f177cbc4e153c328a242811bc24650e/raw/67190ffce7f036c7c3d3628fda15327cef7a41da/nova-compute.log | |
| 16:23:45 | bauzas | (woah, we have 4 topics to discuss today so please discuss everyone by only 5-10 mins maxc) | |
| 16:23:54 | erlon | This bug, is happening every time that we try to do a lot of migrations | |
| 16:24:29 | bauzas | just file a blueprint bug report | |
| 16:24:36 | erlon | For some reason the source host stops responding, and it gets to the time out. I want to know if would be okay to add a new Handler for this kind of exception there: | |
| 16:24:36 | erlon | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/guest.py#L669 | |
| 16:24:40 | bauzas | and ping us anytime on the chat | |
| 16:24:45 | sean-k-mooney | erlon: does that imply you you have set the max conncurrent live migration over 1 | |
| 16:24:51 | erlon | ah ok, sounds good | |
| 16:25:14 | erlon | I don't know the exact configuration but very likely | |
| 16:25:24 | sean-k-mooney | https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.max_concurrent_live_migrations | |
| 16:25:35 | sean-k-mooney | that should bassicly never be set to anything excpet 1 | |
| 16:26:02 | erlon | So why does that exist? Which case should it apply? | |
| 16:26:12 | sean-k-mooney | there are usecases for it to be != 1 but unless you using 100G networking its not recommended | |
| 16:26:16 | bauzas | it just says it can't connect to the libvirt API | |
| 16:26:37 | sean-k-mooney | it exists for usecases wehere a single core is not enough to saturate the network link | |
| 16:26:47 | bauzas | there could be different reasons why libvirt is resetting the connection | |
| 16:27:02 | erlon | It says that because it time it out due to a short keepalive | |
| 16:27:02 | sean-k-mooney | bauzas:right but since they said this was load related | |
| 16:27:22 | sean-k-mooney | not doning concurrent live migrations may help | |
| 16:27:40 | bauzas | oh, missed that sentence, indeed good point | |
| 16:27:50 | bauzas | don't try to max out the number of calls you make to libvirt | |
| 16:28:01 | bauzas | like any other API, it has limits | |
| 16:28:23 | erlon | This comes from the libvirt logs: "At the same time in libvirtd logs: `May 28 13:00:39 ps5-ra1-n6 libvirtd[612268]: internal error: connection closed due to keepalive timeout`" | |
| 16:28:41 | erlon | So, that's why we know thats a timeout error | |
| 16:28:51 | bauzas | erlon: to answer your original question, no, I wouldn't recommend to modify Nova to handle that libvirt exception | |
| 16:29:28 | bauzas | particularly when the bug comes from libvirt, not nova itself | |
| 16:30:04 | erlon | But see, that function does exactly that. it handles only libvirt exceptions | |
| 16:30:31 | sean-k-mooney | there are some cases where retrying at the nova level is correct | |
| 16:30:38 | sean-k-mooney | this may or may not be one of them | |
| 16:30:39 | bauzas | specific and meaningful libvirt exceptions, that's the point :) | |
| 16:30:47 | sean-k-mooney | can you right this up as a bug report if you have not already | |
| 16:31:03 | sean-k-mooney | and we can see see if this is such a case | |
| 16:31:03 | bauzas | retrying on a generic connection failed is never a good idea | |
| 16:31:13 | erlon | ok, I will as soon as I get a reproducer | |
| 16:31:33 | sean-k-mooney | bauzas: we do that all the time for rest calls | |
| 16:31:34 | bauzas | honestly, you have my opinion | |
| 16:31:49 | bauzas | sean-k-mooney: to placement | |
| 16:31:50 | sean-k-mooney | this is us doing a read call to check the status fo the job | |
| 16:31:58 | sean-k-mooney | bauzas: and neutorn and cinder | |
| 16:31:58 | erlon | I was actually thinking and just returning, an empty info and leave nova call it again in the next periodic task, since this is just a info report | |
| 16:32:48 | sean-k-mooney | perhaps or catching it an logging a warning | |
| 16:32:56 | sean-k-mooney | rather then a trace back | |
| 16:33:03 | bauzas | I wouldn't hide | |
| 16:33:11 | sean-k-mooney | anywya if you have a repoducer we can discuss the solution in gerrit | |
| 16:33:12 | bauzas | if I were catching that exception | |
| 16:34:00 | erlon | The issue with the current process is that, although it triggers the section again, the | |
| 16:34:00 | erlon | periodic tasks do not complete the migration. Consequently, while the VM is migrated | |
| 16:34:00 | erlon | to the destination host, it is not properly cleaned up on the source host. | |
| 16:34:03 | bauzas | honestly, exception giving a better log to the operator saying 'libvirt is getting weirdo, see", I don't see the benefit of adding another handler to a periodic | |
| 16:34:11 | opendevreview | Merged openstack/nova stable/2023.2: Improve logging at '_numa_cells_support_network_metadata' https://review.opendev.org/c/openstack/nova/+/900840 | |
| 16:34:30 | bauzas | anyway, we have three other topics to discuss | |
| 16:34:45 | bauzas | erlon: please file a bug report and come to us once's done | |
| 16:34:56 | erlon | Okay, let's move, thanks | |
| 16:34:57 | bauzas | and yeah, a reproducer may help | |
| 16:35:11 | bauzas | particularly with some mitigation actions | |
| 16:35:25 | bauzas | like, I know libvirt gives me this but we can workaround by that | |
| 16:35:49 | bauzas | that way, we could log a better warning than just "heh, look, that's what I got from libvirt" | |
| 16:36:04 | bauzas | moving on anyway | |
| 16:36:14 | bauzas | next item (2 of 4) | |
| 16:36:19 | bauzas | marlinc: rotation_rate blueprint: https://blueprints.launchpad.net/nova/+spec/rotation-rate | |
| 16:36:32 | bauzas | I guess you're asking for a specless approval ? | |
| 16:37:00 | marlinc | Honestly I am not entirely sure what is best, I thought specless might work for this but I'm not sure | |
| 16:37:10 | bauzas | that's the point | |
| 16:37:26 | bauzas | you're saying you would get the detail from the cinder connection info | |
| 16:37:31 | bauzas | do we already have that ? | |
| 16:37:53 | sean-k-mooney | if we add a new compute service version for this and a min compute service version check i think that woudl resolve most if not all the upgrade concerns | |
| 16:38:07 | marlinc | Yes so we have an internal Cinder driver for our own storage platform which does give that property in the cinder connection info, we didn't have change anything in Cinder itself | |
| 16:38:12 | bauzas | I would honestly then recommend to write a spec | |
| 16:38:26 | sean-k-mooney | marlinc: is there an upstream driver that support this | |
| 16:38:31 | bauzas | there could be some upgrade and compatibility concerns I may see | |
| 16:38:36 | sean-k-mooney | marlinc: without that we can proceed with this in nova | |
| 16:38:47 | sean-k-mooney | we do not add supprot for out of tree drivers in other project | |
| 16:38:49 | marlinc | No there is currently no upstream driver that has this | |
| 16:39:12 | bauzas | then we can't just modify nova to blindly check anything that's not upstream | |
| 16:39:22 | sean-k-mooney | so implementing this in the lvm or a diffent intree cinder driver would likely be a requirement if we did this for cinder only | |
| 16:39:29 | bauzas | spec it is | |
| 16:39:49 | bauzas | and I guess you probably need to discuss that with the cinder folks | |
| 16:40:24 | marlinc | Oke I will look into creating a spec and also see if I can get this implemented in a in-tree driver | |
| 16:41:13 | bauzas | thanks | |
| 16:41:26 | bauzas | ping me anytime if you require assistance on the paperwork process | |
| 16:41:27 | marlinc | I'm going to have to see how to implement the min compute service version check and bump | |
| 16:41:42 | sean-k-mooney | marlinc: i would proably add it to the lvm driver personally, i think it woudl be relvitly simple to do | |
| 16:41:58 | bauzas | before doing anything with version checks, please check with the cinder folks about populating the rotation rate into the connection info details | |
| 16:42:14 | bauzas | yeah, lvm seems the easiast approach | |
| 16:42:25 | marlinc | (Also actually the functional test for live migration, honestly I have never implemented something so complex as I have no experience at all with the testing framework and how it works in Nova) | |
| 16:42:26 | bauzas | but that's not my own garden :) | |
| 16:42:48 | marlinc | Especially since there is no existing volume based live migration functional I could see | |
| 16:42:54 | bauzas | marlinc: once you get a go from the cinder folks about exposing the rotation rate, come to me | |
| 16:43:03 | sean-k-mooney | marlinc: if cinder accpet the enhancement we can revisit that and or help you with that | |
| 16:43:07 | marlinc | Alright | |
| 16:43:20 | bauzas | cool | |
| 16:43:23 | bauzas | moving on then | |
| 16:43:26 | marlinc | Thank you, I'll update our internal ticket | |
| 16:43:44 | bauzas | marlinc: I assume you know how to reach the cinder folks? | |
| 16:44:36 | marlinc | Well, I'm going to assume #openstack-cinder and I'll check their contribution documentation | |