Earlier  
Posted Nick Remark
#openstack-nova - 2022-02-17
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
15:16:00 sean-k-mooney e.g. if you were usign the rbd image backend
15:16:15 sean-k-mooney but sicne we dont have a host it might not be there
15:16:46 sean-k-mooney melwitt: how does that work with unified limits?
15:17:22 bauzas sean-k-mooney: look at the change
15:17:33 bauzas sean-k-mooney: there is a TODO for consumer types
15:17:44 bauzas sean-k-mooney: for the moment, we look at instance mappings
15:17:48 bauzas hence my question
15:17:51 sean-k-mooney right but consuemr types is not enough
15:18:08 sean-k-mooney a shelve offloaded instance does not have a host
15:18:23 sean-k-mooney so it cant have allcoation against an rp today
15:18:46 sean-k-mooney since the only resouce usage it has are floating ips, and disk really
15:19:32 sean-k-mooney so without creating a RP per compute with inventories of instance,floating ip ectra we cant tack the useage in palcment
15:19:52 sean-k-mooney so we woudl have to tack this in keystone?
15:20:25 bauzas I don't know, we haven't discussed it in https://specs.openstack.org/openstack/nova-specs/specs/yoga/approved/unified-limits-nova.html
15:20:40 bauzas tbc, I don't want to trample this series because of this question
15:20:55 sean-k-mooney well for now we can just count it in nova
15:21:05 bauzas my only concern is about making sure we continue to have the same behaviour from the before
15:21:53 sean-k-mooney right so we shoudl continue to count instnace using the instance cell mapings in the nova api db proably for yoga
15:22:38 sean-k-mooney in Zed we could eitehr create an RP per porject with invetoires of diffent quata type like instance or tack it somewhere else
15:23:03 sean-k-mooney the question really is should you be able to get all usage just by looking at placment or not
15:25:29 sean-k-mooney bauzas: ""All the resource class usage will be counted using placement, but server count will make use of instance mappings.""

Earlier   Later