| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-10-14 | |||
| 19:08:45 | melwitt | I see. thanks | |
| 22:11:48 | sean-k-mooney[m] | it was related to a comment gibi made | |
| 22:14:09 | sean-k-mooney | ... peopel changed my topic | |
| 22:15:00 | sean-k-mooney | melwitt: i was refering to this comment thread https://review.opendev.org/c/openstack/nova/+/905284/comment/ea51fb75_6013c0cd/ | |
| 22:15:53 | sean-k-mooney | im pretty sure that the entire query_wrapper can go away and the timeout logic can move as we only use it for one thing | |
| 22:17:24 | sean-k-mooney | its been a month since i looke at it but i think i was leaning towards puting the timeout here https://github.com/openstack/nova/blob/01b207e50d307a7e7650b5839535fbd3ad40bc1b/nova/context.py#L421 or https://github.com/openstack/nova/blob/01b207e50d307a7e7650b5839535fbd3ad40bc1b/nova/compute/multi_cell_list.py#L262 | |
| 22:17:52 | melwitt | sean-k-mooney: I remember that thread but I didn't realize that today the timeout doesn't work because of no eventlet hub in full uwsgi mode | |
| 22:18:35 | sean-k-mooney | im not sure if that is true or not | |
| 22:19:08 | sean-k-mooney | when run under uwsig today we are monkey patching so i think we have a hub but honestly i have not looked at that in a long time | |
| 22:19:31 | sean-k-mooney | we were not starting the hub at one point | |
| 22:19:47 | sean-k-mooney | but i think matt booth changed that as a bugfix? | |
| 22:20:47 | sean-k-mooney | https://github.com/openstack/nova/commit/3c5e2b0e9fac985294a949852bb8c83d4ed77e04 | |
| 22:21:48 | melwitt | ok well ... if the timeout context manager works then it seems to me the timeout is in the right place in today's code (in the main thread and kills the child threads if the timeout fires). and I'm not making an opinion about where the timeout should go as part of eventlet removal | |
| 22:22:32 | sean-k-mooney | so i htink the issue is the timeout fires on the main thread but does not kill the child threads | |
| 22:23:02 | sean-k-mooney | again i dont currently have the context loaded form gibi's testing but i blieve we need the timeout to fire on the child thread? | |
| 22:23:51 | melwitt | I thought gibi was testing with native threads not eventlet? | |
| 22:24:19 | sean-k-mooney | yes but with this patch we are using native treads too | |
| 22:24:20 | melwitt | yeah, gibi's comment is about the code proposed in your patch | |
| 22:24:34 | sean-k-mooney | right that is what i was brignin up today | |
| 22:24:55 | melwitt | ok, maybe I misunderstood what you said. I thought you said the timeout doesn't work at all today | |
| 22:25:08 | sean-k-mooney | when we remove eventelte we need to ensure the timeout still works for the treads in the tread pool | |
| 22:26:13 | sean-k-mooney | baisicly what we want to have happen is if the timout expires we want the thread to be killed and clean up by the timeout expction when resumed by the kernel | |
| 22:26:29 | melwitt | right. I had thought you were commenting on what's in tree right now. I misunderstood that you were talking about future stuff, my bad | |
| 22:26:50 | sean-k-mooney | by startign the timer in the child thread if we dont get io before it expires then the kernel shoud resume it whtn the timer interupt fires | |
| 22:27:16 | sean-k-mooney | no worries, ya i was talking about the unresolved comments in the current series | |
| 22:27:49 | melwitt | I agree the timer needs to be in the child thread with native threads | |
| 22:27:51 | melwitt | we don't have the ability anyway to kill them from the parent | |
| 22:27:55 | melwitt | *them = native threads | |
| 22:28:02 | melwitt | ok, gotcha. sorry about that | |
| 22:28:57 | sean-k-mooney | right fi we used processes we could but because native thread could be holding locks (even if this is not) it would be unsafe to kill them form the parent even if we coudl do that without hacks | |
| 22:29:48 | melwitt | right | |
| 22:30:54 | sean-k-mooney | the other thing i brought up is https://github.com/openstack/nova/blob/master/nova/console/websocketproxy.py#L169-L172 | |
| 22:31:43 | sean-k-mooney | the nova/cmd moduels for the proxy dont call eventlet but we do import that moduel here https://review.opendev.org/c/openstack/nova/+/904424/13/nova/cmd/standalone/baseproxy.py#29 | |
| 22:32:05 | melwitt | ah, yeah. that one I haven't known what it's for, none of the work I did on the proxy was related to it | |
| 22:32:35 | sean-k-mooney | so it was added by https://github.com/openstack/nova/commit/cb25bc4530323aaa33d5c42eb01f998d463f2106 | |
| 22:33:25 | sean-k-mooney | which is apprently for https://bugs.launchpad.net/nova/+bug/1193031 | |
| 22:33:57 | melwitt | oh, huh. | |
| 22:34:06 | sean-k-mooney | The solution is to either not monkey_patch for /cmd/nova-novncproxy and do the rpc call that validates the token with the consolauth service in a separate process that is monkey_patched (the rpc won't work without it), or to make openstack/common/rpc/amqp/multicall not use green-threads for this particular call (ie. block the caller). | |
| 22:34:19 | sean-k-mooney | so consolauth is dead | |
| 22:34:26 | sean-k-mooney | and we are trying to not monkeypatch | |
| 22:34:39 | sean-k-mooney | so maybe we just delete that and we are good? | |
| 22:35:50 | melwitt | delete all of consoleauth? sure, what could go wrong | |
| 22:36:03 | sean-k-mooney | i ment the hub | |
| 22:36:12 | sean-k-mooney | but also didnt you remove the console auth service | |
| 22:36:19 | sean-k-mooney | we now do auth in the cell db | |
| 22:36:47 | melwitt | oh, the console auth service, yeah that is no longer used | |
| 22:37:51 | melwitt | for some reason I was thinking the db call but yeah that doesn't use rpc | |
| 22:38:24 | sean-k-mooney | well even if it did we could deal with this differently | |
| 22:38:45 | melwitt | oh wait, there is an rpc call for "validating the console port | |
| 22:39:50 | melwitt | https://github.com/openstack/nova/blob/741e1b38705efecc083f091c51164b3cafb9697c/nova/console/websocketproxy.py#L133 | |
| 22:40:00 | melwitt | that's the only one AFAICT | |
| 22:40:22 | sean-k-mooney | so we are changing the oslo messaging executor to threading | |
| 22:40:42 | sean-k-mooney | so im not sure it that will be a problem or not but ok this is somethign we need to test | |
| 22:42:32 | melwitt | ok, yeah. I would think it should work fine if you delete the hub at the same time as the switch to native threads in oslo.messaging. I agree, definitely something to test | |
| 22:43:53 | sean-k-mooney | that is tomorrows problem o/ | |
| 22:44:20 | melwitt | yes o/ | |
| 23:14:16 | sean-k-mooney[m] | so i reinstalled my home openstack and entirly redidi my sotrage to use nova lvm instead of cinder lvm, most of the speedu is form other change but i wend form about an 80 minute devstack install to about 21 minutes | |
| 23:15:02 | sean-k-mooney[m] | im hoping with a liitlme more turning to shave another miniute or so but thats closer to the speed i would expect on this host | |
| 23:15:53 | sean-k-mooney[m] | i used to reliable be able to get a fresh vm deployed in about 18-20 mins so that pretty close already | |
| 23:19:52 | sean-k-mooney[m] | although there isnt much standing out as a bottle neck to be honest although i think i can speed up the disk io slightly more. beyond that i would likely need a local mirror/cache but the install time for package is already pretty good https://paste.opendev.org/show/825791/ | |
| #openstack-nova - 2024-10-15 | |||
| 01:40:49 | opendevreview | Takashi Kajinami proposed openstack/placement master: Drop db migration tool https://review.opendev.org/c/openstack/placement/+/932324 | |
| 07:02:46 | opendevreview | Balazs Gibizer proposed openstack/nova stable/2024.2: Revert "Test live migration between hosts with differnet cpu_shared_sets" https://review.opendev.org/c/openstack/nova/+/932379 | |
| 07:17:07 | gibi | sean-k-mooney[m]: melwitt: I skimmed the discussion above above about the threading in scatter gather. Yes, my comment about the timer was only about the proposed native thread based code. | |
| 07:19:37 | gibi | also please note in native threading there is no such thing that having a timer in an existing thread. The timer itself is always a seperate thread (with basically a sleep and a callback). So if we want a threading.timer based solution to signal an existing thread we need a communication channel between the timer thread that runs our callback on timeout and the existing thread that needs the timer | |
| 07:19:43 | gibi | signal. The most simple thing is to share an Event between the two threads. All the solution needs the existing thread to monitor some channel (e.g. the Event) periodically. | |
| 07:31:19 | sean-k-mooney[m] | an event wont work as far as im aware since the event wont raise an excpetion unless we check it | |
| 07:31:53 | sean-k-mooney[m] | also isint the time just a syscall to the OS to have the OS signal the thread after a timer interupt fires | |
| 07:36:15 | sean-k-mooney[m] | hum, maybe a https://docs.python.org/3/library/threading.html#threading.Barrier is closer to what we want | |
| 07:36:39 | sean-k-mooney[m] | the main issue is we dont really have a concpet of a frestandign cancelable task | |
| 07:36:46 | sean-k-mooney[m] | that what we really want | |
| 07:45:44 | opendevreview | Michael Still proposed openstack/nova-specs master: Repropose spice-direct console support. https://review.opendev.org/c/openstack/nova-specs/+/932387 | |
| 07:46:31 | sean-k-mooney[m] | gibi: the first example here https://www.geeksforgeeks.org/python-different-ways-to-kill-a-thread/ could be adapted for our use | |
| 07:47:31 | sean-k-mooney[m] | we can wrap the function into a calss with an overloaded call operator and add a function to raise the excption | |
| 07:47:57 | sean-k-mooney[m] | then we can use the current timeout code to invoke that function if it expires | |
| 07:54:03 | sean-k-mooney[m] | ill see if i can come up with a restandign example of that and we can see what we think of the approch | |
| 09:29:49 | gibi | sean-k-mooney[m]: threading.Timer is a thread not an OS signal based construct https://github.com/python/cpython/blob/546dddca43a2a69dbe33d230e9e540636b403270/Lib/threading.py#L1318-L1343 | |
| 09:30:18 | sean-k-mooney | gibi: ya i read the docs on it after | |
| 09:30:24 | sean-k-mooney | its a subclass of thread | |
| 09:30:25 | gibi | sean-k-mooney[m]: python has signal handling if we want but that require special care as signal handlers can only run in the main thread and can be tricky to write them properly | |
| 09:30:51 | sean-k-mooney | no im in two minds | |
| 09:31:03 | gibi | https://docs.python.org/3/library/signal.html#signal.SIGALRM | |
| 09:31:04 | sean-k-mooney | part of me wonders if the current approch is corect | |
| 09:31:21 | sean-k-mooney | i.e. are we trying to handle this in too generic a way | |
| 09:31:39 | sean-k-mooney | the only reason this timeout exist sis to timeout the db query to the cell db | |
| 09:31:56 | sean-k-mooney | so im partly wonderign if sqlachemy has a native way to do that | |
| 09:32:35 | gibi | yeah, we can step back and look at the current use case to see if we can reformulate it | |
| 09:32:37 | sean-k-mooney | on the other hand if we dont and we want a generic killable way to execute a task a process is better for that | |
| 09:33:27 | gibi | what we want is a to limit the API response time which a) is limited by the web server (haproxy) already I guess and b) avoid stuck threads waiting for a result that never arrives. | |
| 09:35:25 | sean-k-mooney | yep we have a few options, create a task obejct that will execute an arbitry funciton but kill itself after a timeout, run the function in a process and kill it outselves, convert it to a asyncio corutine and cancel it after a timeout or use somethign in sqlacment to kill it after a timeout at the low level | |
| 09:39:18 | gibi | Do we know how many places in nova we rely on the eventlet in process timer? If it is just 2-3 places then it make sense to do a specific solution for each, if it is more than 5 then I would vote for a creation of a cancellable task primitive and use that everywhere | |
| 09:40:21 | sean-k-mooney | thats a good question. This query_wrapper function is only used in one place i belive | |
| 09:40:31 | sean-k-mooney | im not sure if we have this patterin in ohter however | |
| 09:41:14 | sean-k-mooney | im currently trying to play with creating a BoundTask class to do that | |
| 09:41:38 | sean-k-mooney | maybe TimerTask naming is hard | |
| 09:41:40 | gibi | I guess we have timers in the compute for e.g. vif_plug events | |
| 09:42:15 | sean-k-mooney | yes but it depend on how they are used | |
| 09:42:33 | sean-k-mooney | what i want to do and other can disagree | |
| 09:42:51 | sean-k-mooney | is have our main loop handel pooling for RPC messages | |
| 09:43:02 | sean-k-mooney | but dispatch the message handler to a worker thread pool | |
| 09:43:36 | sean-k-mooney | so basiclaly each RPC handlelr woudl be spawned into its own thread which it can freely block on | |