IRC logs for #buildstream for Thursday, 2020-04-30

*** tristan has quit IRC03:03
*** traveltissues has quit IRC05:41
*** traveltissues has joined #buildstream05:42
*** slaf has quit IRC05:44
*** slaf has joined #buildstream05:52
*** tristan has joined #buildstream05:53
*** ChanServ sets mode: +o tristan05:53
*** benschubert has joined #buildstream07:28
benschubertjuergbi: hey! https://gitlab.com/BuildStream/buildstream/-/merge_requests/1889#note_333881011 made me think, when we designed the timed activity didn't we want it to be shown only after 1-5s if the process takes more than that instead of immediately? If so, we could "just" fix that part and we should not have any problem anymore in this MR07:29
tristanUmmmm07:30
juergbihi benschubert, I don't remember any plan of doing this for all timed activities07:30
juergbiit could make sense but it might also be confusing if user looks at the log and doesn't know why something is missing07:30
tristantimed activities as I recall are always explicitly intended to show the start/end time of a given thing07:30
juergbifor the 'waiting for casd' case probably not an issue but maybe for others07:30
tristanand we do status messages which are not shown by default unless you ask for --verbose07:30
juergbiso it could be a flag to timed activity, I suppose?07:30
benschubertOk, I might be mis-remembering07:31
benschubertIf it would be to only make this one simpler, I'd rather not :)07:31
tristanWe did have a stint where timed activities could be silenced, I think that was a bit weird and wonder if that's gone now07:31
juergbiI mean we could use the same for buildbox-casd termination07:31
benschuberttristan: we have this for "nested" timed activities07:31
tristani.e. there is a with context.silence(): context manager to silence some nested activities yes07:31
juergbiright, for nested cases07:32
juergbitbh, waiting before starting the scheduler makes more sense to me longer term07:33
juergbiand it might not even be that difficult07:33
juergbior does anyone have concerns with that?07:33
* tristan doesn't have full background on how this affects !1889... but certainly you only need to start the scheduler when you start processing tasks and not before07:34
*** jude has joined #buildstream07:36
juergbitristan: the timed activity in that MR is about (waiting while) establishing a connection to buildbox-casd, which each child job has to do as you can't share a connection across processes07:37
juergbias there isn't too much a child job can typically do without buildbox-casd being available, it probably makes sense to not even spawn child jubs until buildbox-casd is ready07:38
juergbiand then child jobs never have to wait07:38
tristanAhaaa07:38
tristanYeah I was reading the mr summary and scratching my head07:38
juergbiwe currently don't wait for buildbox-casd right when we start it such that we can do some things in the main process in parallel07:39
tristanthis is only about establishing a *first* connection07:39
juergbiwell, the code runs in each child job but as soon as buildbox-casd is ready, noone has to wait for it anymore07:39
tristanRight, I suppose that it stays around for a while when a BuildStream session finishes ?07:40
tristanso it might even be already running when launching a bst command ?07:40
juergbino, not right now07:40
juergbileaving a process running brings some problems07:40
tristanso it is a dedicated casd per session ?07:40
juergbiyes07:40
juergbiit's something we could consider changing but it requires some work07:41
juergbie.g., it doesn't work with our current approach of protecting session blobs from expiry07:41
juergbiand it can be confusing during development, might also need some work in the test suite07:41
tristanand it times out just incase BuildStream crashes after launching it and before talking to it ?07:41
* tristan feels like this could be coupled by a socket connection or such, die only on hangup07:42
juergbithe timeout case right now in master is mainly hit when buildbox-casd takes too much time for initialization07:42
juergbiwhich can happen with cold kernel dentry cache and/or slow disk and a large CAS cache07:42
tristanInterestingly, I suppose we still need casd for cases where we don't run a scheduler07:43
juergbithat is also being improved on the buildbox-casd side, though: https://gitlab.com/BuildGrid/buildbox/buildbox-casd/-/issues/5907:43
juergbiyes, we also use it outside the scheduler. e.g., also bst show updates timestamps of artifact blobs, and that's done by buildbox-casd07:44
tristanI'm just looking at the patch now, I sort of agree that things feel a bit cleaner if you just ensure this is already setup before launching the scheduler07:49
tristanI'm thinking that (A) we want `bst show` to be snappy as possible, and that it is probably not a bad thing if the startup of a `bst build` session is just as slow as the startup of `bst show`07:50
tristanThe only reason I can think of for lazy initialization of casd would be to optimize startup time, could very well be missing something07:51
* tristan thinks that anyway at the beginning of a build, there is already a traversal of elements where we need to know what is cached in order to print the build plan before processing elements07:52
* tristan always sets himself up for a missing (B) whenever starting something with an (A)07:53
juergbitristan: yes, I think in practice most if not all actions wait for casd before starting the scheduler anyway07:53
juergbibut we probably want to make that explicit and then we know we don't have to worry about it in child jobs07:54
*** seanborg_ has joined #buildstream07:55
tristanAs long as we're not losing some hidden value-add, I totally agree07:55
tristanIf the lazy initialization is costly (needing to consider whether it is initialized at some places), it should be justified07:56
juergbias long as we keep the bst loading process parllel to buildbox-casd initialization, I don't think we lose anything07:56
juergbii.e., we should definitely wait for casd after loading, not before07:56
tristanParallelizing initialization of casd with loading of project elements is certainly valuable07:57
*** phildawson has joined #buildstream08:00
juergbiyes, casd startup should get faster but it will still not be instantaneous. with parallel loading we should be able to totally hide that08:00
juergbiwell, at least when we don't need casd for a junction08:00
tristanSo we're not doing a simple fork() anymore for the scheduler ?08:02
tristanI'm seeing confusing, dangerous looking object pickles scattered about08:02
tristan_new_plugin_from_reduction_args ... hmmmmm08:04
juergbitristan: the scheduler is still fork-based. there is optional pickling support, that is a preparatory step to support Windows but that effort is currently on hold08:04
tristanyeah messing with the plugin factory seems to trigger problems with this08:04
juergbiwe do have a CI job that should make sure it doesn't bitrot08:05
tristanyeah I think that's whats biting me08:06
tristantests-spawn-multiprocess indeed08:06
tristanOk so I need to pass more arguments... my branch adds arguments to PluginFactory.lookup()08:06
tristanso that we can report deprecation warnings and missing plugins with provenance information, and so that we can log warnings using the messenger08:07
* tristan wants to ping Angelos on irc and ask him how this thing is supposed to work ... eh08:08
tristanDo we know that a plugin has been instantiated in the main process already if we're going to throw that plugin at _scheduler/jobs/jobpickler.py ?08:10
juergbitristan: the pickling happens when a job is scheduled, so everything should be initialized already at that point08:12
juergbithat said, I haven't looked into the pickling code in a while, might be missing something08:12
juergbis/scheduled/spawned/08:12
tristanYeah what it basically means is I can avoid understanding it08:16
tristanI can call PluginFactory.lookup(None, "kind", None) if I know that the plugin has already been instantiated08:17
juergbitristan: do we even hit the lookup code at that point?08:20
juergbiI would expect all lookups to be complete08:20
juergbiah, jobpickler code08:20
juergbiright, ignore me08:21
tristanits very unclear to me what that code is hehe08:23
*** tpollard has joined #buildstream08:25
*** santi has joined #buildstream08:27
juergbithe issue is that plugin classes probably have to be loaded again in the child processes as that doesn't carry over in spawn08:27
juergbiand I suppose classes themselves don't get pickled08:27
juergbionly instances08:27
tristanI'm not sure08:27
tristanThe pickle jobber doesn't seem to use the default YAML, and seems to go through some obscure constructor08:28
tristancalling cls.__init__()08:28
tristanWhich I expect plays together in some obscure way with the __setstate__/__getstate__ methods08:29
juergbiit shouldn't have to reload anything from YAML. that state should be copied over08:29
juergbiyes, getstate/setstate are called when pickling/unpickling08:29
juergbithe default is to pickle/unpickle all instance variables, however, there are some cases where it's not possible (e.g. extension objects) and other cases where we don't want it (would be very expensive and we don't need it)08:30
juergbiin those cases we override getstate/setstate to limit/manipulate pickling08:30
* tristan keeps feeling so reluctant to try and bake up a pytest fixture which creates and installs a plugins package08:53
tristanI guess I aught to just start08:54
* tristan wonders if this will screw with the python environment too much even08:54
*** chipb has quit IRC09:01
*** lachlan has joined #buildstream09:15
*** tristan has quit IRC09:41
*** phoenix has joined #buildstream09:48
*** lachlan has quit IRC10:03
*** phoenix has quit IRC10:23
*** phoenix has joined #buildstream10:24
*** lachlan has joined #buildstream10:46
*** lachlan has quit IRC10:50
*** lachlan has joined #buildstream11:04
*** lachlan has quit IRC11:11
*** cphang has quit IRC11:21
*** devcurmudgeon has quit IRC11:21
*** ikerperez has quit IRC11:21
*** cphang has joined #buildstream11:22
*** ikerperez has joined #buildstream11:26
*** lachlan has joined #buildstream11:57
*** tpollard is now known as CTtpollard12:11
*** CTtpollard is now known as tpollard12:11
*** lachlan has quit IRC12:19
*** lachlan has joined #buildstream12:32
*** phoenix has quit IRC13:28
WSalmoncan users get bst to pick up config options from the Enviroment?13:30
WSalmonis there some magic that we can put in project.conf?13:31
WSalmonno worries if not, i know we dont like to do this sort of thing13:31
WSalmonif not would you put https://gitlab.com/celduin/crash/bsp-playground/example-app/-/blob/sean/qemu/.gitlab-ci.yml#L13 to be `BST: bst --colors --option board ${board}` then you can set the board env in each stage once?13:33
*** seanborg__ has joined #buildstream13:36
*** seanborg_ has quit IRC13:37
*** seanborg__ has quit IRC13:38
*** seanborg__ has joined #buildstream13:39
*** lachlan has quit IRC14:11
*** lachlan has joined #buildstream14:45
*** lachlan has quit IRC14:48
*** lachlan has joined #buildstream15:02
juergbiWSalmon: no, we don't support anything like that. I think there was some discussion about defining something along the lines of presets a long time ago, not sure whether that would make sense. creating a Makefile with bst commands is also an option, of course, freedeskotp-sdk does that15:23
WSalmonthanks juergbi15:25
*** lachlan has quit IRC15:38
*** seanborg__ has quit IRC16:23
*** phildawson has quit IRC16:42
*** lachlan has joined #buildstream17:02
*** lachlan has quit IRC17:07
*** phoenix has joined #buildstream17:36
*** tpollard has quit IRC17:37
*** lachlan has joined #buildstream17:38
*** chipb has joined #buildstream17:40
*** santi has quit IRC17:41
*** santi has joined #buildstream17:42
*** lachlan has quit IRC17:43
*** santi has quit IRC17:52
*** jude has quit IRC17:55
*** lachlan has joined #buildstream18:07
*** lachlan has quit IRC18:15
*** toscalix has joined #buildstream19:04
*** benschubert has quit IRC20:00
*** phoenix has quit IRC23:38

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