Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-03-19
15:23:46 bogdando what about a tail of rpc calls?
15:24:00 dansmith not sure what you mean
15:24:08 bogdando it won't accept new API requests, but what about a pending work
15:24:24 sean-k-mooney it gets dropped on the floor
15:24:25 bogdando woudln't it heppily pick it up after crash looping?
15:24:27 bogdando ok
15:24:39 sean-k-mooney the http request wont get into the applciation logic and it will get a 500
15:24:54 opendevreview ribaudr proposed openstack/nova master: FUP Remove unnecessary PCI check https://review.opendev.org/c/openstack/nova/+/944105
15:24:55 opendevreview ribaudr proposed openstack/nova master: FUP improve and add integration tests for PCI SR-IOV servers https://review.opendev.org/c/openstack/nova/+/944106
15:24:55 opendevreview ribaudr proposed openstack/nova master: FUP Add a warning to make non-explicit live migration request debugging easier https://review.opendev.org/c/openstack/nova/+/944133
15:24:56 opendevreview ribaudr proposed openstack/nova master: FUP Update pci-passthrough and virtual-gpu documentation https://review.opendev.org/c/openstack/nova/+/944153
15:26:39 bogdando IIRC, there is a logic for recovering ongoing rpc calls (long running ops) after restarting the process. Wouldn't restarting half-configured introduce more problems that it solves, even if never taking new API requests?
15:26:56 sean-k-mooney no
15:27:21 sean-k-mooney config errors should not allow the api to ever come up
15:27:27 dansmith if we never return application, we can never start rpc calls
15:27:28 sean-k-mooney so you shoudl nto be able to start rpcs
15:27:29 dansmith I'm not sure what the concern is
15:27:49 bogdando oh ok then, sorry
15:27:51 opendevreview ribaudr proposed openstack/nova master: FUP Remove unnecessary PCI check https://review.opendev.org/c/openstack/nova/+/944105
15:27:52 opendevreview ribaudr proposed openstack/nova master: FUP improve comment accuracy and variable naming for tag removal https://review.opendev.org/c/openstack/nova/+/943124
15:27:52 opendevreview ribaudr proposed openstack/nova master: FUP Add a warning to make non-explicit live migration request debugging easier https://review.opendev.org/c/openstack/nova/+/944133
15:27:53 sean-k-mooney also if you restart the api and it has done an rpc the reply is dropped on the floor
15:27:53 opendevreview ribaudr proposed openstack/nova master: FUP Update pci-passthrough and virtual-gpu documentation https://review.opendev.org/c/openstack/nova/+/944153
15:27:53 opendevreview ribaudr proposed openstack/nova master: FUP improve and add integration tests for PCI SR-IOV servers https://review.opendev.org/c/openstack/nova/+/944106
15:28:03 dansmith also, the 500 approach specifically avoids restarting if we know we're not going to be able to do so successfull
15:29:13 sean-k-mooney it will be about 2 hours until the ci job finishes
15:29:27 sean-k-mooney ill see if i can do a poc of the 500 approch this evenign or early tomorrow
15:29:47 sean-k-mooney i mean i can push one now
15:29:59 bogdando what about conductor, it service no API, right? but we cannot afford it restarted half-configured for some moment? sorry I am fully noob in Nova services architecture
15:30:00 sean-k-mooney but i guess i can triger a job via a second dnm change
15:30:11 sean-k-mooney the conductor is not a wsig process
15:30:11 bogdando it serves no api*
15:30:18 sean-k-mooney this si only a problem because of mod_wsgi
15:30:24 bogdando oh yes, indeed
15:30:25 sean-k-mooney all the rest exit on config error
15:30:27 dansmith yeah I'm not sure what conductor has to do with this.. it can use the usual systemd stuff
15:30:35 bogdando not that much a noob to grasp that :)
16:28:59 sean-k-mooney dansmith: i have a poc locally but i dont think we want to keep raising the excption and returning 500 in all cases. for config yes but for DBConnectionError maybe not.
16:29:22 sean-k-mooney dansmith: ill push what i have shortly and we can dicuss if we want to filter ot a subset of excptions
16:29:27 dansmith sean-k-mooney: yep, that's fair.. I would default to 500, excluding specific things we know are transient
16:29:37 sean-k-mooney ack
16:29:43 dansmith anything unexpected should be assumed to be fatal
16:31:05 bogdando you see, arguing more with the door gouard? :)
16:31:22 bogdando fail early is always the best approach
16:31:39 sean-k-mooney so today we allow retrying in case teh db is not ready yet
16:31:49 sean-k-mooney so it would be a regression to hard fail in that case
16:31:56 sean-k-mooney a small one but still a chagne in behvior
16:32:05 bogdando well, systemd or k8s or pidone shall restart
16:32:11 sean-k-mooney it might
16:32:14 sean-k-mooney it might not
16:32:25 bogdando I mean it is a normal assumption for underlying OS to expect that
16:32:32 sean-k-mooney we can change the behavior we jsut need to conservitive and advertise it peroperly ot operators
16:32:56 dansmith compute tolerates certain things on startup and not others, so this mirrors that
16:33:09 dansmith compute also handles conductor not started, waits for it, etc
16:33:34 sean-k-mooney i am raising this because im modifying a test that emulatest this and im liek "i proably shoudl not be changing this test"
16:34:32 bogdando then it seems we only have one choice, code 500 with tricky logic around
16:35:01 dansmith how is it tricky logic?
16:35:01 sean-k-mooney no we can hard exit just not on any excption
16:35:07 sean-k-mooney config errors are fine
16:35:16 sean-k-mooney i jsut cant do excpet Excpetion
16:35:21 sean-k-mooney which is what my current patch does
16:35:25 bogdando ack
16:36:16 sean-k-mooney if the genal approch looks ok to folks after i push it ill refactor it to be more selective
16:38:26 bogdando @dansmith the trickyness that I meant is how we have started firstly with a simple return http 500, and this evolved into decisions making
16:39:05 dansmith this is not tricky, it's a single "try...except $allowed: pass except Exception: fail"
16:39:37 sean-k-mooney bogdando: its more we cant regress https://launchpad.net/bugs/1882094 while fixing this
16:41:02 sean-k-mooney while i can change the test case we added to prevent that regressing there are trade offs and if i can aovid changing the test i likely shoudl
16:41:32 bogdando what happens first, oslo config validation or DB connection during wsgi app init?
16:41:49 sean-k-mooney config
16:41:59 sean-k-mooney so that it can know what db to connect too
16:42:05 bogdando I recall there were corner cases with logs eating some events because errors happened before the logs configure
16:44:58 bogdando sean-k-mooney: while that is totally logical, that was not clear to me after reading https://bugs.launchpad.net/nova/+bug/1882094
16:45:49 bogdando that "Removing and moving the DB connection establishment out of our WSGI init script" could be understood like it connects to DB even before config gets validated
16:46:02 dansmith of course config :)
16:46:20 sean-k-mooney bogdando: it could but that not what the code does
16:47:29 bogdando well, sometimes it firstly needs alive MQ connection before to start verifying something else in a config
16:47:43 bogdando not saying that's exactly a case here
16:47:44 sean-k-mooney the api does not
16:48:09 sean-k-mooney so this call to init_global_data is waht calls config.parse_args https://github.com/openstack/nova/blob/master/nova/api/openstack/wsgi_app.py#L128
16:48:18 sean-k-mooney here https://github.com/openstack/nova/blob/master/nova/api/openstack/wsgi_app.py#L101
16:49:17 sean-k-mooney the first time the db is acccesed is here https://github.com/openstack/nova/blob/master/nova/api/openstack/wsgi_app.py#L64
16:49:54 sean-k-mooney which is after we return from init_global_data
16:50:19 sean-k-mooney dansmith: by hte way this is why we are not getting the config error the second time https://github.com/openstack/nova/blob/master/nova/api/openstack/wsgi_app.py#L92-L93
16:50:47 bogdando at least my silly question brought some useful considerations
16:50:47 dansmith sean-k-mooney: ah interesting :)
16:53:06 sean-k-mooney i could proably tweak run_once for this edge case too but im slighly reluctant to broaden the scope of that
16:53:23 bogdando @sean-k-mooney so, that means we can hit that initialization again with an invalid config
16:53:32 bogdando a rogue database connection :)
16:53:51 sean-k-mooney bogdando: yes but rememebr python is ment to die if you dont cache the excption
16:54:02 sean-k-mooney mod_wsgi is the thing that is breaking expections :)
16:54:14 bogdando my point is now this try catch becomes even more nuanced
16:55:24 sean-k-mooney that run_once was added for the orginal bug
16:55:46 bogdando yea, mod_wsgi is the thing that is breaking excEPtions
16:55:48 sean-k-mooney it was there to make sure we dint try and re parse the config amoung other things if it succeed the first time
16:55:51 sean-k-mooney becuase that is an error
16:57:27 sean-k-mooney bogdando: extra context we have shipt the run_once approch for 5 years and backported it to all our downstream release that are still supproted.
16:57:56 sean-k-mooney so while we can condier changing this we have several years fo experince that this work good enough
16:58:09 sean-k-mooney so im trying to be tactical without createing more tech debt
17:00:27 opendevreview sean mooney proposed openstack/nova master: [WIP] raise_forever on wsgi app init https://review.opendev.org/c/openstack/nova/+/945028
17:00:34 bogdando puppet used to be much more restrictive on configs management

Earlier   Later