*** nimish has joined #buildstream | 01:15 | |
*** nimish has quit IRC | 03:09 | |
*** mohan43u has joined #buildstream | 04:58 | |
*** tristan has joined #buildstream | 07:15 | |
gitlab-br-bot | marge-bot123 closed issue #947 (Errors lack context when files are not found) on buildstream https://gitlab.com/BuildStream/buildstream/issues/947 | 08:03 |
---|---|---|
gitlab-br-bot | marge-bot123 merged MR !1216 (tristan/missing-file-errors->master: Improve error reporting when files are not found) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1216 | 08:03 |
*** alatiera has joined #buildstream | 08:46 | |
gitlab-br-bot | tristanvb merged MR !1217 (tristan/missing-file-errors-1.2->bst-1.2: Improve error reporting when files are not found (1.2)) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1217 | 08:49 |
*** toscalix has joined #buildstream | 09:06 | |
gitlab-br-bot | tristanvb merged MR !1212 (tristan/cleanup-workspace-tests->master: tests/frontend/workspace.py: Remove redundant and pointless tests) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1212 | 09:23 |
benschubert | tristan: did you have a look at the update_state yet? | 09:32 |
*** ChanServ sets mode: +o tristan | 09:35 | |
tristan | benschubert, I didn't ! I thought you might say something last night and then I'd look at it in my daytime (which starts way earlier here in Korea) | 09:35 |
benschubert | tristan: Oh, I gave an update yesterday (9:57pm London Time). Anyways no worries. I didn't have time to look at it. Let me know if you want to to it otherwise I'll do it asap | 09:36 |
tristan | benschubert, I'm not in a huge hurry to close #919 to be honest, I'll be in foss asia this week and if I have time to work on it I could, I would certainly have time to roll a bugfix release during the week if the fix is ready | 09:36 |
gitlab-br-bot | Issue #919: 'bst build <elem>' does not assemble all required elements in some circumstances https://gitlab.com/BuildStream/buildstream/issues/919 | 09:36 |
tristan | Ah, I might have not been logged in at that time, my bad :-S | 09:36 |
benschubert | tristan: sure, I'll work on it by the end of the week | 09:36 |
benschubert | no worries | 09:36 |
benschubert | So we should add a "__runtime_deps_cache_key" on every element which is a hash of (direct runtime deps __runtime_deps_cache_key, __strong_cache_key). And add this as a condition for recursion. Correct? | 09:37 |
benschubert | Are you happy with my PQ to speed things up? | 09:38 |
benschubert | and thanks for the review on the linter btw, I'll answer those comments asap :) | 09:39 |
*** raoul has joined #buildstream | 09:39 | |
tristan | benschubert, I don't really understand why we need the PQ if we only store direct runtime deps, in any case it will still unconditionally descend into each direct reverse dep right ? | 09:45 |
tristan | Or does this balance the traversal somehow ? | 09:45 |
tristan | For the hash, when we spoke with juergbi earlier, I thought a saner approach would be to have a single hash which was inclusive of all conditions; maybe call it a __state_hash or something | 09:47 |
tristan | benschubert, then in the recursive function, we only need to check if one thing has changed, instead of various | 09:47 |
benschubert | tristan: even if we store only direct runtime dependencies, we might have multiple elements having it, so the check would be done multiple times (For example A depends on B and C, B depends on C), we would change "C", then check A and B, then check A | 09:47 |
benschubert | tristan: I would rather not do that, since I still hope in the future to know which exact key can change when and have a more fine-grained state update :) | 09:48 |
tristan | Of course, that would have to change in the future when splitting up _update_state() functionality into domains, probably it would mean having different hashes to check | 09:48 |
tristan | benschubert, yeah we're on the same page about that granularity | 09:48 |
*** jonathanmaw has joined #buildstream | 09:48 | |
tristan | So yeah, why not just have the runtime_deps_cache_key right away | 09:48 |
benschubert | exactly! | 09:48 |
tristan | I think in your last you mean s/even if we store only direct runtime dependencies/even if we store only direct reverse dependencies/ ... | 09:49 |
benschubert | correct | 09:50 |
tristan | So in your example, when we recurse into C... we avoid double recursion into B and A, using the priority queue thingamajigie ? | 09:50 |
benschubert | exact | 09:50 |
benschubert | I will obviously benchmark both to see if the PQ is worth it | 09:51 |
benschubert | but my guess is that yes it is | 09:51 |
benschubert | (I'll obviously put it somewhere else, with tests if we go for it) | 09:51 |
tristan | benschubert, I am not against the PQ if it buys us cycles, and I would agree that it does | 09:51 |
tristan | or should probably indeed | 09:51 |
tristan | benschubert, however, it would probably be nice to tie up !1070 without the PQ and do that separately ? | 09:52 |
gitlab-br-bot | MR !1070: WIP: Refactor _update_state() to be called only when needed https://gitlab.com/BuildStream/buildstream/merge_requests/1070 | 09:52 |
benschubert | tristan: sure as long at we don't end up being slower in 1070 because we don't have the PQ (it would be in its own commit anyways) | 09:52 |
tristan | You suspect that without the PQ, 1070 will make things slower than they already are ? | 09:53 |
tristan | Well, if that's the case I don't really object, but I kind of doubt it :) | 09:53 |
benschubert | tristan: in the case were we have a stack that represents all elements yes | 09:53 |
benschubert | because we'll end up calling update_state on the stack way too many times :) | 09:53 |
tristan | I see what you mean | 09:55 |
tristan | benschubert, interestingly it will be more expensive in the case that the stack element's cache key cannot be resolved in the given round | 09:55 |
benschubert | which will be every time until we have it resolved | 09:56 |
benschubert | Hence having a PQ to reduce the number of calls we do there | 09:56 |
tristan | yeah | 09:56 |
tristan | I'm intrigued by the idea of passing along cache keys in another step, but I wonder what kind of data type or algorithm we could use | 09:59 |
tristan | I wonder if we could have a kind of hashing algorithm, which yields the same output hash for N input hashes, regardless of the order in which the input hashes are provided | 10:00 |
benschubert | tristan: with a push-based model for the pipeline, we know we would only need to update direct deps, I'll have a shot at this (and using the PQ for the queue will allow us to keep the order of insertion) | 10:00 |
* tristan not such a math expert to know if that is realistic | 10:00 | |
benschubert | tristan: do we really need this? I would think that's not super important since we have everything ordered anyways | 10:00 |
benschubert | I mean a simpler Xor would do but... | 10:01 |
benschubert | bytes(a) ^ bytes(b) ^ bytes(c) etc | 10:01 |
tristan | benschubert, I'm not sure the order it stable from session to session | 10:01 |
benschubert | on 256 would be actually quite good | 10:01 |
tristan | While the order might be stable within a single session | 10:01 |
benschubert | tristan: from my work in the loader, I'm pretty sure it is | 10:01 |
tristan | i.e. your reverse deps traversals will differ depending on what is loaded as targets | 10:02 |
tristan | Well, let | 10:02 |
tristan | gah ... let's see when we get there :) | 10:02 |
benschubert | the xor would be a good option :) | 10:02 |
benschubert | and probably quicker than hashing :D | 10:02 |
tristan | Yeah, better to not exchange the calls to Element.dependencies() for a different loop of similar complexity ;-) | 10:03 |
tristan | and do the hashing right away progressively, that would be nice | 10:03 |
gitlab-br-bot | aevri approved MR !1213 (chandan/container-plugins->master: doc/source/core_plugins.rst: Add link to bst-plugins-container) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1213 | 10:07 |
*** phildawson_ has joined #buildstream | 10:09 | |
*** tristan has quit IRC | 10:10 | |
*** lachlan has joined #buildstream | 10:41 | |
*** toscalix has quit IRC | 10:58 | |
jmac | Getting "Invalid requirement: '!nocover: -rrequirements/cov-requirements.txt'" when trying to run tox test atm | 11:02 |
*** toscalix has joined #buildstream | 11:02 | |
Kinnison | You need a newer version of tox. Install tox via pip3 and try again. I hit that last week | 11:02 |
Kinnison | If tox --version shows the tox installed via pip you'll be good | 11:03 |
gitlab-br-bot | juergbi approved MR !1221 (jennis/assert_composition_failure->master: Add tests to ensure that overwriting on subsequent compositions does not fail) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1221 | 11:09 |
jennis | ta juergbi | 11:10 |
*** lachlan has quit IRC | 11:32 | |
*** kapil___ has joined #buildstream | 11:34 | |
jmac | Kinnison: OK, but having to replace version of my system utilities is a bit onerous | 11:38 |
jmac | I'm not very impressed with this move to tox tbh | 11:38 |
jmac | My version of tox from pip3 is also 2.5.0 and has the same problem | 11:40 |
juergbi | jmac: are you sure it's the tox from pip3? if so, pip3 -U should help to upgrade | 11:48 |
*** lachlan has joined #buildstream | 11:50 | |
jmac | `pip3 list tox | grep tox` => tox (2.5.0) | 11:53 |
jmac | `tox --version` => 2.5.0 | 11:53 |
benschubert | jmac: `which tox` gives you what? | 11:55 |
jmac | benschubert: /usr/bin/tox | 11:56 |
benschubert | jmac: then you are still using your system's tox | 11:56 |
benschubert | jmac: you should `pip install --user tox` and check it's using your local user's tox | 11:56 |
jmac | OK, now it's /home/jimmacarthur/.local/bin/tox, but still 2.5.0 | 11:57 |
benschubert | pip install --user --upgrade tox ? | 11:59 |
*** raoul_ has joined #buildstream | 12:01 | |
jmac | Not much help. Although it says it's installed tox 3.7.0, my local version is still 2.5.0. | 12:01 |
*** raoul has quit IRC | 12:02 | |
jmac | Ah, might be bash caching it | 12:02 |
jmac | Yeah, that was it | 12:02 |
jennis | sassy marge: https://gitlab.com/BuildStream/buildstream/merge_requests/1221#note_149642188 | 12:42 |
adds68 | jennis make sure marges timeout is set the the same as your CI timeout | 12:43 |
jennis | Turns out there was a hanging test, it seems like it's at a sensible value | 12:48 |
juergbi | phildawson_: this appears to fail now with !1158 merged: tox -- tests/frontend/buildtrack.py | 12:51 |
gitlab-br-bot | MR !1158: Make it easy to test BuildStream against external plugins https://gitlab.com/BuildStream/buildstream/merge_requests/1158 | 12:51 |
juergbi | it first properly runs those tests but then fails with tons of: ValueError: option names {'--integration'} already added | 12:52 |
*** raoul_ has quit IRC | 13:13 | |
*** nimish has joined #buildstream | 13:25 | |
*** phildawson_ is now known as phildawson | 13:32 | |
* phildawson takes a look | 13:32 | |
*** nimish has quit IRC | 13:42 | |
*** nimish has joined #buildstream | 13:43 | |
*** nimish has quit IRC | 13:48 | |
*** nimish has joined #buildstream | 13:48 | |
benschubert | I'm getting | 13:53 |
benschubert | File "/home/bschubert/.local/lib/python3.6/site-packages/buildstream/_scheduler/jobs/job.py", line 425, in _child_action | 13:53 |
benschubert | result = self.child_process() # pylint: disable=assignment-from-no-return | 13:53 |
benschubert | File "/home/bschubert/.local/lib/python3.6/site-packages/buildstream/_scheduler/jobs/cachesizejob.py", line 31, in child_process | 13:53 |
benschubert | return self._casquota.compute_cache_size() | 13:53 |
benschubert | File "/home/bschubert/.local/lib/python3.6/site-packages/buildstream/_cas/cascache.py", line 1065, in compute_cache_size | 13:53 |
benschubert | new_cache_size = self.calculate_cache_size() | 13:53 |
benschubert | File "/home/bschubert/.local/lib/python3.6/site-packages/buildstream/_cas/cascache.py", line 1080, in calculate_cache_size | 13:53 |
benschubert | return utils._get_dir_size(self.casdir) | 13:53 |
benschubert | File "/home/bschubert/.local/lib/python3.6/site-packages/buildstream/utils.py", line 631, in _get_dir_size | 13:53 |
benschubert | return get_size(path) | 13:53 |
benschubert | File "/home/bschubert/.local/lib/python3.6/site-packages/buildstream/utils.py", line 624, in get_size | 13:53 |
benschubert | total += f.stat(follow_symlinks=False).st_size | 13:53 |
benschubert | FileNotFoundError: [Errno 2] No such file or directory: '/home/bschubert/.cache/buildstream-cache/buildstream/cas/tmpfa6k35pq' | 13:53 |
benschubert | when running BuildStream. Any tips on how to debug this? | 13:53 |
*** raoul_ has joined #buildstream | 13:53 | |
juergbi | benschubert: you're not running multiple `bst` instances at the same time, are you? | 13:57 |
juergbi | at first glance it seems like a temporary files was deleted during the file enumeration that happens as part of size calculation | 13:58 |
juergbi | we should probably not error fatally in that case, however, it's not supposed to happen with the current exclusive write access for cache cleanup/size jobs, afaict | 14:00 |
juergbi | we should also create all temporary files in the temp directory | 14:01 |
benschubert | juergbi: I'm not running multiple time bst no | 14:01 |
*** nimish has quit IRC | 14:08 | |
*** nimish has joined #buildstream | 14:08 | |
*** nimish has joined #buildstream | 14:09 | |
benschubert | any idea on where to track for tmp file created in the cas directory? | 14:11 |
juergbi | hm, tried to find it with a quick grep but couldn't spot anything using the cas dir for this instead of the temp dir | 14:16 |
benschubert | juergbi: I am using https://gitlab.com/BuildStream/buildstream/commits/66edc2818ebcc8a470e6dc9878c15e2cca672e3b as a buildstream version. Anything modified in the CAS afterwards that might fix it? | 14:19 |
benschubert | and the problem is consistent. 7 of my 7 runs failed with different tmp files | 14:20 |
gitlab-br-bot | marge-bot123 merged MR !1213 (chandan/container-plugins->master: doc/source/core_plugins.rst: Add link to bst-plugins-container) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1213 | 14:20 |
juergbi | benschubert: I don't think master has any relevant changes since then | 14:22 |
juergbi | ah, reproducible, interesting, so unlikely to be a race condition | 14:22 |
benschubert | juergbi: 8 out of 8 runs now | 14:22 |
benschubert | different tmp files obviously | 14:22 |
juergbi | anything that I could also quickly test here? | 14:22 |
juergbi | i.e., what are you doing to trigger it? | 14:23 |
benschubert | internal stuff, just building one of our projects | 14:23 |
*** nimish has quit IRC | 14:24 | |
benschubert | it runs for 3 to 10 minutes and then crashes | 14:24 |
*** nimish has joined #buildstream | 14:24 | |
juergbi | ah, it might actually be the temp file used to atomically create the 'cache_size' file | 14:26 |
benschubert | can you provide a test ? | 14:27 |
juergbi | that could only be an issue if two cache size jobs are running at the same time, though | 14:28 |
juergbi | which shouldn't happen anymore since the scheduler was fixed up | 14:28 |
juergbi | but maybe there is still something wrong there, hm | 14:28 |
*** nimish has quit IRC | 14:29 | |
*** nimish has joined #buildstream | 14:30 | |
benschubert | juergbi: I tried adding "tempdir=self.tmpdir" in that place, let's see what happens | 14:33 |
juergbi | benschubert: ignoring FileNotFoundError in utils._get_dir_size() or only checking the 'objects' directory in calculate_cache_size() should both work around this issue, but it might hide a real underlying scheduling issue | 14:33 |
aevri | Seems like we should use the tempdir arg here regardless: https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_cas/cascache.py#L1182 | 14:33 |
*** lachlan has quit IRC | 14:46 | |
*** nimish has quit IRC | 14:50 | |
*** nimish has joined #buildstream | 14:50 | |
laurence- | i've noticed a few marge bot comments about CI taking too long recently | 14:58 |
laurence- | the limit is set to 90 mins at the moment | 14:58 |
laurence- | should we raise this? | 14:58 |
phildawson | I think adds68's suggestion of setting marges timeout the same as the CI timeout makes sense. | 15:00 |
benschubert | juergbi: adding the tmpdir managed to fix the problem. Any idea on how to hunt the root cause of it though? | 15:08 |
juergbi | benschubert: maybe debug log prints for start/end of calculate_cache_size() as well as calls to set_cache_size() | 15:09 |
juergbi | they are supposed to never overlap, but it sounds like they do | 15:10 |
*** nimish has quit IRC | 15:10 | |
*** nimish has joined #buildstream | 15:10 | |
gitlab-br-bot | marge-bot123 closed issue #938 (Overwrite does not raise error when we try to overwrite a non-existent node) on buildstream https://gitlab.com/BuildStream/buildstream/issues/938 | 15:13 |
gitlab-br-bot | marge-bot123 merged MR !1221 (jennis/assert_composition_failure->master: Add tests to ensure that overwriting on subsequent compositions does not fail) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1221 | 15:13 |
benschubert | juergbi: sure I'll do that thanks | 15:14 |
*** nimish has quit IRC | 15:20 | |
*** jonathanmaw has quit IRC | 15:20 | |
*** nimish has joined #buildstream | 15:21 | |
*** jonathanmaw has joined #buildstream | 15:21 | |
*** nimish has quit IRC | 15:26 | |
*** nimish has joined #buildstream | 15:26 | |
*** drll has joined #buildstream | 15:43 | |
*** drll has quit IRC | 15:47 | |
*** nimish has quit IRC | 15:56 | |
*** nimish has joined #buildstream | 15:56 | |
*** lachlan has joined #buildstream | 16:00 | |
*** nimish has quit IRC | 16:01 | |
*** nimish has joined #buildstream | 16:02 | |
gitlab-br-bot | BenjaminSchubert opened (was WIP) MR !1222 (bschubert/linter-for-tests->master: Enable pylint on the tests) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1222 | 16:03 |
*** lachlan has quit IRC | 16:18 | |
*** nimish has quit IRC | 16:27 | |
*** nimish has joined #buildstream | 16:27 | |
*** lachlan has joined #buildstream | 16:28 | |
*** nimish has quit IRC | 16:32 | |
*** nimish has joined #buildstream | 16:32 | |
*** nimish has joined #buildstream | 16:33 | |
*** nimish has quit IRC | 16:38 | |
*** nimish has joined #buildstream | 16:38 | |
*** lachlan has quit IRC | 16:39 | |
*** tristan has joined #buildstream | 16:40 | |
*** nimish has quit IRC | 16:48 | |
*** nimish has joined #buildstream | 16:48 | |
*** rdale has joined #buildstream | 16:48 | |
gitlab-br-bot | aevri opened MR !1223 (aevri/doc_artifact_log->master: 'artifact log': document the 'artifacts' argument) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1223 | 16:48 |
aevri | Fun easter-egg: you can list all your cached artifacts with the error message from "bst show '*'" - handy before you run 'bst artifact log' :) | 16:49 |
*** nimish has quit IRC | 16:58 | |
*** nimish has joined #buildstream | 16:59 | |
*** lachlan has joined #buildstream | 17:03 | |
*** lachlan has quit IRC | 17:13 | |
*** nimish has quit IRC | 17:14 | |
*** nimish has joined #buildstream | 17:14 | |
*** lachlan has joined #buildstream | 17:16 | |
*** alatiera has quit IRC | 17:23 | |
*** alatiera has joined #buildstream | 17:28 | |
gitlab-br-bot | aevri opened (was WIP) MR !1210 (aevri/nodefaultsset->master: element.__init_default: treat `None` plugin_conf as if missing file + refactor) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1210 | 17:35 |
*** toscalix has quit IRC | 17:37 | |
*** nimish has quit IRC | 17:39 | |
*** nimish has joined #buildstream | 17:39 | |
*** lachlan has quit IRC | 17:41 | |
benschubert | phildawson: would it be possible to move the tests for external plugins in another tox environment? It's super annoying to have it fail whenever you run "tox -- tests/my/file/to/test" | 17:50 |
*** raoul_ is now known as raoul | 17:51 | |
raoul | had just rebased my branch onto master and was wondering why that was happening now | 17:52 |
phildawson | benschubert, I don't see any reason why not. I'll put together a quick MR | 17:52 |
benschubert | phildawson: thanks :D | 17:52 |
juergbi | it looks like there are lots of CI issues right now | 18:00 |
*** lachlan has joined #buildstream | 18:01 | |
*** jonathanmaw has quit IRC | 18:02 | |
gitlab-br-bot | juergbi opened issue #955 (Follow-up from "Artifact 'abstraction' class") on buildstream https://gitlab.com/BuildStream/buildstream/issues/955 | 18:04 |
*** nimish has quit IRC | 18:09 | |
*** nimish has joined #buildstream | 18:10 | |
phildawson | benschubert, I've opened !1224. I'll deWIP it in the morning once I've verified the coverage collection is working properly :) | 18:13 |
gitlab-br-bot | MR !1224: WIP: Move external plugin tests to seperate tox environment. https://gitlab.com/BuildStream/buildstream/merge_requests/1224 | 18:13 |
benschubert | phildawson: thanks! | 18:15 |
*** alatiera has left #buildstream | 18:19 | |
*** nimish has quit IRC | 18:20 | |
*** nimish has joined #buildstream | 18:20 | |
*** lachlan has quit IRC | 18:24 | |
*** alatiera has joined #buildstream | 18:32 | |
*** raoul has quit IRC | 18:32 | |
*** nimish has quit IRC | 19:05 | |
*** tristan has quit IRC | 19:45 | |
gitlab-br-bot | marge-bot123 closed issue #908 (Create Artifact abstraction class) on buildstream https://gitlab.com/BuildStream/buildstream/issues/908 | 21:16 |
gitlab-br-bot | marge-bot123 merged MR !1175 (tpollard/908->master: Artifact 'abstraction' class) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1175 | 21:16 |
*** slaf_ has joined #buildstream | 21:59 | |
*** slaf_ has joined #buildstream | 21:59 | |
*** slaf_ has joined #buildstream | 22:00 | |
*** slaf_ has joined #buildstream | 22:00 | |
*** slaf_ has joined #buildstream | 22:00 | |
*** slaf_ has joined #buildstream | 22:01 | |
*** slaf has quit IRC | 22:01 | |
*** slaf_ is now known as slaf | 22:01 | |
*** tristan has joined #buildstream | 22:04 | |
*** tristan has quit IRC | 23:51 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!