*** tristan has quit IRC | 03:03 | |
*** traveltissues has quit IRC | 05:41 | |
*** traveltissues has joined #buildstream | 05:42 | |
*** slaf has quit IRC | 05:44 | |
*** slaf has joined #buildstream | 05:52 | |
*** tristan has joined #buildstream | 05:53 | |
*** ChanServ sets mode: +o tristan | 05:53 | |
*** benschubert has joined #buildstream | 07:28 | |
benschubert | juergbi: 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 MR | 07:29 |
---|---|---|
tristan | Ummmm | 07:30 |
juergbi | hi benschubert, I don't remember any plan of doing this for all timed activities | 07:30 |
juergbi | it could make sense but it might also be confusing if user looks at the log and doesn't know why something is missing | 07:30 |
tristan | timed activities as I recall are always explicitly intended to show the start/end time of a given thing | 07:30 |
juergbi | for the 'waiting for casd' case probably not an issue but maybe for others | 07:30 |
tristan | and we do status messages which are not shown by default unless you ask for --verbose | 07:30 |
juergbi | so it could be a flag to timed activity, I suppose? | 07:30 |
benschubert | Ok, I might be mis-remembering | 07:31 |
benschubert | If it would be to only make this one simpler, I'd rather not :) | 07:31 |
tristan | We did have a stint where timed activities could be silenced, I think that was a bit weird and wonder if that's gone now | 07:31 |
juergbi | I mean we could use the same for buildbox-casd termination | 07:31 |
benschubert | tristan: we have this for "nested" timed activities | 07:31 |
tristan | i.e. there is a with context.silence(): context manager to silence some nested activities yes | 07:31 |
juergbi | right, for nested cases | 07:32 |
juergbi | tbh, waiting before starting the scheduler makes more sense to me longer term | 07:33 |
juergbi | and it might not even be that difficult | 07:33 |
juergbi | or 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 before | 07:34 | |
*** jude has joined #buildstream | 07:36 | |
juergbi | tristan: 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 processes | 07:37 |
juergbi | as 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 ready | 07:38 |
juergbi | and then child jobs never have to wait | 07:38 |
tristan | Ahaaa | 07:38 |
tristan | Yeah I was reading the mr summary and scratching my head | 07:38 |
juergbi | we currently don't wait for buildbox-casd right when we start it such that we can do some things in the main process in parallel | 07:39 |
tristan | this is only about establishing a *first* connection | 07:39 |
juergbi | well, the code runs in each child job but as soon as buildbox-casd is ready, noone has to wait for it anymore | 07:39 |
tristan | Right, I suppose that it stays around for a while when a BuildStream session finishes ? | 07:40 |
tristan | so it might even be already running when launching a bst command ? | 07:40 |
juergbi | no, not right now | 07:40 |
juergbi | leaving a process running brings some problems | 07:40 |
tristan | so it is a dedicated casd per session ? | 07:40 |
juergbi | yes | 07:40 |
juergbi | it's something we could consider changing but it requires some work | 07:41 |
juergbi | e.g., it doesn't work with our current approach of protecting session blobs from expiry | 07:41 |
juergbi | and it can be confusing during development, might also need some work in the test suite | 07:41 |
tristan | and 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 hangup | 07:42 | |
juergbi | the timeout case right now in master is mainly hit when buildbox-casd takes too much time for initialization | 07:42 |
juergbi | which can happen with cold kernel dentry cache and/or slow disk and a large CAS cache | 07:42 |
tristan | Interestingly, I suppose we still need casd for cases where we don't run a scheduler | 07:43 |
juergbi | that is also being improved on the buildbox-casd side, though: https://gitlab.com/BuildGrid/buildbox/buildbox-casd/-/issues/59 | 07:43 |
juergbi | yes, we also use it outside the scheduler. e.g., also bst show updates timestamps of artifact blobs, and that's done by buildbox-casd | 07:44 |
tristan | I'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 scheduler | 07:49 |
tristan | I'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 |
tristan | The only reason I can think of for lazy initialization of casd would be to optimize startup time, could very well be missing something | 07: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 elements | 07:52 | |
* tristan always sets himself up for a missing (B) whenever starting something with an (A) | 07:53 | |
juergbi | tristan: yes, I think in practice most if not all actions wait for casd before starting the scheduler anyway | 07:53 |
juergbi | but we probably want to make that explicit and then we know we don't have to worry about it in child jobs | 07:54 |
*** seanborg_ has joined #buildstream | 07:55 | |
tristan | As long as we're not losing some hidden value-add, I totally agree | 07:55 |
tristan | If the lazy initialization is costly (needing to consider whether it is initialized at some places), it should be justified | 07:56 |
juergbi | as long as we keep the bst loading process parllel to buildbox-casd initialization, I don't think we lose anything | 07:56 |
juergbi | i.e., we should definitely wait for casd after loading, not before | 07:56 |
tristan | Parallelizing initialization of casd with loading of project elements is certainly valuable | 07:57 |
*** phildawson has joined #buildstream | 08:00 | |
juergbi | yes, casd startup should get faster but it will still not be instantaneous. with parallel loading we should be able to totally hide that | 08:00 |
juergbi | well, at least when we don't need casd for a junction | 08:00 |
tristan | So we're not doing a simple fork() anymore for the scheduler ? | 08:02 |
tristan | I'm seeing confusing, dangerous looking object pickles scattered about | 08:02 |
tristan | _new_plugin_from_reduction_args ... hmmmmm | 08:04 |
juergbi | tristan: 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 hold | 08:04 |
tristan | yeah messing with the plugin factory seems to trigger problems with this | 08:04 |
juergbi | we do have a CI job that should make sure it doesn't bitrot | 08:05 |
tristan | yeah I think that's whats biting me | 08:06 |
tristan | tests-spawn-multiprocess indeed | 08:06 |
tristan | Ok so I need to pass more arguments... my branch adds arguments to PluginFactory.lookup() | 08:06 |
tristan | so that we can report deprecation warnings and missing plugins with provenance information, and so that we can log warnings using the messenger | 08:07 |
* tristan wants to ping Angelos on irc and ask him how this thing is supposed to work ... eh | 08:08 | |
tristan | Do 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 |
juergbi | tristan: the pickling happens when a job is scheduled, so everything should be initialized already at that point | 08:12 |
juergbi | that said, I haven't looked into the pickling code in a while, might be missing something | 08:12 |
juergbi | s/scheduled/spawned/ | 08:12 |
tristan | Yeah what it basically means is I can avoid understanding it | 08:16 |
tristan | I can call PluginFactory.lookup(None, "kind", None) if I know that the plugin has already been instantiated | 08:17 |
juergbi | tristan: do we even hit the lookup code at that point? | 08:20 |
juergbi | I would expect all lookups to be complete | 08:20 |
juergbi | ah, jobpickler code | 08:20 |
juergbi | right, ignore me | 08:21 |
tristan | its very unclear to me what that code is hehe | 08:23 |
*** tpollard has joined #buildstream | 08:25 | |
*** santi has joined #buildstream | 08:27 | |
juergbi | the issue is that plugin classes probably have to be loaded again in the child processes as that doesn't carry over in spawn | 08:27 |
juergbi | and I suppose classes themselves don't get pickled | 08:27 |
juergbi | only instances | 08:27 |
tristan | I'm not sure | 08:27 |
tristan | The pickle jobber doesn't seem to use the default YAML, and seems to go through some obscure constructor | 08:28 |
tristan | calling cls.__init__() | 08:28 |
tristan | Which I expect plays together in some obscure way with the __setstate__/__getstate__ methods | 08:29 |
juergbi | it shouldn't have to reload anything from YAML. that state should be copied over | 08:29 |
juergbi | yes, getstate/setstate are called when pickling/unpickling | 08:29 |
juergbi | the 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 |
juergbi | in those cases we override getstate/setstate to limit/manipulate pickling | 08:30 |
* tristan keeps feeling so reluctant to try and bake up a pytest fixture which creates and installs a plugins package | 08:53 | |
tristan | I guess I aught to just start | 08:54 |
* tristan wonders if this will screw with the python environment too much even | 08:54 | |
*** chipb has quit IRC | 09:01 | |
*** lachlan has joined #buildstream | 09:15 | |
*** tristan has quit IRC | 09:41 | |
*** phoenix has joined #buildstream | 09:48 | |
*** lachlan has quit IRC | 10:03 | |
*** phoenix has quit IRC | 10:23 | |
*** phoenix has joined #buildstream | 10:24 | |
*** lachlan has joined #buildstream | 10:46 | |
*** lachlan has quit IRC | 10:50 | |
*** lachlan has joined #buildstream | 11:04 | |
*** lachlan has quit IRC | 11:11 | |
*** cphang has quit IRC | 11:21 | |
*** devcurmudgeon has quit IRC | 11:21 | |
*** ikerperez has quit IRC | 11:21 | |
*** cphang has joined #buildstream | 11:22 | |
*** ikerperez has joined #buildstream | 11:26 | |
*** lachlan has joined #buildstream | 11:57 | |
*** tpollard is now known as CTtpollard | 12:11 | |
*** CTtpollard is now known as tpollard | 12:11 | |
*** lachlan has quit IRC | 12:19 | |
*** lachlan has joined #buildstream | 12:32 | |
*** phoenix has quit IRC | 13:28 | |
WSalmon | can users get bst to pick up config options from the Enviroment? | 13:30 |
WSalmon | is there some magic that we can put in project.conf? | 13:31 |
WSalmon | no worries if not, i know we dont like to do this sort of thing | 13:31 |
WSalmon | if 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 #buildstream | 13:36 | |
*** seanborg_ has quit IRC | 13:37 | |
*** seanborg__ has quit IRC | 13:38 | |
*** seanborg__ has joined #buildstream | 13:39 | |
*** lachlan has quit IRC | 14:11 | |
*** lachlan has joined #buildstream | 14:45 | |
*** lachlan has quit IRC | 14:48 | |
*** lachlan has joined #buildstream | 15:02 | |
juergbi | WSalmon: 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 that | 15:23 |
WSalmon | thanks juergbi | 15:25 |
*** lachlan has quit IRC | 15:38 | |
*** seanborg__ has quit IRC | 16:23 | |
*** phildawson has quit IRC | 16:42 | |
*** lachlan has joined #buildstream | 17:02 | |
*** lachlan has quit IRC | 17:07 | |
*** phoenix has joined #buildstream | 17:36 | |
*** tpollard has quit IRC | 17:37 | |
*** lachlan has joined #buildstream | 17:38 | |
*** chipb has joined #buildstream | 17:40 | |
*** santi has quit IRC | 17:41 | |
*** santi has joined #buildstream | 17:42 | |
*** lachlan has quit IRC | 17:43 | |
*** santi has quit IRC | 17:52 | |
*** jude has quit IRC | 17:55 | |
*** lachlan has joined #buildstream | 18:07 | |
*** lachlan has quit IRC | 18:15 | |
*** toscalix has joined #buildstream | 19:04 | |
*** benschubert has quit IRC | 20:00 | |
*** phoenix has quit IRC | 23:38 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!