| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-03-19 | |||
| 15:23:21 | bogdando | (if we got unlucky enough) | |
| 15:23:25 | sean-k-mooney | and if we do the 500 thing it wont either | |
| 15:23:30 | dansmith | it's not going to operate half-configured either way if we never return application | |
| 15:23:32 | dansmith | right | |
| 15:23:33 | bogdando | ok, makes sense | |
| 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 | |