Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-02-17
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 gibi sean-k-mooney: I think remotable methods need a bump
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: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
12:23:41 sean-k-mooney gibi: by the way nova still has root wrap solely for os-brick right. is there a reason we cant move this to os-brick?
12:23:43 sean-k-mooney https://github.com/openstack/nova/blob/master/etc/nova/rootwrap.d/compute.filters
12:25:36 sean-k-mooney gibi: looking at what os-brick is doing really make me think we shoudl not be loading it in the compute agent directly since its problematic in several ways
12:26:11 gibi I think only os-brick using rootwrap we converted the rest of privsep
12:26:16 gibi so I think we can move it
12:26:27 gibi but probably we need a volunteer
12:26:57 sean-k-mooney yep its just techdebth
12:26:57 gibi as of what os-brick does under the hood, I'm not that familiar with. I saw that iscsi connector also spawn threads so I assume that works other
12:27:12 gibi otherwise we would have failures
12:27:23 sean-k-mooney it works but they are green threads
12:27:27 sean-k-mooney not native threads
12:27:36 sean-k-mooney because we monkey patch and its running in the same process
12:27:50 sean-k-mooney os-brick does not use eventlets iteself
12:28:08 sean-k-mooney which is true for os-vif alos both both inherit it becasue they are imported into nova which does
12:28:46 gibi ahh you have a good point
12:28:58 gibi those just look like native threads but we monkey patched them
12:29:05 gibi but then I don't fully get the problem
12:29:21 gibi why don't they spawn greenlets freely?
12:29:25 gibi under the hood
12:29:52 sean-k-mooney gibi: simple since they are monkey patched if they dont yeild nova's event loop is blocked
12:30:11 gibi they have thight loops without io?
12:30:22 sean-k-mooney yes in some places
12:30:31 gibi ahh so they have some thight loops
12:30:41 gibi then those needs some careful yields
12:30:59 sean-k-mooney https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/lightos.py#L164-L182
12:31:38 sean-k-mooney so this yeild because monitor_db does some writes to a file and those sleeps
12:32:11 sean-k-mooney monitor_message_queue wont yeild unless the log satement casue it to happen
12:32:29 sean-k-mooney but it will terminate when the queue is empty
12:32:52 gibi I see
12:33:41 sean-k-mooney isci is potentally worse
12:33:43 sean-k-mooney https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/iscsi.py#L723-L780
12:34:04 sean-k-mooney it spawns a bunch of threds then joins in the multipath code
12:34:47 sean-k-mooney the join shoudl cause a yeild and those threads will be interleaved with nova
12:35:11 sean-k-mooney but this type of raw threadign is really not a good thing with eventlets in the mix
12:35:31 gibi jeah that seem a lot more complex. but still it works or we would have strange bugs
12:35:36 sean-k-mooney they also have sleeps https://github.com/openstack/os-brick/blob/master/os_brick/initiator/connectors/iscsi.py#L777
12:36:35 sean-k-mooney ya so that will do io an dthe sleep will yeild which the do on each iteration
12:37:11 sean-k-mooney i just think the use of thread in general in os-brick is problematic
12:45:37 opendevreview Imran Hussain proposed openstack/nova master: [nova/libvirt] Support for checking and enabling SMM when needed https://review.opendev.org/c/openstack/nova/+/825496
14:15:22 IPO Hello, all ! What are the next steps toward https://review.opendev.org/c/openstack/nova/+/805649 ? Or it is ok,it is needed just wait a bit ?
14:18:37 gibi IPO: I guess we need a second core
14:18:58 gibi sean-k-mooney: you were +2 before on that patch ^^
14:26:57 IPO gibi: I see, thanks for info !
14:42:29 yuval_ sean-k-mooney other the monitor thread - anything is a concern?
15:05:15 sean-k-mooney yuval_: that is my main one.
15:06:32 sean-k-mooney yuval_: gibi listed other issue liek py2 support ectra inline
15:07:15 sean-k-mooney yuval_: so other then the monitor thread and the comments that gibi has already reaied i dont really have other issues with it
15:11:52 bauzas melwitt: gibi: question about InstanceMapping records vs. shelved-offloaded instances and how we quota them
15:12:01 bauzas melwitt: gibi: https://review.opendev.org/c/openstack/nova/+/713301/34/nova/limit/placement.py#80
15:12:42 bauzas iirc, we don't have quota usage for shelved-offloaded instances, right?
15:12:43 sean-k-mooney shelved instance shoudl consume the same quota for disk as running but they should not consume quota for cpu or ram
15:13:12 sean-k-mooney bauzas: it depend on if it BFV really i think
15:13:22 bauzas sean-k-mooney: my question is about the instances/servers quota usage
15:13:38 sean-k-mooney well i t should consume isntance quota
15:14:13 sean-k-mooney that is a limit on the number of non deleted instance you can have in the db
15:14:16 bauzas surely, but I don't remember about the current behaviour
15:14:18 sean-k-mooney its independt fo the state
15:14:34 sean-k-mooney it wont be tracked in placment currently
15:14:42 sean-k-mooney since we dont consume allocations of instance
15:14:47 sean-k-mooney but i guess
15:14:57 sean-k-mooney you coudl count allocation by consumer type maybe
15:15:39 sean-k-mooney when its in shelve offloaded i dont know if we still have any allcoation in placment at all
15:15:51 sean-k-mooney if we had local disk i think we should for storage

Earlier   Later