Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-02-17
08:34:25 bauzas gibi: sean-k-mooney: I agree with sean's proposal for a specless bp approval during Feb 22th meeting
08:34:38 bauzas this way, we will see whether we have a consensus
08:38:17 bauzas sean-k-mooney: gibi: procedural -2 for https://review.opendev.org/c/openstack/nova/+/823941
08:38:39 gibi bauzas: ack
08:39:07 bauzas sean-k-mooney: if you want, you can review this change and +2 it
08:39:37 bauzas so, in case we agree on accepting it for Yoga, I'd just remove the -2 and do +W
08:40:37 sean-k-mooney[m] i have actully reviewed it and the lack of bluepinrt/bug was the main thing that stopped me form +2ing before
08:40:50 sean-k-mooney[m] but sure ill take a look again
08:41:07 sean-k-mooney[m] and ya we can decide on tuesday
08:42:39 bauzas ++
08:42:57 bauzas sean-k-mooney: thanks btw. for having reviewed it
09:18:25 yuval hello
09:18:53 yuval I am just making sure the feature freeze is the week of the 21 meaning -> 25.2 right?
09:26:15 gibi yuval: I think the cut date for us will be 2.24 EOB but things that are approved can be rebased and merged after it
09:35:22 yuval whats EOB stand for?
09:36:12 yuval I have this: https://review.opendev.org/c/openstack/nova/+/821606
09:36:30 yuval its not complicate - its just waiting for os-brick release to pass zuul
09:40:57 gibi end of business
09:41:25 gibi so basically end of the day of cores on the west coast
09:42:27 yuval got it
09:42:31 gibi yuval: as far as I see your os-brick patch is not part of that release https://review.opendev.org/c/openstack/os-brick/+/823654
09:42:36 gibi as this is not landed yetr
09:43:46 yuval gibi: this is not the important patch for me, although I do think brian want it in the release
09:44:07 yuval os-brick 5.2 dont have the lightos connector in it
09:44:15 yuval so zuul is failing in nova for it
09:44:29 gibi ahh so you only need https://review.opendev.org/c/openstack/os-brick/+/821603
09:44:53 yuval yes
09:45:02 gibi and that is already landed and will be in os-brick 5.2
09:45:03 gibi cool
09:45:10 gibi then I think you are on the good track
09:45:20 yuval ok, thanks
09:45:23 gibi I will check you nova patch soon
09:45:31 yuval that would be great
10:15:56 sean-k-mooney gibi: regarding the placment changes i think your current implemenation is likely ok to proceed with. we could add followup patches to try the alternitive approch if we do think the perfload resulst are refective fo a real regression
10:16:55 gibi sean-k-mooney: I have to figure out how to make a less noisy perfload measurement because currenty the noise is comparabel with the amount of degradation I can measure
10:17:19 gibi but at least I did not make the performance halved :D
10:18:00 gibi sean-k-mooney: I'm happy to make followup patches trying to improve especially as you gave directions with the SQL I can try to apply and measure
10:19:02 sean-k-mooney ya i dont really want to hold up the review on the subquery vs join approch escpially since i dont have performacne data one way or another
10:19:28 gibi sean-k-mooney: do you have any other comment that I should fix before FF?
10:19:39 gibi I'm happy to if there is anythign
10:19:39 sean-k-mooney so i woudl prefer to take the "get it working; then make it fast" approch provided there is no large regression
10:20:01 sean-k-mooney am not so far i have not got to the end of the series yet
10:20:13 sean-k-mooney but so far things look pretty good
10:21:06 gibi sean-k-mooney: thanks. let me know if I need to fix up thing quickly :)
10:21:12 gibi and thanks for the review
10:22:11 sean-k-mooney huh i didnt know that tuple unpacking work with dictionaries
10:22:16 sean-k-mooney https://review.opendev.org/c/openstack/placement/+/826491/8/placement/handlers/resource_provider.py
10:22:52 sean-k-mooney i understand how it could i guess i just tought that would be a syntax error
10:24:14 gibi yeah that was a nice touch from the python language that you can pack out into a dict
10:24:22 gibi into a dict value
10:24:28 sean-k-mooney gibi: its out of scope of the current spec but do you want to eventually support any_traits for root_required by the way
10:24:57 gibi I have no need for it right now, but if there is some use case from others then I can do it, most of the infra is in place for it
10:25:25 gibi probably the DB query needs some touching and the API needs to call the new trait parsing code
10:26:00 sean-k-mooney ok do we use root_required in nova today. i dont think so but perhaps we do
10:27:44 sean-k-mooney im not sure i have a usecase for any traits with it currently either but your serise will serve as a template for adding it if we need it in the future anyway
10:29:00 gibi sean-k-mooney: require_image_type_support, accelerators_filter and remote_managed_ports_filter prefilters are using it in nova today
10:29:17 gibi it is mosly to filter for compute capabilities
10:29:30 gibi and for that the simple AND trait list is enough
10:29:32 sean-k-mooney really i tought those were just in the unnamed group
10:29:48 gibi they could be there too
10:29:58 gibi both works today as we always allocate from the root
10:30:22 sean-k-mooney ya i just was not aware we converted to useing root_requried for that
10:30:22 gibi the code is a bit simpler with root_required as you don't need to find the unnamed group
10:30:35 sean-k-mooney ya
10:35:46 sean-k-mooney gibi: which patch are you adding the 1.39 micorversion is it the last patch
10:35:54 gibi yes
10:35:56 sean-k-mooney ah yes it is
10:36:19 sean-k-mooney ok i was just looking at the func test changes and wondering why you only had the negitive tests for 1.38
10:36:28 sean-k-mooney ok that makes sense
11:16:45 gibi yuval: left comments in https://review.opendev.org/c/openstack/nova/+/821606
11:19:54 yuval Thanks
11:32:45 sean-k-mooney yuval: how much work woudl it be to entirely factor out the monitor thread and "message queue" out of the nova code
11:33:12 sean-k-mooney i really dont like the idea of os-brick or the volume driver spawnign a background thread
11:33:46 sean-k-mooney im pretty sure i have objected to adding that to os-brick on the mailing list in the past
11:34:26 gibi chateaulav: I have an idea what is wrong with the object compatibility code still in https://review.opendev.org/c/openstack/nova/+/828369
11:34:52 gibi chateaulav: left a comment
11:35:26 sean-k-mooney hum we likely can move the call to super to the end
11:36:33 sean-k-mooney gibi: actully looking at the version im a littel confused why was a version bump done for 1.18 and 1.19
11:36:59 sean-k-mooney i have not looked at the patches that added those but adding a new method does not require an object version bump
11:36:59 gibi sean-k-mooney: I think remotable methods need a bump
11:37:10 sean-k-mooney ah
11:37:16 sean-k-mooney they are not jsut local
11:37:21 sean-k-mooney perhaps
11:38:00 gibi about the super call, I think yes we can move even way down
11:38:21 sean-k-mooney i would prefer not to put it in the midel of the compat ifs
11:38:47 sean-k-mooney so either have it first or after all of them ideally
11:40:56 yuval sean-k-mooney: can you add your comment to the patchset I discuss it with my colleagues
11:42:11 sean-k-mooney https://review.opendev.org/c/openstack/nova/+/821606/8/nova/virt/libvirt/volume/lightos.py#59
11:46:13 sean-k-mooney yuval: this is the feedback i left on the cinder spec over a year ago https://review.opendev.org/c/openstack/cinder-specs/+/766732/12/specs/wallaby/nvme-agent.rst#73
11:53:18 sean-k-mooney yuval: os-bricks executor.Thread is just a monkey patched green thread so if you are doing long running work that never yeild it will block nova
11:56:44 chateaulav gibi: sounds good, I'll take a look as soon as get in in about an hour
11:56:52 gibi chateaulav: ack
11:58:36 sean-k-mooney yuval: this for exampel will block nova until its finished processing it https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/lightos.py#L145-L162
11:58:59 sean-k-mooney so these sleeps https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/lightos.py#L174-L180
11:59:11 sean-k-mooney are the only thing that cause that to yeild back to nova
12:02:44 sean-k-mooney some of the operation in https://github.com/openstack/os-brick/blob/5902166149ba46da5f36eb352e62f3361269fb2c/os_brick/initiator/connectors/lightos.py#L107 also do io which will cause eventlet to yeild but in genrealy i dont think we want to allow that to run in a thread in the compute agent
12:04:24 sean-k-mooney lightos is not the only problematic driver that is now tryign to do this
12:05:30 sean-k-mooney but i really dong like this design parrent in os-brick and i think all montors/deamon threads shoudl be seperate processes or not part of os-brick at all
12:07:13 sean-k-mooney ... os-brick is doing this in several places.... that is really not good we should probaly make this a ptg topic
12:19:32 yuval ok, thanks for the feedback, we will think what our options

Earlier   Later