| opendevreview | Ghanshyam Maan proposed openstack/nova master: Fix intermittent 409 in TestGracefulShutdown tests https://review.opendev.org/c/openstack/nova/+/996381 | 00:51 |
|---|---|---|
| gmaan | melwitt: sean-k-mooney ^^ i think this will fix the test_cold_migration_dest_compute_graceful_shutdown (i think revert resize test also have same issue ) failure | 00:53 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Use domain capabilities to detect SEV-SNP support by libvirt/QEMU https://review.opendev.org/c/openstack/nova/+/996316 | 02:59 |
| opendevreview | minwoo seo proposed openstack/nova master: Add availability_zone support for migration https://review.opendev.org/c/openstack/nova/+/976085 | 05:22 |
| opendevreview | minwoo seo proposed openstack/nova master: Add availability_zone support for migration https://review.opendev.org/c/openstack/nova/+/976085 | 05:25 |
| opendevreview | minwoo seo proposed openstack/nova master: Add availability_zone support for migration https://review.opendev.org/c/openstack/nova/+/976085 | 05:31 |
| opendevreview | minwoo seo proposed openstack/nova master: Add availability_zone support for migration https://review.opendev.org/c/openstack/nova/+/976085 | 05:31 |
| opendevreview | minwoo seo proposed openstack/nova-specs master: Add spec for cross-AZ migration support https://review.opendev.org/c/openstack/nova-specs/+/976202 | 05:32 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Use domain capabilities to detect SEV-SNP support https://review.opendev.org/c/openstack/nova/+/996316 | 06:27 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Use domain capabilities to detect SEV-SNP support https://review.opendev.org/c/openstack/nova/+/996316 | 06:29 |
| ralonsoh | hello folks, if you have a few minutes, please check https://review.opendev.org/c/openstack/os-vif/+/995933 | 06:33 |
| ralonsoh | thanks in advance! | 06:33 |
| opendevreview | Takashi Kajinami proposed openstack/nova-specs master: Fix SEV-SNP feature detection in start up https://review.opendev.org/c/openstack/nova-specs/+/996394 | 06:34 |
| opendevreview | Takashi Kajinami proposed openstack/nova-specs master: Use domain capability to detect SEV-SNP support https://review.opendev.org/c/openstack/nova-specs/+/996395 | 06:39 |
| gibi | gmaan: sean-k-mooney: re threading as default in our unit test jobs. I'm OK in general to switch. The remaining disable unit tests needed DB fixture changes currently being developed in the functional test series. | 08:13 |
| opendevreview | minwoo seo proposed openstack/nova master: Add availability_zone support for migration https://review.opendev.org/c/openstack/nova/+/976085 | 08:37 |
| rubasov | hi nova folks: may I ask for a review on this bugfix (with a functional reproducer)? https://review.opendev.org/q/topic:bug/2051685 | 09:08 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Use domain capabilities to detect SEV-SNP support https://review.opendev.org/c/openstack/nova/+/996316 | 09:40 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron networks https://review.opendev.org/c/openstack/nova/+/928022 | 10:33 |
| opendevreview | Lajos Katona proposed openstack/nova master: Use SDK for Neutron networks https://review.opendev.org/c/openstack/nova/+/928022 | 10:58 |
| tkajinam | mhen, can you share the full content of the firmware descriptor file in Ubuntu ? | 11:27 |
| tkajinam | mhen, it's probably best to report a bug for ubuntu then add the link to it, just in case they fix that invalid descriptor | 11:27 |
| tkajinam | instead of describing the whole details in the local doc | 11:28 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Update documentations for AMD SEV-SNP support https://review.opendev.org/c/openstack/nova/+/995090 | 11:47 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Remove [libvirt] num_memory_encrypted_guests https://review.opendev.org/c/openstack/nova/+/995120 | 11:47 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Use domain capabilities to detect SEV-SNP support https://review.opendev.org/c/openstack/nova/+/996316 | 11:47 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Update documentations for AMD SEV-SNP support https://review.opendev.org/c/openstack/nova/+/995090 | 11:51 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Remove [libvirt] num_memory_encrypted_guests https://review.opendev.org/c/openstack/nova/+/995120 | 11:51 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Use domain capabilities to detect SEV-SNP support https://review.opendev.org/c/openstack/nova/+/996316 | 11:51 |
| opendevreview | Ashish Gupta proposed openstack/placement master: tests: Support file-backed SQLite URL in placement fixtures https://review.opendev.org/c/openstack/placement/+/993106 | 12:06 |
| opendevreview | Ashish Gupta proposed openstack/nova master: tests: use file-backed Placement SQLite in functional threading https://review.opendev.org/c/openstack/nova/+/992581 | 12:21 |
| *** chandank` is now known as chandankumar | 13:14 | |
| opendevreview | Ashish Gupta proposed openstack/nova master: tests: Use per-database write locks instead of global lock https://review.opendev.org/c/openstack/nova/+/992862 | 14:50 |
| Uggla | Reminder: Upstream bug triage in ~30mn. | 15:00 |
| sean-k-mooney | gibi oh actlly https://review.opendev.org/c/opendev/zuul-providers/+/996449 merged an hour ag | 15:15 |
| sean-k-mooney | *ago | 15:15 |
| gibi | ack | 15:15 |
| sean-k-mooney | so we shoudl not see failure in new jobs | 15:15 |
| sean-k-mooney | Uggla: url? | 15:34 |
| Uggla | https://meet.google.com/zjr-rxus-hzj | 15:34 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: libvirt: Remove redundant version check for virtio-fs https://review.opendev.org/c/openstack/nova/+/996496 | 15:41 |
| tkajinam | ^^^ wondering if we should check the virt type instead | 15:41 |
| sean-k-mooney | we woudl need to check for both qemu and kvm | 15:42 |
| tkajinam | (this also makes me wonder if we should require kvm for sev support. it may not work for the other virt type really. | 15:42 |
| sean-k-mooney | i.e virtio fs shoudl wor for qemu and kvm | 15:42 |
| sean-k-mooney | sev maybe | 15:43 |
| tkajinam | sean-k-mooney, yeah and I don't think it may work for lxc for example | 15:43 |
| sean-k-mooney | i dont know if that works for qemu or just with kvm | 15:43 |
| sean-k-mooney | well for lxc we are not actully using a vm so i dont think sev works for contianer in general | 15:44 |
| sean-k-mooney | you can enabeld sev for all memoy at the host level but if you do that it used to break sriov | 15:44 |
| tkajinam | sev definitely requires kvm because it relies on the implementations in kvm_amd module. I'm not too sure about virtiofs on the other hand | 15:45 |
| sean-k-mooney | i dont think that requires kvm | 15:45 |
| sean-k-mooney | but ya for sev you could add checkign teh vrit type to the supprot crtiria | 15:46 |
| sean-k-mooney | if its not already there | 15:46 |
| tkajinam | it's not, yet | 15:46 |
| tkajinam | that's has never been added since sev support was first introduced. | 15:46 |
| tkajinam | because that's an existing problem and may be trivial I'll look into that later, separately from sev-snp work | 15:47 |
| sean-k-mooney | well if it does not work it would just fail to boot the vm | 15:47 |
| tkajinam | yeah | 15:47 |
| sean-k-mooney | so its not really a regression to not report sev supprot when virt type is qemu | 15:47 |
| sean-k-mooney | tkajinam: if you can confim it i would just file a bug for it | 15:47 |
| sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/996496 looks ok ot me although im serpised we dont have any unit test coverage of the functions your removing | 15:48 |
| tkajinam | sean-k-mooney, I'll file one by my end | 15:49 |
| tkajinam | we might want to change that check to virtio-fs to account virt_type, instead of just removing it. | 15:49 |
| tkajinam | (I'll record that as a separate bug, too | 15:49 |
| opendevreview | Takashi Kajinami proposed openstack/nova master: Do not report SEV capability for non kvm virt_type https://review.opendev.org/c/openstack/nova/+/996498 | 16:07 |
| gibi | sean-k-mooney: I think futurist is just slow to shut down an exector https://bugs.launchpad.net/futurist/+bug/2160159 | 16:16 |
| sean-k-mooney | ah ok | 16:17 |
| sean-k-mooney | i mean we could actully swap to concurrent.futures.ThreadPoolExecutor(max_workers=100) if needed | 16:17 |
| sean-k-mooney | we have centralised the executor interaction in nova.utils | 16:18 |
| sean-k-mooney | but cool that sound like a real bug | 16:19 |
| gibi | yeah I will check if I can spot why | 16:19 |
| sean-k-mooney | https://github.com/openstack/futurist/blob/master/futurist/_thread.py#L110-L147 | 16:21 |
| sean-k-mooney | so its polling ever second https://github.com/openstack/futurist/blob/master/futurist/_thread.py#L98-L108 | 16:22 |
| sean-k-mooney | when its waitign for the work to complete | 16:22 |
| sean-k-mooney | but its also joining the work thread https://github.com/openstack/futurist/blob/master/futurist/_thread.py#L142 | 16:23 |
| sean-k-mooney | but in your lamda case that should not be costly | 16:24 |
| gibi | is it due to https://github.com/openstack/futurist/blob/master/futurist/_thread.py#L103 | 16:24 |
| gibi | ? | 16:24 |
| gibi | there is a clear 1 sec timeout passed there | 16:24 |
| sean-k-mooney | that what i was wondering too | 16:25 |
| sean-k-mooney | but that get is in blockign mode with a max of 1 second | 16:25 |
| sean-k-mooney | if the queue has a value it wont wait for one second | 16:25 |
| sean-k-mooney | i was wondering if you coudl jsut monkeypatch https://github.com/openstack/futurist/blob/master/futurist/_thread.py#L142 to 0.1 | 16:26 |
| sean-k-mooney | if that woudl have any effect on the timeings | 16:26 |
| sean-k-mooney | but i dont think it should | 16:26 |
| gibi | yeah I'm about to try it :) | 16:26 |
| sean-k-mooney | jsut looking at the code im not seeing nayting obviouly dumb like a hardcoded sleep or similar | 16:27 |
| sean-k-mooney | so your tesitn calling shudown but do we call stop before we do that. i assume shutdown sill do that interenally | 16:29 |
| sean-k-mooney | oh well stop is on the threadworker and shutdown is on the executor | 16:29 |
| gibi | it is the problem | 16:30 |
| gibi | if I set it to 0.1 then the shutdown is 10 times faster | 16:30 |
| gibi | added a comment to the bug | 16:30 |
| gibi | I will propose a patch top of Ashish's functional series to monkey patch that value in the test | 16:31 |
| sean-k-mooney | oh | 16:31 |
| sean-k-mooney | i know what happenign i think | 16:31 |
| sean-k-mooney | so each tradwroker is waiting up to a secodn but we have 10 workers | 16:31 |
| sean-k-mooney | and i bet when we call shutdown it loops over all of them serially | 16:32 |
| sean-k-mooney | i.e. the executor is calling stop on each threqad worker one at a time | 16:32 |
| sean-k-mooney | gibi: i bet if you double the worker count you will see it doubel coorect? | 16:32 |
| gibi | nope I tried that adding more worker does not change the timing | 16:33 |
| sean-k-mooney | huh weird | 16:34 |
| sean-k-mooney | https://docs.python.org/3/library/queue.html#queue.Queue.get | 16:34 |
| sean-k-mooney | it really is the upper bound | 16:35 |
| gibi | but the queue is empty in this case as there is no work left. So it does wait 1 sec before it raises Empty | 16:36 |
| sean-k-mooney | yes | 16:37 |
| gibi | and the shutdown logic only checked in the empty exception handler | 16:37 |
| gibi | so we need better logic. We need to wake up the queue during shutdown with a poison added to the queue | 16:37 |
| gibi | or ignore this and just monkey patch the 1 sec to a small number from our test :) In production that 1 sec is totally OK during shutdown | 16:38 |
| sean-k-mooney | https://github.com/openstack/futurist/blob/6f870896819197ca323b3dd8af8f3b0389aff69c/futurist/_futures.py#L224-L240 | 16:39 |
| sean-k-mooney | gibi: so yes in the tests we can set ti to somethign smal for now | 16:40 |
| sean-k-mooney | but that for over the woekers | 16:40 |
| sean-k-mooney | where we call join is why i tought it would scale with teh worksers | 16:40 |
| gibi | I think we wait that 1 sec per worker in parallel for each worker | 16:41 |
| opendevreview | Merged openstack/nova master: Fix intermittent 409 in TestGracefulShutdown tests https://review.opendev.org/c/openstack/nova/+/996381 | 16:41 |
| sean-k-mooney | sortof each worker will do that | 16:42 |
| sean-k-mooney | but https://github.com/openstack/futurist/blob/6f870896819197ca323b3dd8af8f3b0389aff69c/futurist/_futures.py#L237 | 16:42 |
| sean-k-mooney | when we call join there is seriailised | 16:42 |
| sean-k-mooney | so we are taking the shutdwon lock, setting studwo to ture and waiting for the queu to drain, | 16:44 |
| gibi | yes but join is not the one that triggers the wait for 1 sec. Join is just checks if the given thread is finished or not. At the first join we wait for 1 sec, then we see that the first thread finished, at the second join we see that the second thread already finished. etc | 16:44 |
| sean-k-mooney | then notifying all workser to stop | 16:44 |
| sean-k-mooney | and finally joining them all | 16:44 |
| gibi | we join serially but the thread wait for their own 1 sec in parallel | 16:44 |
| gibi | *threads | 16:44 |
| sean-k-mooney | i think your correct | 16:46 |
| sean-k-mooney | im just not sure why we see the wait | 16:46 |
| gibi | anyhow I will dig deeper tomorrow. The last thing I do today is put a patch up to nova to nuke this 1 sec in the test | 16:47 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: HACK:Speed up functional test in threading mode https://review.opendev.org/c/openstack/nova/+/996500 | 16:57 |
| sean-k-mooney | gibi: i might quickly hack on somehting, if i push anything ill add you to it and you can free free to udpate it or push your own fix | 16:57 |
| gibi | OK thanks | 16:57 |
| sean-k-mooney | im basiclly goign to ask ai to reporduce your finding in a test case and then see if it can rootcause it | 16:57 |
| gibi | https://review.opendev.org/c/openstack/nova/+/996500 for me locally re-gains all the lost time in the threading mode | 16:57 |
| gibi | sean-k-mooney: OK cool | 16:57 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: HACK:Speed up functional test in threading mode https://review.opendev.org/c/openstack/nova/+/996500 | 16:58 |
| sean-k-mooney | also cool lets see how that ^ goes in ci over night | 16:58 |
| gibi | yepp | 16:58 |
| gibi | dropping now | 16:58 |
| gibi | see you tomorrow | 16:58 |
| opendevreview | Thibaut Démaret proposed openstack/nova master: libvirt: add disk rotation_rate support for local disks https://review.opendev.org/c/openstack/nova/+/979693 | 17:42 |
| melwitt | I have a patch that has been up for a while with one +2, to make QEMU_IMG_LIMITS configurable if anyone might be interested https://review.opendev.org/c/openstack/nova/+/969538 this came from an old upstream CI issue where the image limit was too low for an encrypted disk with ceph | 18:45 |
| sean-k-mooney | melwitt: so while that helps | 20:22 |
| sean-k-mooney | we proably shoudl fix the memory limit if we are doign this | 20:22 |
| sean-k-mooney | melwitt: currently we are limiting on adress space but we shoudl be limiting on RSS | 20:22 |
| melwitt | hm ok .. I am not yet familiar with that | 20:23 |
| sean-k-mooney | address space is how much well adrsss space that is mapped into the process with or without actual memory usage | 20:24 |
| sean-k-mooney | RSS is residnet set size | 20:24 |
| sean-k-mooney | i.e. how much actual memory is being used | 20:24 |
| sean-k-mooney | in ci with some config we were seing newver version fo ceph/rbd map alot of memory without actully usign a lot of memroy | 20:25 |
| melwitt | oh I see | 20:25 |
| sean-k-mooney | https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L162 | 20:25 |
| melwitt | a-ha | 20:26 |
| melwitt | so you think if we just s/address_space/resident_set_size/ | 20:27 |
| melwitt | looks like rss has been a choice as long as address space ... so it is not a newly available choice. yet it has not been used | 20:27 |
| melwitt | I wonder if that was deliberate | 20:27 |
| sean-k-mooney | i would just sed it yes | 20:28 |
| melwitt | looks like no one is using it, huh. https://codesearch.openstack.org/?q=resident_set_size&i=nope&literal=nope&files=&excludeFiles=&repos= | 20:29 |
| sean-k-mooney | so i moved the ceph jobs to debian to work around it https://review.opendev.org/c/openstack/devstack-plugin-ceph/+/955714 and proposed a reviert of the tempest skip https://review.opendev.org/c/openstack/tempest/+/955177 | 20:31 |
| sean-k-mooney | but we never actully merge the tempest skip reviert | 20:31 |
| melwitt | yeah, I remembered that. I had wondered what the "real fix" should be | 20:32 |
| sean-k-mooney | i +1 your patch becasue we coudl proceed with it, i think movign to rss instead fo adresss space woudl make sense btu let see if gibi has or somoen else has an opion | 20:33 |
| sean-k-mooney | melwitt: im not agaisnt makign it configurabl i just dont think that the correct limit to use in this case in general | 20:34 |
| sean-k-mooney | but we coudl change that in a followup | 20:34 |
| melwitt | sure. I'm also happy to just change it to resident_set_size if there is no potential bad thing about that haha | 20:35 |
| melwitt | I am a bit wondering why no one has made use of it before | 20:35 |
| sean-k-mooney | well almost nothign uses this in general https://codesearch.openstack.org/?q=ProcessLimits&i=nope&literal=nope&files=&excludeFiles=&repos= | 20:36 |
| sean-k-mooney | and i think the code was cargo culted form nova to the rest | 20:36 |
| sean-k-mooney | tweaking the adress space limit will allow more meory to be used like rss would | 20:37 |
| melwitt | nova trend setter | 20:37 |
| sean-k-mooney | but you may have to set it higher then otherwise | 20:37 |
| melwitt | yeah. I mostly saw how cinder and ironic both doing it | 20:38 |
| sean-k-mooney | you know what ill appove your current patch but lets chat about this more | 20:38 |
| sean-k-mooney | your currrent patch matches cinder and ironics approch | 20:38 |
| sean-k-mooney | so if it works for them it will proably be fine | 20:39 |
| melwitt | it's ok, I don't mind chatting for rss | 20:39 |
| sean-k-mooney | ok but lets check back tomorrow so we dont forget :) | 20:39 |
| melwitt | I feel like dan would have a good input for this but I think he's away until next week | 20:40 |
| sean-k-mooney | gmaan: related to ^ i rebased https://review.opendev.org/c/openstack/tempest/+/955177 to get new results | 20:40 |
| sean-k-mooney | ok im goign to go eat dinner o/ | 20:41 |
| melwitt | ok seeya o/ | 20:41 |
| gmaan | sean-k-mooney: ack | 20:48 |
| opendevreview | Ghanshyam Maan proposed openstack/nova master: Task tracking mechanism for graceful shutdown https://review.opendev.org/c/openstack/nova/+/996299 | 21:38 |
| opendevreview | Merged openstack/nova master: Rename cyborg-tempest to cyborg-tempest-py3 https://review.opendev.org/c/openstack/nova/+/996297 | 21:57 |
| opendevreview | Ashish Gupta proposed openstack/nova master: tests: file-backed SQLite with WAL in threading mode for Database and CellDatabases Fixtures https://review.opendev.org/c/openstack/nova/+/988583 | 23:18 |
| opendevreview | Ashish Gupta proposed openstack/nova master: tests: use file-backed Placement SQLite in functional threading https://review.opendev.org/c/openstack/nova/+/992581 | 23:24 |
| opendevreview | Ashish Gupta proposed openstack/nova master: tests: Use per-database write locks instead of global lock https://review.opendev.org/c/openstack/nova/+/992862 | 23:27 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!