IRC logs for #buildstream for Thursday, 2020-06-18

*** benschubert has quit IRC00:56
*** hasebastian has joined #buildstream03:16
*** tristan has quit IRC03:48
*** traveltissues has joined #buildstream04:36
*** tristan has joined #buildstream04:41
*** ChanServ sets mode: +o tristan04:41
*** hasebastian has quit IRC05:30
*** toscalix has joined #buildstream05:32
*** toscalix has quit IRC05:40
*** traveltissues has quit IRC05:42
*** benschubert has joined #buildstream07:20
benschubertOk, 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
benschubertAlso, 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
juergbibenschubert: lingering threads: we/asyncio should properly join the threads07:22
juergbiwhile this should be fixed, we actually can drop the background thread checks in environments where we don't use forked multiprocessing at all anymore07:23
benschubertyeah, I know :/ I've got still 5 threads just after closing the asyncio loop07:23
benschubertthat's a good point actually07:23
benschubertI'll try that tonight07:23
juergbicasd 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
benschubertgood point, I'll investigate07:24
benschubertAlso, do you think https://gitlab.com/BuildStream/buildstream/-/merge_requests/1967 makes sense in its own? It's definitely needed for the next part07:25
juergbibenschubert: please note that we currently use forked multiprocessing also at least in one case outside job subprocesses (cache usage monitor)07:25
benschubertwhich we could move to a thread too right?07:25
juergbiI'd hope so07:25
juergbiwe'll have to see about asyncio interaction07:25
benschubertso it might "just" require moving it in the same commit to a thread and we might be able to work with that07:25
juergbiI think right now it's not aware of the event loop07:25
benschubertwe could also put it as part of the event loop actually07:26
benschubert(as a "job"07:26
juergbiyes, generally sounds sensible, just need to figure out sensible API for that07:26
juergbias right now it's outside the scheduler07:26
juergbishould not be a big issue, though07:26
benschubertyep07:27
juergbibenschubert: !1967: could it make sense to ensure the signal handler is always invoked in the main thread instead?07:28
juergbiby blocking relevant signals in all other threads07:28
juergbimight be easier to think about overall07:28
benschubertsignals can't be send to threads no? Only to the main thread AFAIK07:29
juergbithere are process as well as thread signals07:29
juergbitraditional async signals such as SIGTERM/SIGINT are process signals07:29
benschubertoh, and SIGTSTP is a thread signal?07:29
juergbino07:29
juergbialso process07:29
juergbithese can all be triggered in any thread that doesn't have them blocked07:30
benschubertah so we should block the signals in threads so they don't send it to the main thread? Or am I misunderstanding07:30
benschubertah wait07:31
juergbis/they/the kernel/07:31
juergbino, also not quite right07: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
juergbiyes07:31
benschubertah that's already done by python07:32
benschubertpython will always call it in the main thread07:32
juergbiah, ok07:32
juergbiI guess it internally blocks the signals in extra threads07:32
juergbior redirects them in the interpreter07:32
benschubertwhich _might_ have some weird consequences for thread-local stuff, I'd need to investigate07:32
benschubertyeah, AFAIK, the interpreter catches all signales and then processes them accordingly07:32
benschubertah my MR was poorly phrased, I rephrased to make it clearer07:34
juergbiah, ta07:35
juergbibenschubert: I was thinking it could make sense to send signals to each thread to invoke their handlers on their own07:38
juergbihowever, it seems python doesn't support thread signals at all07:38
juergbirunning other thread's signal handlers in the main thread is somewhat odd and potentially racy07:38
benschubertthe control is not given back to the other threads until the signal handler returns07:39
juergbifor the low level signal handler that's fine07:39
juergbithe real handling is anyway done via raised exception, right?07:40
benschubertright07:40
juergbiand I'd expect that to allow threading07:40
juergbieven the signalling to other threads would probably not need to be in the low level signal handler07:40
juergbihowever, we need a way to raise an exception in another thread07:41
juergbiI'm not familiar with Python threading, so not sure what the best approach is for this07:41
juergbior whether nothing sensible exists07:41
benschubertI'll have to dig more, but I think that cancelling the promise will be doing the right thing07:41
juergbia promise for the whole job/thread?07:41
benschubertyeah07:42
juergbithat might make work when we want to terminate the job/thread07:42
juergbinot for suspend handlers, though07:42
juergbibut I guess we could centralize those07:42
benschubertyeah, we'll definitely have to test that thoroughly07:43
juergbiyes07:43
* tristan missing the fun07:44
juergbiwith 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) thread07:44
juergbii.e., higher level07:44
benschubertjuergbi: we also have cleanup of temp files we set in the signal handlers, so we'd have both?07:44
benschubertor have 2 higher level APIs, one for subprocesses, one for files?07:45
juergbibenschubert: 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
tristanI think we might get away with similar context managers we currently have, but handling them differently07:45
tristani.e. for the various different things we need to do for suspension07:46
tristanJust have it controlled by the main thread no ?07:46
benschuberttristan: suspension is fine, it works in my branch AFAIK :D07:46
juergbiwell, we/I don't want to execute such thread-specific handlers in the main thread07:46
tristanI see07:46
juergbiso if we can't get those handlers executed in the job thread, we need a bit of reshuffling07:47
benschubertfor suspend, the only thing done is to send a SIGSTP to the child group, so that would work from the main thread07:47
tristanIndeed that sounds reasonable07:47
juergbibenschubert: if promise cancellation works with the thread-local exception, it might be worth checking how that's implemented07:47
tristanWell, there are other things I think, for instance pausing of timers07:47
juergbimaybe we can trigger arbitrary exceptions also outside cancel07:47
benschuberttristan: the pausing of timers should be fine, there is nothing racy there, everything is local :)07:48
juergbithat is setup and handled by the main thread for all jobs?07:48
benschubertI think so yes07:49
benschubertI need to test that more07:49
benschubertbut from my early tests, things seem to react fine07:49
tristanSounds exciting, I am fwiw close to being ready for a review of the juncle07:50
juergbi:)07:50
benschubertoh nice07:50
tristanI've got all the cases nailed, code simplified compared to what I had, and just need to add the "internal" thing + reference manual updates07:50
tristanTo be honest I don't expect the code to change much anymore https://gitlab.com/BuildStream/buildstream/-/merge_requests/1901/07:51
benschubertjuergbi: 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 now07:51
benschubert*or not07:51
tristanIt'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 there07:51
juergbibenschubert: ok. might asyncio be using a thread pool with an idle timeout?07:52
juergbitristan: I need to focus on another task first but will take a look later07:52
tristanjuergbi, Yeah, no hurry07:52
benschubertI 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 there07:52
tristanHowever, if we are bent on changing the name 'duplicates', the deadline is really closing in on that07:52
*** tristan has quit IRC08:01
*** santi has joined #buildstream08:37
*** samwilson has joined #buildstream08:44
*** hasebastian has joined #buildstream08:55
*** tristan_ has joined #buildstream09:04
*** ChanServ sets mode: +o tristan_09:04
*** hasebastian has quit IRC09: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 story10: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
juergbitristan_: https://buildstream.build/source_install.html#installing-in-virtual-environments10:42
juergbiwe definitely need to clean up the installation docs10:42
tristan_Uhhh, so we have source_install.html without any TOC entry ???10:42
juergbiI tend towards putting them back into the git tree, because they are version specific10:42
tristan_And we're accepting patches to a ToC-less page ?10:42
juergbiit's linked from here10:42
juergbihttps://buildstream.build/install.html10:43
tristan_Holy10:43
tristan_Yeah that doesnt count10:43
juergbithat's the main install docu10:43
juergbiwhich should be improved, though10: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 all10:44
tristan_Like say, our release process documentation, and coding guidelines10:44
juergbiit's not on docs.buildstream.build10:44
tristan_Which have been nuked to hell10:44
tristan_Oh it's not10:44
juergbias I mentioned, I'd like to get the install docs back into buildstream git10:44
tristan_Sorry I misread the first link... ok so... this is *all* on the website and doesn't touch buildstream.git at all10:45
juergbiwhich would mean it shows up in the doc TOC10:45
juergbicorrect10:45
tristan_I see yes sorry10:45
juergbiwe probably thought it's not version-specific at some point10:45
juergbibut it actually is10:45
tristan_We probably want the "Please use these commands to install under these distros X, Y, Z, Windows, OSX" instructions on the website10:46
tristan_And the "If you are a distro packager or a hacker who wants to install from source code" instructions in the reference manual10:46
juergbiwell, given that that's version-specific as well, I'd probably put that in docs as well10:46
juergbiah, for the fully packaged distro packages, yes, I suppose those instructions could make sense directly only the website10:47
tristan_We need to have a single source of truth, we've failed at that spectacularly10:47
*** tristan_ has quit IRC11:08
*** douglaswinship has joined #buildstream12:39
douglaswinshipDamnit, I got disconnected silently. Didn't realise I wasn't still in the group.12:40
*** hasebastian has joined #buildstream14:31
*** tristan_ has joined #buildstream14:43
*** ChanServ sets mode: +o tristan_14:43
*** samwilson has quit IRC14:57
*** hasebastian has quit IRC17:21
*** xjuan has joined #buildstream17:36
*** santi has quit IRC18:07
*** juergbi has quit IRC20:15
*** juergbi has joined #buildstream20:16
*** xjuan has quit IRC22:36

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!