| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-05-06 | |||
| 12:51:08 | sean-k-mooney | im not sure why the combined one could not work however | |
| 12:51:13 | stephenfin | There's no combined one | |
| 12:51:24 | stephenfin | Not in WSGI. Only eventlet (now removed) | |
| 12:51:25 | sean-k-mooney | oh then that would be why i guess | |
| 12:51:31 | sean-k-mooney | ack | |
| 12:52:08 | gibi | yepp the combined one was eventlet only | |
| 12:52:40 | gibi | in the nova-operator we always start the metadata wsgi as separate pod either on top or on cell level | |
| 12:53:13 | sean-k-mooney | ok i guess we were just tryign to say if you want to run per cell metadata you need to deploy addtional metadata api (at least 1 per cell) | |
| 12:53:27 | stephenfin | gibi: tbc, when you say on top or cell-level you're refererring to the global and local deployment topologies described in that doc, yeah? | |
| 12:53:36 | gibi | yeapp | |
| 12:53:39 | sean-k-mooney | yep | |
| 12:53:42 | stephenfin | okay, sweet | |
| 12:53:57 | gibi | stephenfin: do you see a need for clarifying things in our doc? | |
| 12:54:18 | stephenfin | I think so. The sentence I quoted above is confusing IMO | |
| 12:54:21 | stephenfin | > The nova metadata API service must not be run as a standalone service, using the nova-metadata-wsgi service, in this case. | |
| 12:54:47 | stephenfin | To me, that says "you cannot use nova-metadata-wsgi" service if deploying in a global configuration | |
| 12:55:06 | sean-k-mooney | that was not ture even before we remvoe the combined one | |
| 12:55:10 | sean-k-mooney | but now it can be simplifed | |
| 12:55:16 | stephenfin | and since we no longer provide nova-metadata (i.e. the eventlet one), that would suggest global deployments weren't possible anymore | |
| 12:55:20 | sean-k-mooney | we only have the wsgi service on master | |
| 12:55:25 | sean-k-mooney | and only the split endpoints | |
| 12:55:27 | stephenfin | yeah, hence my concern | |
| 12:55:37 | stephenfin | but it sounds like it's just worded weirdly | |
| 12:55:39 | opendevreview | Merged openstack/osc-placement stable/2025.1: Add bindep.txt for ubunutu 24.04 support https://review.opendev.org/c/openstack/osc-placement/+/948867 | |
| 12:55:48 | sean-k-mooney | we used to have seperate console script for nova-api and nvoa-api-metadata | |
| 12:55:51 | stephenfin | and I can rephrase like so | |
| 12:56:07 | sean-k-mooney | i think gibi dropped the refences to the console scripts when they were remvoed | |
| 12:56:28 | stephenfin | > The ``api.local_metadata_per_cell`` option must be set to ``False`` | |
| 12:56:31 | sean-k-mooney | but keepd the verbage related to the pbr wsgi scripts | |
| 12:56:35 | stephenfin | Or drop the sentence entirely | |
| 12:56:40 | gibi | yeah I tried to cleanup the doc when the eventlet server is removed but probably missed things | |
| 12:57:02 | sean-k-mooney | it need to be set based on teh toplogy | |
| 12:57:55 | gibi | yeah, if the metadata is deployed globally (top level) the local_metadata_per_cell needs to be false, but when deployed metadata to each cell then it need to be true | |
| 12:58:15 | sean-k-mooney | it tecnically optiona to set this to true for the per cell deployment by the way. it default to false | |
| 12:58:37 | sean-k-mooney | meanign if you deploy per cell metadata they can lookup metdata for other cells if they have access to the api db to do so | |
| 12:58:47 | gibi | sean-k-mooney: it depends, the the local metadata has cell db access then sure it can be set to false | |
| 12:58:57 | gibi | yeah that | |
| 12:59:28 | gibi | sean-k-mooney: cotyledon has the worker respawn implemented here https://github.com/sileht/cotyledon/blob/be444189de32a8c29c7107a9b02da44248a7e64a/cotyledon/_service_manager.py#L254-L256 | |
| 12:59:39 | sean-k-mooney | https://docs.openstack.org/nova/latest/configuration/config.html#api.local_metadata_per_cell is basically a configuration to prevnet that cross cell lookup | |
| 13:00:08 | gibi | yepp | |
| 13:14:09 | gibi | option b) aka re-initing the executor after fork works, based on the process name stored on the executor compared to the current process name. But this is ugly and nova only solution. Fortunately we will move all our spawns to executors so at least it applicable easily for now | |
| 13:38:46 | opendevreview | Merged openstack/osc-placement stable/2025.1: Update .gitreview for stable/2025.1 https://review.opendev.org/c/openstack/osc-placement/+/943757 | |
| 14:02:24 | dansmith | gibi: that move to spawn soon is surprising to me, as I suspect it will be hella slow, especially for openstack code | |
| 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 | |