Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-05-06
14:02:53 dansmith gibi: I'm not sure where we care about process workers, other than the api services in standalone mode.. in wsgi mode we shouldn't be forking at all, right?
14:02:54 gibi we don't spawn much, just at the start to get the worker processes (or when they die)
14:03:31 gibi dansmith: this is scheduler. The default behavior of oslo.service is to fork the workers
14:03:31 dansmith oh I guess conductor needs process workers too
14:03:46 gibi conductor too
14:04:00 dansmith do we need to do that for scheduler though? we did under eventlet for parallelism, but I'm not sure either do going forward
14:05:10 gibi threading in python 3.12 still limited by the GIL, so having a way to spawn processes as scaling make sense as that way we can saturate more CPU cores if needed
14:06:02 dansmith sure, but there are lots of multithreaded python programs providing reasonable performance with the GIL :)
14:06:04 gibi Im not 100% sure but I feel that oslo.service with worker=1 still forks a worker proc from the master proc
14:06:14 dansmith I suspect neither are cpu-intensive enough to really need full parallelism
14:06:46 dansmith okay
14:09:13 gibi but I think I understand you, we might not need to fork just use a single proc with big enough thread pools. I'm not sure this is something that supported by oslo.service out of the box.
14:09:32 gibi could be an improvement request
14:09:49 gibi to provide alternative way to avoid the fork
14:10:22 dansmith yeah, just might be worth consideration
14:11:06 dansmith conductor especially I suspect is mostly waiting on mysql and rabbit connections anyway.. scheduler *might* do enough work traversing lots of host objects or something, but I suspect not anymore with placement
14:14:31 gibi I think we had issues with the Numa filter where I needed to add cacheing to help with the execution time
14:14:34 gibi so meh
14:17:23 opendevreview Merged openstack/osc-placement stable/2025.1: Update TOX_CONSTRAINTS_FILE for stable/2025.1 https://review.opendev.org/c/openstack/osc-placement/+/943758
14:17:59 opendevreview Balazs Gibizer proposed openstack/nova master: Translate scatter-gather to futurist https://review.opendev.org/c/openstack/nova/+/947966
14:18:00 opendevreview Balazs Gibizer proposed openstack/nova master: Use futurist for _get_default_green_pool() https://review.opendev.org/c/openstack/nova/+/948072
14:18:00 opendevreview Balazs Gibizer proposed openstack/nova master: Replace utils.spawn_n with spawn https://review.opendev.org/c/openstack/nova/+/948076
14:18:01 opendevreview Balazs Gibizer proposed openstack/nova master: Add spawn_on https://review.opendev.org/c/openstack/nova/+/948079
14:18:02 opendevreview Balazs Gibizer proposed openstack/nova master: Move ComputeManager to use spawn_on https://review.opendev.org/c/openstack/nova/+/948186
14:18:02 opendevreview Balazs Gibizer proposed openstack/nova master: Move ConductorManager to use spawn_on https://review.opendev.org/c/openstack/nova/+/948187
14:18:03 opendevreview Balazs Gibizer proposed openstack/nova master: Make nova.utils.pass_context private https://review.opendev.org/c/openstack/nova/+/948188
14:18:03 opendevreview Balazs Gibizer proposed openstack/nova master: Rename DEFAULT_GREEN_POOL to DEFAULT_EXECUTOR https://review.opendev.org/c/openstack/nova/+/948086
14:18:04 opendevreview Balazs Gibizer proposed openstack/nova master: Make the default executor configurable https://review.opendev.org/c/openstack/nova/+/948087
14:18:05 opendevreview Balazs Gibizer proposed openstack/nova master: Print ThreadPool statistics https://review.opendev.org/c/openstack/nova/+/948340
14:18:05 opendevreview Balazs Gibizer proposed openstack/nova master: WIP: allow service to start with threading https://review.opendev.org/c/openstack/nova/+/948311
14:18:07 opendevreview Balazs Gibizer proposed openstack/nova master: DNM:Run nova-next with n-sch in threading mode https://review.opendev.org/c/openstack/nova/+/948450
14:18:28 opendevreview Stephen Finucane proposed openstack/nova master: setup: Remove pbr's wsgi_scripts https://review.opendev.org/c/openstack/nova/+/902688
14:18:46 stephenfin sean-k-mooney: gibi: Context for my question earlier ^
14:19:56 gibi dansmith: sean-k-mooney: so I think I fixed the fork issue with re-initing the executor in the worker processes https://review.opendev.org/c/openstack/nova/+/947966/8/nova/utils.py#1276 not nice but seem to work. (I noticed the issue when tested slow / never finishing scatter gatter scenarios locally)
14:21:09 dansmith yeah that's not great
14:21:09 gibi stephenfin: thank I will take a loook
14:21:50 dansmith gibi: is there any way I could get you to stop putting many lines of comments in the middle of a condition like that? seems like a pattern that has been emerging and it completely breaks my brain when trying to reason about the logic like that :(
14:22:14 gibi feel free to leave a comment and I will move the comment
14:22:37 gibi I tried to be as close to the condition as possible but I can move it a bit further
14:28:26 dansmith gibi: I may have missed context from the above conversation about where this gets initialized so early, but is moving that an option?
14:28:49 gibi it is the service version check that happens before the fork
14:28:54 gibi and that uses scatter-gather
14:29:05 gibi (and also any init that happens at Service.create() today)
14:29:27 gibi we can move it but we have no way to signal to the master process that a worker want to kill the master if the version check fails
14:29:38 gibi I need to drop for a bit, I will be back for the nova meeting
14:29:46 dansmith ah, I see
14:30:16 dansmith seems like we could maybe just disable the pooling there, or just destroy the threadpool at the end of service.create before we return to allow the fork to be clean?
14:31:21 gibi disable pooling needs infra as the version check uses the generic scatter-gather. I would not duplicate that
14:31:42 gibi destroying the pool before the fork could work
14:31:51 gibi probably a bit cleaner than the re-init
14:31:54 gibi but
14:32:03 gibi I need to test it
14:32:46 dansmith for the disable pooling suggestion, I meant something like "use a temporary pool instead of the global one" - I know we need *a* pool there
14:33:34 dansmith you could also have a "sequential thread pool" that just runs each cell query synchronously in order instead of the threads maybe too
14:33:46 dansmith but yeah, just destroying after we're done with that check would be simple and clean, I suspect
14:34:11 dansmith I will miss the nova meeting today, btw. see you later
14:36:14 sean-k-mooney gibi: if notthing else it confirm that the issue is with how it forking and not reinitalisting it
14:36:48 sean-k-mooney gibi: dansmith i was on a metting but would markign the pool as thread local help
14:37:25 sean-k-mooney they would not be shared with the process correct. the only problem with that is if work on a thread pool wanted to add somethign to the same thread pool
14:37:56 sean-k-mooney my thinking is we should not have that patter in general
14:38:17 sean-k-mooney so if we mark the module level thread pool at threadlocal then on fork it will be initalsed to empty
14:38:39 sean-k-mooney and the first attempt to use it will init it in the child process without any shareing of state
14:39:24 sean-k-mooney having a way to disable scatter gahter for that first call on startup would also be an option
14:40:40 sean-k-mooney dansmith: by "sequential thread pool" you mean a sequtial executor right. like the one we used in tests
14:41:06 dansmith yes
14:41:55 sean-k-mooney ok futureist already provides that https://github.com/openstack/futurist/blob/master/futurist/_futures.py#L227
14:42:14 sean-k-mooney so that would be an option for the early init
14:50:35 sean-k-mooney gibi: i comment with links to the thread local apporch but i have not tested that change it mich be worth trying with a DNM patch or locally
14:50:51 sean-k-mooney im just not sure if its the correct patthern to apply to this type of problem
14:51:26 sean-k-mooney i.e. can we convert all module global state to thread local storage if we have this problem or would that only work in this case
14:51:54 sean-k-mooney we dotn really want to have to have thread local db engine facades for example
14:54:31 opendevreview Stephen Finucane proposed openstack/placement master: setup: Remove pbr's wsgi_scripts https://review.opendev.org/c/openstack/placement/+/919582
14:54:48 stephenfin sean-k-mooney: Think you can remove your -1 on that now? ^
14:55:22 sean-k-mooney probaly we had that supprot in epoxy right
14:55:35 sean-k-mooney so we should be able to remove it now if we wanted too
14:58:08 stephenfin Yep
14:58:33 sean-k-mooney stephenfin: im still hesitent to remvoe this entrily for the simple reason that apache mod_wsgi does not suport using the module approch supproted by uwsgi/gunicorn
14:58:37 sean-k-mooney with that said
14:58:47 sean-k-mooney can the wsgi module be used directly
14:59:17 sean-k-mooney i.e. can you just poing mod_wsgi at https://github.com/openstack/placement/blob/master/placement/wsgi/api.py
14:59:49 sean-k-mooney that effectivly what was generated by pbr right
15:00:12 sean-k-mooney so for modwsgi you would just point to that in site packages ?
15:02:18 sean-k-mooney ok so not quite https://termbin.com/5n98
15:02:35 sean-k-mooney but for the mod_wsgi usecase the answer is yes
15:02:53 sean-k-mooney care to call that out in the release note?
15:05:34 sean-k-mooney hum
15:05:36 sean-k-mooney python3 -c "import placement.wsgi.api; print(placement.wsgi.api.__file__);"
15:05:50 sean-k-mooney so because we dont card that with if __main__
15:06:24 sean-k-mooney that import actully runs some of the code
15:07:16 sean-k-mooney stephenfin: anyway im not sure if we need to expalin how to locate /opt/stack/placement/placement/wsgi/api.py on the system
15:07:41 sean-k-mooney but instead of /opt/stack/data/venv/bin/placement-api they just need to use ^
15:09:32 sean-k-mooney if this was not devstack that would be in /usr/lib/python3/dist-packages/ or /usr/lib/python3/site-packages/
15:10:42 Uggla Nova meeting in ~50mn
15:49:43 Uggla Nova meeting in ~10mn, time for you to grab a cup of coffee.
15:53:25 gibi If I drink a coffee no then won't sleep until 2 in the morning
15:54:24 gibi sean-k-mooney: the thread local has a problem that if we have a periodic that wants to run a scatter-gather then the thread holding the periodic will see a different scatter-gather pool than the main one
15:55:03 gibi so I will look into the destroy pool before fork idea
15:57:56 sean-k-mooney i think there is a generic hook we can implemnt for that
15:58:10 sean-k-mooney we have some module levle reset functionaltiy that we use for mutable config
15:58:23 sean-k-mooney im thinking of thing like atexit()

Earlier   Later