*** benschubert has quit IRC | 00:56 | |
*** hasebastian has joined #buildstream | 03:16 | |
*** tristan has quit IRC | 03:48 | |
*** traveltissues has joined #buildstream | 04:36 | |
*** tristan has joined #buildstream | 04:41 | |
*** ChanServ sets mode: +o tristan | 04:41 | |
*** hasebastian has quit IRC | 05:30 | |
*** toscalix has joined #buildstream | 05:32 | |
*** toscalix has quit IRC | 05:40 | |
*** traveltissues has quit IRC | 05:42 | |
*** benschubert has joined #buildstream | 07:20 | |
benschubert | Ok, I've got something working for thread workers, I however still have threads lingering on after closing the scheduler, which makes the tests fail :/ Not sure how to debug that. juergbi any idea? | 07:21 |
---|---|---|
benschubert | Also, we used to create a connection per process for casd, do we want to create one per thread now or just use a shared one? | 07:21 |
juergbi | benschubert: lingering threads: we/asyncio should properly join the threads | 07:22 |
juergbi | while this should be fixed, we actually can drop the background thread checks in environments where we don't use forked multiprocessing at all anymore | 07:23 |
benschubert | yeah, I know :/ I've got still 5 threads just after closing the asyncio loop | 07:23 |
benschubert | that's a good point actually | 07:23 |
benschubert | I'll try that tonight | 07:23 |
juergbi | casd connections: ideally we can share a connection, however, we need to verify that the shared gRPC object is actually thread-safe (at least for Python/GIL-protected threads) | 07:24 |
benschubert | good point, I'll investigate | 07:24 |
benschubert | Also, do you think https://gitlab.com/BuildStream/buildstream/-/merge_requests/1967 makes sense in its own? It's definitely needed for the next part | 07:25 |
juergbi | benschubert: please note that we currently use forked multiprocessing also at least in one case outside job subprocesses (cache usage monitor) | 07:25 |
benschubert | which we could move to a thread too right? | 07:25 |
juergbi | I'd hope so | 07:25 |
juergbi | we'll have to see about asyncio interaction | 07:25 |
benschubert | so it might "just" require moving it in the same commit to a thread and we might be able to work with that | 07:25 |
juergbi | I think right now it's not aware of the event loop | 07:25 |
benschubert | we could also put it as part of the event loop actually | 07:26 |
benschubert | (as a "job" | 07:26 |
juergbi | yes, generally sounds sensible, just need to figure out sensible API for that | 07:26 |
juergbi | as right now it's outside the scheduler | 07:26 |
juergbi | should not be a big issue, though | 07:26 |
benschubert | yep | 07:27 |
juergbi | benschubert: !1967: could it make sense to ensure the signal handler is always invoked in the main thread instead? | 07:28 |
juergbi | by blocking relevant signals in all other threads | 07:28 |
juergbi | might be easier to think about overall | 07:28 |
benschubert | signals can't be send to threads no? Only to the main thread AFAIK | 07:29 |
juergbi | there are process as well as thread signals | 07:29 |
juergbi | traditional async signals such as SIGTERM/SIGINT are process signals | 07:29 |
benschubert | oh, and SIGTSTP is a thread signal? | 07:29 |
juergbi | no | 07:29 |
juergbi | also process | 07:29 |
juergbi | these can all be triggered in any thread that doesn't have them blocked | 07:30 |
benschubert | ah so we should block the signals in threads so they don't send it to the main thread? Or am I misunderstanding | 07:30 |
benschubert | ah wait | 07:31 |
juergbi | s/they/the kernel/ | 07:31 |
juergbi | no, also not quite right | 07:31 |
benschubert | "could it make sense to ensure the signal handler is always invoked in the main thread instead?" | 07:31 |
benschubert | -> you mean when the signal happens, ensuring that the handler will be called by the main thread right? | 07:31 |
juergbi | yes | 07:31 |
benschubert | ah that's already done by python | 07:32 |
benschubert | python will always call it in the main thread | 07:32 |
juergbi | ah, ok | 07:32 |
juergbi | I guess it internally blocks the signals in extra threads | 07:32 |
juergbi | or redirects them in the interpreter | 07:32 |
benschubert | which _might_ have some weird consequences for thread-local stuff, I'd need to investigate | 07:32 |
benschubert | yeah, AFAIK, the interpreter catches all signales and then processes them accordingly | 07:32 |
benschubert | ah my MR was poorly phrased, I rephrased to make it clearer | 07:34 |
juergbi | ah, ta | 07:35 |
juergbi | benschubert: I was thinking it could make sense to send signals to each thread to invoke their handlers on their own | 07:38 |
juergbi | however, it seems python doesn't support thread signals at all | 07:38 |
juergbi | running other thread's signal handlers in the main thread is somewhat odd and potentially racy | 07:38 |
benschubert | the control is not given back to the other threads until the signal handler returns | 07:39 |
juergbi | for the low level signal handler that's fine | 07:39 |
juergbi | the real handling is anyway done via raised exception, right? | 07:40 |
benschubert | right | 07:40 |
juergbi | and I'd expect that to allow threading | 07:40 |
juergbi | even the signalling to other threads would probably not need to be in the low level signal handler | 07:40 |
juergbi | however, we need a way to raise an exception in another thread | 07:41 |
juergbi | I'm not familiar with Python threading, so not sure what the best approach is for this | 07:41 |
juergbi | or whether nothing sensible exists | 07:41 |
benschubert | I'll have to dig more, but I think that cancelling the promise will be doing the right thing | 07:41 |
juergbi | a promise for the whole job/thread? | 07:41 |
benschubert | yeah | 07:42 |
juergbi | that might make work when we want to terminate the job/thread | 07:42 |
juergbi | not for suspend handlers, though | 07:42 |
juergbi | but I guess we could centralize those | 07:42 |
benschubert | yeah, we'll definitely have to test that thoroughly | 07:43 |
juergbi | yes | 07:43 |
* tristan missing the fun | 07:44 | |
juergbi | with regards to suspend handlers I would tend towards registering started subprocesses instead of registering arbitrary suspend handlers that will then be executed in another (the main) thread | 07:44 |
juergbi | i.e., higher level | 07:44 |
benschubert | juergbi: we also have cleanup of temp files we set in the signal handlers, so we'd have both? | 07:44 |
benschubert | or have 2 higher level APIs, one for subprocesses, one for files? | 07:45 |
juergbi | benschubert: cleanup of temp files is only for termination, which we may be able to handle via cancellation (which hopefully raises a thread-local exception) | 07:45 |
tristan | I think we might get away with similar context managers we currently have, but handling them differently | 07:45 |
tristan | i.e. for the various different things we need to do for suspension | 07:46 |
tristan | Just have it controlled by the main thread no ? | 07:46 |
benschubert | tristan: suspension is fine, it works in my branch AFAIK :D | 07:46 |
juergbi | well, we/I don't want to execute such thread-specific handlers in the main thread | 07:46 |
tristan | I see | 07:46 |
juergbi | so if we can't get those handlers executed in the job thread, we need a bit of reshuffling | 07:47 |
benschubert | for suspend, the only thing done is to send a SIGSTP to the child group, so that would work from the main thread | 07:47 |
tristan | Indeed that sounds reasonable | 07:47 |
juergbi | benschubert: if promise cancellation works with the thread-local exception, it might be worth checking how that's implemented | 07:47 |
tristan | Well, there are other things I think, for instance pausing of timers | 07:47 |
juergbi | maybe we can trigger arbitrary exceptions also outside cancel | 07:47 |
benschubert | tristan: the pausing of timers should be fine, there is nothing racy there, everything is local :) | 07:48 |
juergbi | that is setup and handled by the main thread for all jobs? | 07:48 |
benschubert | I think so yes | 07:49 |
benschubert | I need to test that more | 07:49 |
benschubert | but from my early tests, things seem to react fine | 07:49 |
tristan | Sounds exciting, I am fwiw close to being ready for a review of the juncle | 07:50 |
juergbi | :) | 07:50 |
benschubert | oh nice | 07:50 |
tristan | I've got all the cases nailed, code simplified compared to what I had, and just need to add the "internal" thing + reference manual updates | 07:50 |
tristan | To be honest I don't expect the code to change much anymore https://gitlab.com/BuildStream/buildstream/-/merge_requests/1901/ | 07:51 |
benschubert | juergbi: I'll fix the 'thread' hcecks to see if I can get everything to work, and then start looking whether the state is fine or now | 07:51 |
benschubert | *or not | 07:51 |
tristan | It's not complete yet, so I'll ping again when it's complete, but in case anyone feels like taking a look, it's up there | 07:51 |
juergbi | benschubert: ok. might asyncio be using a thread pool with an idle timeout? | 07:52 |
juergbi | tristan: I need to focus on another task first but will take a look later | 07:52 |
tristan | juergbi, Yeah, no hurry | 07:52 |
benschubert | I checked that, and they do. So I instead passed my own thread pool, and forced it to close and waited for it instead, didn't change the number of threads... so I have no idea what's going on there | 07:52 |
tristan | However, if we are bent on changing the name 'duplicates', the deadline is really closing in on that | 07:52 |
*** tristan has quit IRC | 08:01 | |
*** santi has joined #buildstream | 08:37 | |
*** samwilson has joined #buildstream | 08:44 | |
*** hasebastian has joined #buildstream | 08:55 | |
*** tristan_ has joined #buildstream | 09:04 | |
*** ChanServ sets mode: +o tristan_ | 09:04 | |
*** hasebastian has quit IRC | 09:38 | |
tristan_ | juergbi, recently cs-shadow (or benschubert ?) did something about creating a "story about how to install BuildStream into a venv"... i.e. a page which lots of documentation needs to refer to in order to explain the parallel install story | 10:41 |
tristan_ | That webpage which is constantly impossible to find, does it really have a location ? | 10:41 |
tristan_ | https://docs.buildstream.build/master/ <-- says: "If you wanna install BuildStream, you've come to the wrong place... go to the website instead !" | 10:41 |
juergbi | tristan_: https://buildstream.build/source_install.html#installing-in-virtual-environments | 10:42 |
juergbi | we definitely need to clean up the installation docs | 10:42 |
tristan_ | Uhhh, so we have source_install.html without any TOC entry ??? | 10:42 |
juergbi | I tend towards putting them back into the git tree, because they are version specific | 10:42 |
tristan_ | And we're accepting patches to a ToC-less page ? | 10:42 |
juergbi | it's linked from here | 10:42 |
juergbi | https://buildstream.build/install.html | 10:43 |
tristan_ | Holy | 10:43 |
tristan_ | Yeah that doesnt count | 10:43 |
juergbi | that's the main install docu | 10:43 |
juergbi | which should be improved, though | 10:43 |
tristan_ | If there is anything in docs.buildstream.build which doesnt have a ToC entry on docs.buildstream.build, it might as well not exist at all | 10:44 |
tristan_ | Like say, our release process documentation, and coding guidelines | 10:44 |
juergbi | it's not on docs.buildstream.build | 10:44 |
tristan_ | Which have been nuked to hell | 10:44 |
tristan_ | Oh it's not | 10:44 |
juergbi | as I mentioned, I'd like to get the install docs back into buildstream git | 10:44 |
tristan_ | Sorry I misread the first link... ok so... this is *all* on the website and doesn't touch buildstream.git at all | 10:45 |
juergbi | which would mean it shows up in the doc TOC | 10:45 |
juergbi | correct | 10:45 |
tristan_ | I see yes sorry | 10:45 |
juergbi | we probably thought it's not version-specific at some point | 10:45 |
juergbi | but it actually is | 10:45 |
tristan_ | We probably want the "Please use these commands to install under these distros X, Y, Z, Windows, OSX" instructions on the website | 10:46 |
tristan_ | And the "If you are a distro packager or a hacker who wants to install from source code" instructions in the reference manual | 10:46 |
juergbi | well, given that that's version-specific as well, I'd probably put that in docs as well | 10:46 |
juergbi | ah, for the fully packaged distro packages, yes, I suppose those instructions could make sense directly only the website | 10:47 |
tristan_ | We need to have a single source of truth, we've failed at that spectacularly | 10:47 |
*** tristan_ has quit IRC | 11:08 | |
*** douglaswinship has joined #buildstream | 12:39 | |
douglaswinship | Damnit, I got disconnected silently. Didn't realise I wasn't still in the group. | 12:40 |
*** hasebastian has joined #buildstream | 14:31 | |
*** tristan_ has joined #buildstream | 14:43 | |
*** ChanServ sets mode: +o tristan_ | 14:43 | |
*** samwilson has quit IRC | 14:57 | |
*** hasebastian has quit IRC | 17:21 | |
*** xjuan has joined #buildstream | 17:36 | |
*** santi has quit IRC | 18:07 | |
*** juergbi has quit IRC | 20:15 | |
*** juergbi has joined #buildstream | 20:16 | |
*** xjuan has quit IRC | 22:36 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!