| mborzecki | morning | 05:48 |
|---|---|---|
| mardy | mborzecki: hi! | 06:02 |
| mborzecki | mardy: hey | 06:02 |
| mborzecki | mvo: hey | 06:50 |
| mvo | good morning mborzecki | 06:51 |
| zyga-mbp | good morning | 06:53 |
| mvo | good morning zyga-mbp | 06:58 |
| mvo | fwiw, 10287 needs a second review | 06:59 |
| pstolowski | morning | 07:03 |
| mvo | good morning pstolowski ! | 07:07 |
| zyga-mbp | pstolowski nice work! | 07:07 |
| zyga-mbp | oh wait, that was actually ian! | 07:08 |
| zyga-mbp | sorry ian and pawel | 07:08 |
| zyga-mbp | I'm a bit sleepy still | 07:08 |
| pstolowski | hey zyga-mbp :) | 07:08 |
| mborzecki | mvo: reviewed, there seems to be some race in the test, google:centos-8-64:tests/main/snap-quota-groups failed and there's more than 1 task in the cgroup | 07:08 |
| mborzecki | pstolowski: zyga-mbp: hey | 07:08 |
| mvo | mborzecki: oh, interessting, thanks, looking | 07:10 |
| mvo | mborzecki: I think I do a followup, this is strange | 07:21 |
| mborzecki | mvo: that's fine, let's land it and we can tweak the test later | 07:22 |
| mborzecki | at least the whole feature will progress | 07:22 |
| mvo | 10329 should now also be unblocked | 07:32 |
| mborzecki | another one merged, we're down to 65 PRs | 07:51 |
| mborzecki | mvo: i've added some comments to https://github.com/snapcore/snapd/pull/10329 i can address those once i push out an rfc with serial & remodel, unless you want to have a go at it first | 08:12 |
| mvo | mborzecki: go wild, I'm looking at spread things right now | 08:13 |
| pedronis | mborzecki: does it mean the task count is always 0 on centos ? | 08:24 |
| mborzecki | pedronis: it's not, https://paste.ubuntu.com/p/R487f3RGRy/, disabling TasksAccounting makes system not create a group for the slice under the pids controller | 08:29 |
| mborzecki | pedronis: at least that what i determined through experimentation | 08:29 |
| mborzecki | pedronis: oh and when there's no cgroup, that's the current tasks count i get: TasksCurrent=18446744073709551615 so fun again | 08:30 |
| mborzecki | otoh, the value of MemoryCurrent is 0, so not sure what's really happening there | 08:31 |
| pedronis | mborzecki: I suspect we shoild just not support this there for now, it seems need a lot of attention and we don't really know what other bugs might hide there | 08:32 |
| mborzecki | it's only high when there's no memory controller at all, i.e. no memory limit set for given slice | 08:32 |
| mborzecki | where high == exabytes ian saw | 08:32 |
| pstolowski | mvo, pedronis there's an interesting old comment from zyga-mbp on a download gettng stuck bug here: https://bugs.launchpad.net/snapd/+bug/1891618/comments/2 | 08:37 |
| pedronis | mborzecki: mvo: it sounds like we would need a lot spread tests to be sure things actually work | 08:38 |
| pedronis | mborzecki: mvo: anyway I don't think it makes sense to block the chain of PRs on this, it's probably best to not support these systems and reconsider later | 08:39 |
| mvo | pstolowski: that is interessting indeed | 08:39 |
| mvo | pedronis: +1 for not supporting these old systemds | 08:39 |
| zyga-mbp | mvo is it possible to move snapd to latest go? | 08:39 |
| mborzecki | pedronis: mvo: added one more comment with more explanation and a pastebin | 08:39 |
| mborzecki | pedronis: postponing support for older systems sounds like a good call, we can always revisit this once we've done more experiments | 08:42 |
| mborzecki | wondering if systemd somehow reports that memory controller is available at all | 08:43 |
| pedronis | zyga-mbp: we plan to move to 1.13 but not latest | 08:43 |
| mborzecki | zyga-mbp: do you recall anything like that in systemd? ^^ | 08:43 |
| zyga-mbp | mborzecki like what exactly? | 08:43 |
| mborzecki | zyga-mbp: whether it's possible to find out if memory controller is even available | 08:44 |
| zyga-mbp | hmmm | 08:44 |
| zyga-mbp | isn't that visible in mountinfo in v1? | 08:44 |
| mborzecki | iirc it doesn't need to be mounted if memory accounting is disabled globally, and then you can always disable it in the kernel | 08:45 |
| zyga-mbp | (in v2 you have to consult what's enabled in the part you belong to | 08:45 |
| mborzecki | zyga-mbp: it is ;) but i'm more into poking systemd to know if that's doable | 08:45 |
| zyga-mbp | I see | 08:45 |
| zyga-mbp | I don't recall _but_ systemd has extensive dbus apis | 08:45 |
| zyga-mbp | perhaps it's somewhere there | 08:45 |
| zyga-mbp | I'd have to look | 08:45 |
| mborzecki | zyga-mbp: hm yeah, i should take a quick look | 08:46 |
| zyga-mbp | there is a property | 08:47 |
| zyga-mbp | MemoryAccounting | 08:47 |
| zyga-mbp | bool | 08:47 |
| zyga-mbp | on org.freedesktop.systemd1.Service | 08:47 |
| zyga-mbp | but I'd double check how it is implemented to know it is what you want | 08:47 |
| zyga-mbp | mborzecki https://github.com/systemd/systemd/blob/7dbc38db509f153256d3a3bfe6cbb26e2731c741/man/systemd.resource-control.xml#L269 | 08:50 |
| mborzecki | zyga-mbp: mhm, thanks, looking at it | 08:52 |
| === doko_ is now known as doko | ||
| mardy | pstolowski: does this line look totally correct to you? https://github.com/snapcore/snapd/blob/master/overlord/servicestate/service_control_test.go#L295 | 10:21 |
| mardy | shouldn't the action name be "test-snap.foo"? | 10:21 |
| pstolowski | looking | 10:22 |
| pstolowski | mardy: correct. it works because makeControlChange test helper creates apps out of it and current implementation of servicestate.Control() doesn't care about inst.Names anymore and uses apps. So it's confusing but test-only issue afaict | 10:31 |
| mardy | pstolowski: good! Then I guess it's fine if I make a small change to makeControlChange and to the tests, to use the fill names, right? | 10:32 |
| mardy | s/fill/full/ | 10:32 |
| pstolowski | mardy: yes, sounds fine | 10:33 |
| mborzecki | pedronis: opened https://github.com/snapcore/snapd/pull/10330 with the check for serial assertion in remodel, need to sort out the spread tests still | 10:46 |
| pedronis | mborzecki: thx, I will look in a bit | 10:47 |
| mborzecki | thanks | 11:00 |
| mborzecki | hm i think i got the tests runnig now | 11:29 |
| pstolowski | mborzecki: do you have some time for a 2nd review of https://github.com/snapcore/snapd/pull/10261 ? | 12:01 |
| mborzecki | pstolowski: yes, i can take a look in a bit | 12:02 |
| pstolowski | thx | 12:03 |
| mborzecki | meh the remodel spread tests | 12:36 |
| mborzecki | pedronis: quick questions, even if i don't put my account & account key assertions in the seed, will they be automatically pulled from the store? | 13:38 |
| pedronis | mborzecki: they are needed for first seeding, prepare-image puts them there for you, but I don't know what the test does | 13:39 |
| pedronis | mborzecki: without first seeding can't validate the model | 13:39 |
| mborzecki | pedronis: ok, i need to drop them there then | 13:39 |
| mborzecki | pedronis: the remodel tests, clean all of state from /var/lib/snapd and kick off first boot again | 13:40 |
| GunnarHj | pedronis: Hi Samuele! Apparmor fixes are needed to make fcitx5 (bug #1928360) and to some extent ibus (bug #1890905) work as expected in the snaps. (The fcitx5 task is higher priority.) Any chance you can take it on? | 13:45 |
| GunnarHj | https://launchpad.net/bugs/1928360 | 13:46 |
| GunnarHj | https://launchpad.net/bugs/1890905 | 13:46 |
| mardy | pedronis: I forgot to ask you to put https://github.com/snapcore/snapd/pull/10282 back into your review queue :-) | 13:53 |
| ijohnson | GunnarHj: you might want to also try pinging jamesh about such issues those bugs smell very desktopy to me | 13:55 |
| pedronis | GunnarHj: apparmor fixes in which sense? fixes to apparmor or to our apparmor rules? | 13:56 |
| GunnarHj | pedronis: The fcitx5 issue is about our apparmor rules, but the ibus one is about improving apparmor itself to start with. | 13:59 |
| GunnarHj | ijohnson: Thanks for the tip. Yeah, the bugs are indeed desktop'ish. | 14:00 |
| mborzecki | dumped the assertions into a single file, wondering if that will work | 14:08 |
| mborzecki | wooo, remodel-base workd | 14:15 |
| mborzecki | and pushed, remodel spread tests should work again | 15:03 |
| mvo | mborzecki: \o/ | 15:10 |
| ijohnson | mvo: pedronis: can you land https://github.com/snapcore/snapd/pull/10099 ? the failure on debian-10 are unrelated | 15:50 |
| ijohnson | mvo: also I noticed you cherry picked https://github.com/snapcore/snapd/pull/10324 to release/2.51, so I added the cherry-picked label to that PR for you | 15:55 |
| pedronis | ijohnson: I think mvo beat me to it | 16:01 |
| ijohnson | yeah, no worries thanks for checking on it though | 16:01 |
| mvo | ijohnson: thanks! I keep forgetting this | 16:09 |
| ijohnson | it will become habit and save us much time in the future :-) | 16:09 |
| mvo | yeah, it's good! | 16:16 |
| === ijohnson is now known as ijohnson|lunch | ||
| === ijohnson|lunch is now known as ijohnson | ||
| ijohnson | pedronis: can you merge 10331 then ? I will open up our two alternatives after that so you can decide in the AM what direction we want to go | 18:12 |
| ogra | did anyone tak to iemeyer yet about mup ? | 18:12 |
| ogra | (i see it re-connecting in a 30sec loop in the old channel ... FWIW) | 18:13 |
| ijohnson | ogra: I thought mvo did already, I assume Gustavo's been a bit busy lately to move it over | 18:14 |
| pedronis | ijohnson: how can you open the alternatives if we don't have spread tests about current memory usage? | 18:14 |
| ogra | ijohnson, ah | 18:14 |
| pedronis | ijohnson: we probably want to fix the CLI first | 18:14 |
| ijohnson | pedronis: well now that the daemon is reporting memory usage, regardless of whether the CLI displays it, if there are problems the CLI dies | 18:15 |
| pedronis | ijohnson: yes, but not dying is not a criteria enough to decide on the workarounds | 18:15 |
| ijohnson | I mean yeah I guess if you want to do the CLI changes first then we can, but we can't really measure the memory usage specifically in the spread test | 18:17 |
| ijohnson | the spread test update I had in the CLI change PR just sees that there are numbers in the output, it doesn't actually confirm that the numbers make sense | 18:17 |
| ijohnson | it would be hard to check that the numbers make sense given that memory usage is not predictable in exact terms, I guess I could compare to make sure the number is under 100 megabytes or something obvious like that | 18:18 |
| pedronis | ijohnson: there's probably a min and max that can be applied | 18:18 |
| ijohnson | I guess in other words, I'm just not sure what the threshold is to decide on the workaround | 18:19 |
| ijohnson | alright I guess I can write a min/max detection snippet for the functional spread test | 18:19 |
| pedronis | ijohnson: do we have a test where we see processes hit the OOM ? | 18:22 |
| ijohnson | yes that's `tests/main/snap-quota-groups` | 18:22 |
| pedronis | I see one where they don't start, it that the OOM or something else? | 18:22 |
| ijohnson | yes that's checking the OOM | 18:22 |
| ijohnson | well indirectly checking that the service fails due to oom | 18:22 |
| ijohnson | we check that the service starts properly with a large limit, then we check that it doesn't start with a small limit | 18:22 |
| pedronis | it seems interfaces-packagekit-control has started failing consistently on arch | 18:26 |
| pedronis | ijohnson: I merged that PR, please consider my comment in the next, anyway I still think we want to fix the CLI first thing, before considering the corner case distros | 18:27 |
| ijohnson | pedronis: ok yes I have that change implemented in the CLI PR, which I will open shortly after adding this min / max logic to the snap-quota-groups spread test for that PR | 18:28 |
| ijohnson | pedronis: also yeah I saw that test has started to fail, I will try to have a look this afternoon, if I don't get to it, I left a comment about it with logs in the SU doc for someone to look at | 18:29 |
| pedronis | ijohnson: I suppose it might be something for Maciej to look at | 18:29 |
| ijohnson | yeah nothing obvious came up when I looked at it | 18:30 |
| ijohnson | to me at least | 18:30 |
| === axino is now known as ux | ||
| === ux is now known as axino | ||
| * cachio afk | 19:04 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!