| opendevreview | YushoYamaguchi proposed openstack/nova-specs master: Allow disabling host checksum offload for vhost-user https://review.opendev.org/c/openstack/nova-specs/+/1011067 | 02:54 |
|---|---|---|
| opendevreview | Abhishek Kekane proposed openstack/nova master: Add Glance preferred stores support https://review.opendev.org/c/openstack/nova/+/1011470 | 03:37 |
| tkajinam | hmm it seems https://review.opendev.org/c/openstack/nova/+/1006786 broke some tests (at least a few founds in cinder-tempest-plugin) repeating volume attachments ... | 04:50 |
| tkajinam | it actually breaks it correctly but I guess we have to update some test scenarios to make sure that they wait until attachment actually completes... | 04:50 |
| tkajinam | context: https://zuul.opendev.org/t/openstack/build/7557579c282b406e95138b928e8e91e7 | 04:50 |
| opendevreview | Kamil Sambor proposed openstack/nova master: deprecate: concurrency_backend config option https://review.opendev.org/c/openstack/nova/+/1011450 | 07:16 |
| opendevreview | Kamil Sambor proposed openstack/nova master: Default native threading for CLI entry points https://review.opendev.org/c/openstack/nova/+/1011606 | 07:16 |
| opendevreview | Juan Larriba proposed openstack/nova master: Add OpenTelemetry tracing middleware for Nova https://review.opendev.org/c/openstack/nova/+/982871 | 07:44 |
| opendevreview | dalekseev proposed openstack/nova master: Restrict machine type check to QEMU instances https://review.opendev.org/c/openstack/nova/+/991137 | 11:25 |
| gibi | tkajinam: good catch. could you please file a bug. I agree, based on the logs, that the test is not waiting for the attachment to finish | 12:19 |
| tkajinam | gibi, will do | 12:46 |
| gibi | thanks | 12:51 |
| opendevreview | Kamil Sambor proposed openstack/nova master: Fix pickling error that forces oslo.service to fall back to fork https://review.opendev.org/c/openstack/nova/+/1000956 | 13:23 |
| tkajinam | reported https://bugs.launchpad.net/nova/+bug/2171624 | 13:26 |
| sean-k-mooney | attching multiple volume sconcurently should be porhibitied as we can only have one task in flight at once | 13:27 |
| tkajinam | bauzas0 gibi , I guess ^^^ is the expected behavior after we merged 1006786 and we should drop that test case because such concurrent volume attachments to a single server should be avoided, but it'd be nice if you could double check my thought | 13:27 |
| sean-k-mooney | previsoulsy it woucl have been serialised at the compute | 13:27 |
| tkajinam | yeah that's what I thought | 13:27 |
| sean-k-mooney | we have a chice i guess we can do it at the compute serially using the instnace lock | 13:28 |
| sean-k-mooney | or we can block it at the api based on task_Sate!=none | 13:28 |
| sean-k-mooney | although that can race | 13:28 |
| sean-k-mooney | if your talking to 2 apis at the same time so we always need the instnace lock on teh compute regardless | 13:29 |
| sean-k-mooney | the instance lock on the compute agent is also one of the things that guards agaisn starting a cinder volume migrate or retype durign say an instance live migrate | 13:30 |
| tkajinam | I personally prefer api level lock and reject concurrent actions explicitly so that revert action can be simplified | 13:30 |
| sean-k-mooney | the task state shoudl also help but its the final backstop to prevent 2 operations form trying to muteate the domain at the same time | 13:31 |
| tkajinam | yeah | 13:31 |
| sean-k-mooney | tkajinam: same but that does rely on use updating the task state before doing the rpc to the compute or ealy in the conductor flow | 13:31 |
| tkajinam | 1006786 added task state update in conductor layer so that implements what you said iiuc | 13:33 |
| sean-k-mooney | basicly if task state is the primary guard then the api has to update it before doing an rpc to the conducotr or comptue for all instnace actions | 13:33 |
| tkajinam | it's not lock at api layer(which I said) but conductor layer | 13:33 |
| sean-k-mooney | tkajinam: the conductor is kind fo too late but its better then the compute | 13:33 |
| tkajinam | yeah I understand | 13:34 |
| sean-k-mooney | the action are async and diffent conductors could start processing it howpever wehn we do save we pass the expefcteed state | 13:34 |
| sean-k-mooney | so one of the 2 conductor shoudl get a db error and fail teh second attach | 13:35 |
| sean-k-mooney | i think that is fine. we do not provide any ordering guarentey for concurnet api calls anyway | 13:35 |
| sean-k-mooney | im bit surpprosed that no functional regression test was created for this bug however | 13:37 |
| sean-k-mooney | this is they type of bug where we normally required those before proceeding with the fix | 13:37 |
| sean-k-mooney | bauzas0: was there a reason you didnt create a functional regression test first to demonstrate the race? | 13:38 |
| tkajinam | I've made the job non-voting in the mean time but also proposed 1011657 to stop testing concurrent attachments. just fyi... | 13:45 |
| sean-k-mooney | ack | 13:51 |
| sean-k-mooney | tkajinam: what job was failing? | 13:52 |
| sean-k-mooney | and is that test in core tempest or a plugin? | 13:52 |
| tkajinam | sean-k-mooney, cinder-tempest-plugin-lvm-concurrency-tests job | 13:52 |
| sean-k-mooney | ah ok | 13:52 |
| tkajinam | the failing test case is in cinder-tempest-plugin | 13:52 |
| sean-k-mooney | i was just wonderign why id was not failing on our gate | 13:52 |
| tkajinam | yeah | 13:53 |
| tkajinam | the plugin has a few test cases testing concurrent operations (such as concurrent volume creations, concurrent volume backups) and concurrent volume attachments is one of these | 13:54 |
| tkajinam | though I feel like these tests can reside in core tempest | 13:54 |
| tkajinam | hm interestingly that job only runs in cinder-tempest-plugin. idk why | 13:55 |
| opendevreview | Michael Sherman proposed openstack/nova master: ironic: Restore retries on 409 and 503 from Ironic https://review.opendev.org/c/openstack/nova/+/1011683 | 15:54 |
| *** raineszm0 is now known as raineszm | 16:55 | |
| opendevreview | Merged openstack/nova master: Enable G (flake8-logging-format) rules https://review.opendev.org/c/openstack/nova/+/998557 | 18:34 |
Generated by irclog2html.py 4.1.0 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!