| opendevreview | Ian Wienand proposed opendev/infra-openafs-deb master: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885200 | 00:01 |
|---|---|---|
| *** JasonF is now known as JayF | 00:01 | |
| opendevreview | Ian Wienand proposed opendev/infra-openafs-deb master: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885200 | 00:02 |
| ianw | "Could not perform action: move changes endpoint is disabled" ... huh | 00:04 |
| fungi | where did you see that error? | 00:06 |
| opendevreview | Ian Wienand proposed opendev/infra-openafs-deb jammy: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885201 | 00:06 |
| ianw | trying to move the change from the UI | 00:06 |
| fungi | gerrit ui? what exactly does "moving" a change mean in that context? | 00:06 |
| ianw | sorry, yeah the gerrit UI. there's a "move change" from the hamburger dots. for i guess like me when you "git review" and push the change to master, when you really meant to push to the "jammy" branch | 00:08 |
| fungi | aha, so retargeting it to a different branch | 00:09 |
| ianw | i guess we can certainly get it fixed upstream; but this would not be the first time we've benefited from running from our own ppa built versions | 00:09 |
| ianw | upstream being ubuntu in this case | 00:10 |
| fungi | more like an sru for jammy | 00:10 |
| fungi | kinertic already has it | 00:11 |
| fungi | kinetic | 00:11 |
| ianw | the time consuming part is generally if we want to backport 1.8.9 to the older distros in our ppa to be in sync | 00:11 |
| ianw | probably a decent argument that they will never get a kernel that breaks this, so leaving well enough alone | 00:12 |
| fungi | yeah, i wouldn't be concerned with <jammy for support of newer kernels | 00:16 |
| opendevreview | Ian Wienand proposed opendev/infra-openafs-deb jammy: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885201 | 00:44 |
| opendevreview | Ian Wienand proposed opendev/infra-openafs-deb jammy: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885201 | 00:53 |
| opendevreview | Ian Wienand proposed opendev/infra-openafs-deb jammy: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885201 | 01:22 |
| ianw | fungi: ^ that works now, and has produced the source @ https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_578/885201/4/check/openafs-deb-package-build/5786e2c/openafs-debs.tar.gz | 01:32 |
| ianw | so on commit it should get pushed to the PPA for build | 01:33 |
| ianw | one thing is I guess we don't have a pre-validation build stage; i'm not sure when i made all this if I forgot that or the idea is that the ppa will build it, and if it fails we have logs, can iterate and push again | 01:34 |
| ianw | i think i'm tending towards the second, because otherwise we're just re-creating the ppa build process but maybe not exactly, so doing more work to find the same thing | 01:34 |
| ianw | the other thing is I think that it has only worked once under my supervision for 1.8.8.1 so i wouldn't class it as "fire and forget" at this point | 01:35 |
| ianw | it wants review but probably good for someone else to babysit it to reduce the bus factor of me pushing it again | 01:36 |
| *** amoralej|off is now known as amoralej | 06:51 | |
| fungi | ianw: thanks! i went ahead and single-core approved the dependency to fix the build job. will wait until there are a few more people awake to determine how we want to test the updated build | 12:00 |
| corvus | given that jammy afs appears broken, seems like we can push it to the ppa, pull that onto one of the replacement ze hosts (they're all still running) and try it out? | 14:34 |
| fungi | that works for me. i mainly worried about other jammy hosts auto-updating to that before we know it works. i *think* the changelog scoping the upload to jammy will limit the ppa build to there, though i'm not 100% sure it won't update our packages for older releases | 14:42 |
| fungi | i'm happy to approve it and keep an eye on it so we can mitigate if something goes horribly wrong | 14:42 |
| fungi | also i suppose the blast radius is further limited by the fact that the new module will only be used after a reboot | 14:43 |
| fungi | so as long as we test in a timely manner before we get bitten by some critical bit of infrastructure rebooting on us due to an emergency, it should be quite safe | 14:44 |
| corvus | hrm good point, it might work on jammy hosts that don't execute that syscall. do we have existing jammy afs clients? | 14:53 |
| corvus | oh and sorry i just picked up on the concern that it might affect older releases | 14:53 |
| fungi | well, again since it needs a reboot to swap in the rebuild lkm, i'm not super worried | 14:54 |
| corvus | then yeah, that is outside my area of expertise. | 14:54 |
| fungi | i'll be able to tell for sure once the build finishes whethert it's showing up just for jammy | 14:54 |
| fungi | and as long as we take swift corrective action i think it will be fine | 14:55 |
| fungi | i suspect we don't have any jammy-based afs clients yet anyway | 14:55 |
| fungi | looks like backup02.ca-ymq-1.vexxhost:/opt/backups-202010 is at 92%, i'll get a prune going in a root screen session there | 15:02 |
| opendevreview | Merged opendev/infra-openafs-deb jammy: Update Jammy to 1.8.9 https://review.opendev.org/c/opendev/infra-openafs-deb/+/885201 | 15:02 |
| fungi | underway | 15:03 |
| clarkb | note that 885201 will update static's openafs deployment | 15:13 |
| clarkb | I don't think that is necessary a problem but we should be aware of it | 15:13 |
| fungi | thanks, yeah i didn't realize static was upgraded to jammy already, but that's a good one to keep an eye on | 15:14 |
| fungi | it's read-only though i think, so likely why we didn't hit the same problems on it as the executors? | 15:14 |
| clarkb | ya I'm guessing the issue the executors hit was due to write activity and not read activity since static has been fine as far as I can tell | 15:14 |
| fungi | https://launchpad.net/~openstack-ci-core/+archive/ubuntu/openafs now has 1.8.9-1~ppa1~jammy | 15:23 |
| fungi | i'll try an apt update on a focal server just to make sure it's not showing up, and a jammy server to make sure it is, then we can try upgrading one of the new executors | 15:23 |
| fungi | oh, i guess that was the upload notice not the build notice we got | 15:27 |
| fungi | it's not showing up with an apt update yet | 15:27 |
| fungi | though i notice apt complains about the fact that we have two files trying to add the same ppa to sources | 15:28 |
| fungi | /etc/apt/sources.list.d/openafs.list and /etc/apt/sources.list.d/ppa_openstack_ci_core_openafs_jammy.list | 15:28 |
| fungi | aha, added by both our openafs-server-config role and our zuul-executor role, i think? | 15:29 |
| clarkb | I suspect static won't update until unattended upgrades occur. We may want ot get ahead of that? | 15:35 |
| clarkb | but ya testing on servers not yet in use first is a good idea | 15:35 |
| fungi | https://launchpad.net/~openstack-ci-core/+archive/ubuntu/openafs/+build/26277525 says the build completed 23 minutes ago | 15:37 |
| fungi | but an apt update on the replacement ze01 isn't showing it available | 15:37 |
| fungi | static02 does show them as available though | 15:39 |
| clarkb | load balancer differences maybe? | 15:40 |
| fungi | aha, it was the duplicate sources.list entries causing it to be ignored. i moved one of them to an invalid filename and now it's appearing | 15:40 |
| fungi | review02 is running on focal and doesn't see updated openafs packages, so that's good | 15:44 |
| fungi | on 104.130.124.246 (replacement ze02) i'm running `sudo apt upgrade` which should pull in the new builds of openafs-client, openafs-krb5, and openafs-modules-dkms | 15:46 |
| fungi | er, replacement ze01 i mean | 15:46 |
| fungi | it's also applying some new security updates for grub, perl, and libssh | 15:46 |
| fungi | dkms is going to take a few minutes to build the new lkm | 15:48 |
| clarkb | any idea if it will modprobe the update successfully replacing the old one or do we need a reboot? In particular i wonder because one is from a ppa and the other from upstream and not sure they properly replace each other (I think they do) | 15:54 |
| fungi | it looks like it did indeed modprobe it in and restarted the client | 15:55 |
| fungi | though we may want to reboot the machine anyway just to be on the safe side | 15:56 |
| fungi | also i totally forgot that module hotloading was possible for this, so yes the risk with static02 is a little higher than i estimated | 15:56 |
| fungi | corvus: any concerns with rebooting the replacement ze01? i see there is an executor container currently running on it, not sure if it needs to be stopped gracefully first | 15:57 |
| fungi | assuming things go well with the reboot, we can try upgrading static02 and make sure stuff served out of afs is still browseable thereafter | 15:58 |
| corvus | fungi: it should be a zombie | 16:02 |
| corvus | fungi: we should docker-compose down it | 16:02 |
| corvus | so docker-compose down, then reboot should be okay. if something goes wrong and it does start up, then i think it will either work or safely crash, so no worries there. | 16:03 |
| fungi | on it, thanks! | 16:04 |
| fungi | downed the container and then rebooted the server | 16:05 |
| fungi | rebooted successfully, container does not appear to have started on boot | 16:07 |
| fungi | as a cursory check, i'm able to browse /afs/openstack.org and /afs/.openstack.org from it, so static02 will likely be fine | 16:08 |
| fungi | corvus: did you have a manual reproducer for the bug you saw which i can test on the replacement ze01 or should we just try to swap it back into service? | 16:11 |
| corvus | fungi: no i don't; gimme 1 minute to see if i can repro on new-ze02 | 16:16 |
| corvus | fungi: as root: `docker run --rm -it --privileged -v /afs:/afs -v /etc/openafs:/etc/openafs quay.io/zuul-ci/zuul-executor:latest zuul-bwrap /tmp /bin/sh` | 16:21 |
| fungi | thanks | 16:21 |
| *** amoralej is now known as amoralej|off | 16:21 | |
| corvus | fungi: if it works, you'll get a bunch of bwrap log entries and a /bin/sh prompt; if not, you'll get dropped right back into your current shell with a bad return code and no output | 16:21 |
| corvus | (and a kernel trace in dmesg) | 16:22 |
| clarkb | corvus: oh wow the errors were just starting the container then? I guess idfferent than what I expected given static is happy but static isn't running in a container either so that could be the difference | 16:23 |
| fungi | BUG: kernel NULL pointer dereference | 16:23 |
| fungi | :( | 16:23 |
| clarkb | fungi: and I guess you double checked that the new module is installed and not the old one? | 16:26 |
| corvus | clarkb: it's in the bwrap check on startup when it tries to make a pag | 16:27 |
| clarkb | fcntl.ioctl(open("/proc/fs/openafs/afs_ioctl"), 0x40084301, struct.pack("lllll", 0, 0, 0, 0, 21)) I guess | 16:28 |
| fungi | `uname -a` says we're booted on 5.15.0-73-generic and the last update time on /lib/modules/5.15.0-73-generic/updates/dkms/openafs.ko was last modified half an hour ago | 16:34 |
| clarkb | fungi: I wonder if pagsh works | 16:34 |
| clarkb | (mostly out of curiousity I don't know that this would fix anyhting for us, but maybe it would point to us trying to hit internal apis that are no longer supported?) | 16:34 |
| corvus | https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/bubblewrap/__init__.py#L83-L89 | 16:35 |
| corvus | clarkb: yeah, i think it's worth a check whether that is still the right api/data | 16:35 |
| fungi | also `find /lib/modules/ -name openafs.ko` only finds the one module so there's no other version it could be loading i don't think | 16:35 |
| clarkb | fungi: cool | 16:36 |
| clarkb | looking at the old strace we're sending a TCGETS ioctl request and getting back an inappropriate ioctl for device. fungi is the new traceback similar? That might lead some additional evidence towards doing the wrong thing with ioctl there now? | 16:48 |
| clarkb | I think struct.pack("lllll", 0, 0, 0, 0, 21) is setting AFSCALL_SETPAG which resets the PAG. I'm not sure about the request value though as that seems to line up with TCGETS | 16:58 |
| fungi | i did not see any traceback. you mean strace? | 16:58 |
| fungi | i can try to run the container under strace i think | 16:58 |
| fungi | do i need to strace docker-run or zuul-bwrap? | 16:59 |
| fungi | looks like there's no strace inside the container image | 17:00 |
| clarkb | fungi: ya I meant strace but also dmesg should have something? You want to trace zuul-bwrap | 17:00 |
| fungi | ahh, the call trace from the kernel, yeah | 17:01 |
| corvus | python3 -c 'import fcntl; fcntl.ioctl(open("/proc/fs/openafs/afs_ioctl"), 0x40084301, struct.pack("lllll", 0, 0, 0, 0, 21))' | 17:01 |
| corvus | shorter reproducer with no docker/etc. can just run that from the host | 17:01 |
| clarkb | I think https://github.com/openafs/openafs/blob/630d423897e5fffed1873aa9d12c4e74a8481041/src/sys/glue.c#L24-L44 is where afs is giving us an entrypoint to do this sutff | 17:04 |
| fungi | https://paste.opendev.org/show/bgS2qV5CUcp1IGlqNcOE/ | 17:05 |
| fungi | that's what dmesg reports from the kernel oops | 17:05 |
| clarkb | https://github.com/openafs/openafs/blob/630d423897e5fffed1873aa9d12c4e74a8481041/src/config/afs_args.h#L258 This defines the request in terms of _IOW() | 17:06 |
| fungi | yeah, on the updated server, corvus's python-based reproducer results in "Killed" on the terminal and a 137 exit code | 17:08 |
| clarkb | fungi: you should be able to trace that | 17:09 |
| corvus | clarkb: https://paste.opendev.org/show/byum0QqoWxpe9Ow6Li3e/ double checking current values | 17:10 |
| corvus | (that's on a local jammy host) | 17:11 |
| fungi | https://paste.opendev.org/show/b4gQlSSvajxWGSi728Aa/ | 17:11 |
| fungi | that's the strace on the ze01 replacement | 17:11 |
| clarkb | ok so unlikely that we're hitting an invalid interface and instead the bug is likely in openafs :/ | 17:11 |
| clarkb | and TCGETS must just be an unfortunate collision and strace it trying to be helpful | 17:11 |
| corvus | what if the tcgets is a python thing and it's causing the issue? | 17:12 |
| corvus | openat(AT_FDCWD, "/proc/fs/openafs/afs_ioctl", O_RDONLY) = 3 | 17:13 |
| corvus | ioctl(3, _IOC(_IOC_WRITE, 0x43, 0x1, 0x8), 0x7ffcea36e210) = 0 | 17:13 |
| corvus | close(3) = 0 | 17:13 |
| corvus | that's from pagsh | 17:13 |
| corvus | like python itself is performing an unrequested tcgets | 17:14 |
| clarkb | oh weird | 17:14 |
| clarkb | you can set strace --raw and set a syscall value for it to not interpolate arguments | 17:14 |
| clarkb | maybe we do that to see the raw value being passed there just ot be sure it isn't a some helpful mixup but actually a different value? | 17:15 |
| clarkb | hrm its trying to do something with encodings | 17:17 |
| clarkb | is it possible that is just a red herring? its doing that to figure out terminal stuff then failing on our thing later? | 17:17 |
| clarkb | no scrolling to the end of fungi's paste the last ioctl it does is that after opening the file we want to talk to | 17:18 |
| clarkb | unless strace can't log the lsat ioctl (the one we care about) because it crashes before strace can strace it? (I didn't think strace worked that way though) | 17:19 |
| fungi | i could try to strace it with a null stdin to convince python3 there is no terminal | 17:19 |
| clarkb | or we just need a little C program to do it | 17:19 |
| clarkb | see if that reproduces or not | 17:19 |
| clarkb | I guess that is what pagsh does though and it worked according to coruvs? though not sure where that ran | 17:20 |
| fungi | still does a tcgets when i </dev/null | 17:20 |
| corvus | https://paste.opendev.org/show/bMllXA2i6CFZxvGeNGOT/ | 17:21 |
| corvus | that works for me | 17:21 |
| corvus | i think it's the high-level python open method | 17:21 |
| clarkb | I'm guessing older openafs just sort of ignored what python was doing? or maybe it answered properly and now it doesn't and we explode? this is a really fun bug :) | 17:22 |
| fungi | or python on jammy is trying to be helpful | 17:22 |
| clarkb | corvus: does open() work if we open it in byte mode | 17:22 |
| clarkb | fungi: zuul is on python3.11 from custom compiles (but also jammy's python seems to reproduce) | 17:23 |
| fungi | oh, good point | 17:23 |
| clarkb | ya I'm guessing if we switch to binary mode we might be able to do the high level open | 17:23 |
| fungi | so we're not seeing it with python 3.11 on older kernels | 17:23 |
| corvus | clarkb: "rb" doesn't fix it | 17:23 |
| clarkb | fungi: correct | 17:23 |
| clarkb | corvus: darn, was worth a shot I guess, but switching to os.open seems reasonable? | 17:24 |
| corvus | (nor does "wb") | 17:24 |
| fungi | keep in mind we were seeing this break with jammy on the openafs version we use with focal | 17:24 |
| corvus | yeah working on the os.open change | 17:24 |
| clarkb | fungi: no I don't think so | 17:24 |
| clarkb | fungi: jammy nodes should use jammy openafs | 17:24 |
| clarkb | (because it is newer than what was in the ppa) | 17:24 |
| fungi | clarkb: anf our focal nodes use jammy openafs backported to focal via ppa | 17:24 |
| clarkb | fungi: no the ppa is older than jammy. I dug into this a bit when I deployed static | 17:25 |
| clarkb | static "properly" installed jammy openafs from upstream and not our ppa because apt found a newer version in upstream | 17:25 |
| clarkb | but ya it could be the kernel itself being more discerning about ioctl requests against specific endpoints | 17:25 |
| clarkb | and have nothing todo with afs itself other than afs not supporting that request | 17:26 |
| fungi | 1.8.8.1-2~ppa0~focal vs 1.8.8.1-3ubuntu2~22.04.1 | 17:26 |
| clarkb | ah ok so minimally newer | 17:26 |
| fungi | so the patches from -3ubuntu2 are the difference i guess | 17:26 |
| clarkb | fungi: either that or the kernel itself being to blame regardless of the openafs version | 17:27 |
| fungi | yeah, that's why i was leaning toward it being kernel-version-specific, but you're right the packages don't have exactly the same source | 17:27 |
| fungi | i'll try to compare patches in a sec | 17:27 |
| tonyb | Question from the peanut gallery? that strace seems to have the ioctl file opened read-only shouldin't it we write-only? | 17:28 |
| corvus | write access isn't required for ioctls | 17:28 |
| clarkb | but also it is doing a TCGETS which is a read operation | 17:29 |
| clarkb | the problem here is python trying to infer encoding types for the file open I think | 17:29 |
| corvus | (it's definitely a good thing to check, and i did check "wb" on the high level open call just to make sure something weird isn't happening) | 17:29 |
| clarkb | and to do that it does a TCGETS and that explodes on ioctl devices that don't expect it | 17:29 |
| clarkb | I'm guessing this works on normal files in a less explodey manner | 17:29 |
| corvus | (but since it isn't/shouldn't be necessary, best to request minimal perms so we don't run into a permission denied error) | 17:30 |
| clarkb | corvus: fungi: so I guess the next steps are to land this then restest on new ze01 and new ze02 and decide if we even need new openafs for jammy? | 17:30 |
| corvus | clarkb: indeed -- my earlier reproducer (the python one-liner) actually has a bug in that i forgot the 'import struct' which means it was failing before we even got around to doing the ioctl, it really was just at the open() call. | 17:31 |
| corvus | clarkb: wfm | 17:31 |
| clarkb | ok my only concern with that is we be aware of static's state since I think unattended upgrades will update it if it hasn't already | 17:31 |
| clarkb | those run around 0600 UTC? | 17:31 |
| corvus | might even be a good idea to pull that onto those from the intermediate registry; get answers sooner and without landing it | 17:32 |
| clarkb | ++ | 17:33 |
| clarkb | corvus: I'm going to leave a comment about error checking :) | 17:33 |
| corvus | clarkb: i was going for like-for-like | 17:35 |
| clarkb | corvus: posted. Ya but open() raises and handles some errors a bit more graceuflly I think. We don't have that anymore so need to check if the fd is -1 | 17:35 |
| opendevreview | Neil Hanlon proposed openstack/diskimage-builder master: Set platform argument in container build command for cross-arch builds https://review.opendev.org/c/openstack/diskimage-builder/+/884451 | 17:35 |
| clarkb | corvus: but ya its probably not critical either | 17:36 |
| corvus | clarkb: nothing about the old code was graceful; i think the new code will explode in equivalent ways | 17:36 |
| clarkb | ack | 17:36 |
| clarkb | corvus: I guess a followup can add more error checking then. I'll +2 this | 17:36 |
| corvus | i mean we don't need/want error checking | 17:36 |
| corvus | if this fails, we want it to crash | 17:36 |
| corvus | if it does something other than crash, then we need error checking | 17:36 |
| clarkb | corvus: looks like -1 does raise if you try to operate on it as an fd so it will crash | 17:37 |
| corvus | right, so this code has exactly the right error checking | 17:38 |
| corvus | i do not think we should mask the exception which is what adding a try/except handler would do | 17:38 |
| clarkb | ack wfm | 17:38 |
| clarkb | fwiw there are libraries that will calculate the _IOW values dynamically at runtime. If we need this to be more portable we could do that but seems that is overkill since we haven't needed it yet. That said maybe we should add a commentabout where that magic number comes from. I'll push that up on top of corvus' change | 17:43 |
| clarkb | corvus: fungi: where do I find the new executor IP addrs? | 17:57 |
| fungi | clarkb: in the revert commit near the top of system-config | 17:58 |
| clarkb | `python3 -c 'import fcntl; import os; import struct; fd = os.open("/proc/fs/openafs/afs_ioctl", os.O_RDONLY); fcntl.ioctl(fd, 0x40084301, struct.pack("lllll", 0, 0, 0, 0, 21))'` works on jammy ze02 | 18:01 |
| clarkb | implying distro openafs is ok | 18:01 |
| clarkb | https://zuul.opendev.org/t/zuul/build/a79927386da74218aa7394ee7d5875b2/artifacts we have artifacts now if we want to test zuul stuff on new ze01/ze02 | 18:02 |
| fungi | so we should be able to unwind the ppa update | 18:02 |
| clarkb | fungi: I suspect so but we can test with the zuul images above on ze01 and ze02 to determine that. We also need to make sure we don't orphan that package on some hosts if they already pulled it | 18:03 |
| clarkb | fungi: maybe you want to pull the executor image from ^ and run your bwrap tests again? | 18:03 |
| clarkb | I think that is a good next step | 18:03 |
| fungi | how do i pull from the buildset registry? | 18:11 |
| corvus | you can do that with right-click copy link for the image in question on the artifacts page; so it'd be like "docker pull insecure-ci-registry.opendev.org:5000/quay.io/zuul-ci/zuul-executor:a79927386da74218aa7394ee7d5875b2_latest" and then "docker run ..." same thing | 18:11 |
| fungi | aha, thanks | 18:11 |
| fungi | you anticipated my question | 18:11 |
| clarkb | I'm trying to sort out in my head if we report this as a bug to either python or linux | 18:12 |
| clarkb | I think python probably should handle this more gracefully particularly since open() worked before. But at the same time linux says you shouldn't break userspace... | 18:12 |
| clarkb | maybe we do neither and just accept that for ioctls using os.open is a good idea anyway | 18:12 |
| corvus | but linux might stop reading at "tainted" | 18:13 |
| clarkb | ya we might have to reproduce on some other ioctl with python | 18:13 |
| fungi | sudo docker run --rm -it --privileged -v /afs:/afs -v /etc/openafs:/etc/openafs insecure-ci-registry.opendev.org:5000/quay.io/zuul-ci/zuul-executor:a79927386da74218aa7394ee7d5875b2_latest zuul-bwrap /tmp /bin/sh | 18:15 |
| fungi | [...] | 18:15 |
| fungi | # | 18:15 |
| fungi | yay! | 18:15 |
| corvus | \o/ | 18:15 |
| fungi | hole in one | 18:15 |
| clarkb | woot | 18:15 |
| clarkb | so now we have two threads to pull on. Which openafs version do we want to use on jammy? And do we proceed with replacing executors? | 18:16 |
| fungi | i vote we roll back to distro original packages on jammy | 18:16 |
| fungi | which presumably means reverting the build change and then cleaning up the ppa | 18:16 |
| clarkb | I'm good with that. We can always roll forward again too if we need to by reverting | 18:17 |
| fungi | and then double-checking the jammy hosts to make sure they haven't updated | 18:17 |
| fungi | working on the change first | 18:17 |
| clarkb | static02 has not updated according to dpkg -l | 18:17 |
| clarkb | fungi: before we do that can you run our bwrap test on ze02? | 18:17 |
| clarkb | fungi: since that will be a bit more complete than the test I did | 18:17 |
| fungi | sure, i can repeat it there | 18:18 |
| fungi | 104.130.127.14 is the replacement ze02 | 18:18 |
| fungi | still running openafs 1.8.8.1-3ubuntu2~22.04.1 | 18:19 |
| fungi | pulling now | 18:19 |
| fungi | still worked! | 18:21 |
| fungi | i've gone ahead and approved the zuul change | 18:21 |
| clarkb | great so ya I think trying distro openafs to start with is good and we can always do this update to openafs if we need to later | 18:21 |
| clarkb | I'm not sure I understand what made https://review.opendev.org/c/opendev/infra-openafs-deb/+/885201 jammy only | 18:25 |
| opendevreview | Jeremy Stanley proposed opendev/infra-openafs-deb master: Revert "Update Jammy to 1.8.9" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885304 | 18:26 |
| fungi | clarkb: corvus: ianw: ^ | 18:26 |
| clarkb | fungi: ^ do you know why that only made jammy packages? | 18:26 |
| fungi | yes, apparently we upload for specific ubuntu releases to build | 18:26 |
| clarkb | right but what controls that? | 18:26 |
| fungi | the repo itself is branched, and the jammy branch uploads for jammy | 18:26 |
| clarkb | aha I missed that was on the jammy branch. Thanks | 18:27 |
| fungi | yeah, me too originally, until i realized what i was looking for/at | 18:27 |
| clarkb | fungi: any idea what the process is there? Do we need to cleanup the ppa first? then land the change? | 18:27 |
| clarkb | I don't know if the ppa kept our old version around. I guess it doens't matter in this case bceause we'll grab the distro version. | 18:27 |
| clarkb | So we can probably land this chagne. It will push the old stuff and then we delete the new stuff? | 18:28 |
| clarkb | +2 on https://review.opendev.org/c/opendev/infra-openafs-deb/+/885304 either way. Just let me know if I can help with the order of operations or anything | 18:28 |
| fungi | yeah, i'm assuming we clean up after that pushes | 18:31 |
| fungi | though i expect we can clean up sooner as long as nothing triggers a new upload | 18:32 |
| clarkb | looking at https://launchpad.net/~openstack-ci-core/+archive/ubuntu/openafs/+packages lading the revert may just replace the package? it isn't keeping older packges there | 18:33 |
| fungi | https://help.launchpad.net/Packaging/PPA/Deleting | 18:33 |
| clarkb | assuming it doesn't do any special version comparisons then I think it would just replace newer thing with newly built older thing? | 18:33 |
| fungi | yeah | 18:33 |
| fungi | and then we presumably delete both | 18:33 |
| clarkb | I think it would just be the one and ya we could decide if we need to delete it or not | 18:34 |
| clarkb | but ya I think first step is landing that revert and see what happens | 18:34 |
| fungi | agreed. if there end up being two versions then we can delete both at the same time | 18:34 |
| clarkb | sounds good I need to sort out lunch momentarily but I'm mostly around if we want to do that | 18:35 |
| fungi | also, thinking about it, unattended-upgrades probably won't upgrade it automatically since it's not a security update? | 18:35 |
| clarkb | fungi: oh good call. We might need the daily ansible runs to do it if the role explicitly updates the package? or maybe it was alwys a manual thing | 18:35 |
| fungi | i'm around to continue hammering on this, so happy to monitor it | 18:35 |
| clarkb | fungi: approve at will then I guess | 18:35 |
| fungi | thanks | 18:35 |
| Clark[m] | fungi: did you want me to approve it or are you waiting for a good time spot? | 19:00 |
| fungi | oh, sorry, i misread earlier and thought you said you approved it. will do now | 19:19 |
| fungi | though chris just asked if we can go out to get something to eat, so maybe i should hold off? | 19:20 |
| fungi | regardless, i don't think the new upload is going to cause any issues that the earlier one wouldn't, so let's just go ahead and get it rolling | 19:21 |
| fungi | and approved | 19:21 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Update Jammy to 1.8.9" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885304 | 19:22 |
| Clark[m] | Sounds good | 19:22 |
| fungi | okay, headed out for a bit, shouldn't be more than an hour and then i'll keep working on this | 19:28 |
| clarkb | ok that change didn't actually do anything | 19:47 |
| clarkb | because it was pushed to master | 19:48 |
| clarkb | fungi: ^ | 19:48 |
| clarkb | how did that even make sense to git I'm really confused now | 19:49 |
| tonyb | orgin/master and origin/jammy were the same | 19:52 |
| clarkb | tonyb: no they differed I think. The first parent was 599c797 which only has a readme in it | 19:53 |
| tonyb | Yeah Okay I spoke to soon | 19:53 |
| clarkb | first thing is I feel like this is a gerrit issue | 19:53 |
| clarkb | because the diff is completely wrong given that situation | 19:54 |
| clarkb | but I suspect this happened because the content is fast forwardable | 19:54 |
| clarkb | anyway we don't want to revert the merge we want ot revert all the changes between here and there which is annoying | 19:54 |
| clarkb | let me get a proper revert up first though | 19:55 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb jammy: Fix jammy branch .gitreview branch https://review.opendev.org/c/opendev/infra-openafs-deb/+/885308 | 19:56 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb jammy: Revert "Update Jammy to 1.8.9" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885309 | 19:57 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb master: Revert "Revert "Update Jammy to 1.8.9"" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885310 | 20:03 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb master: Revert "Update Jammy to 1.8.9" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885311 | 20:03 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb master: Revert "Add build stamp to push to production PPA" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885312 | 20:03 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb master: Revert "Test generic job path" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885313 | 20:03 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb master: Revert "Run promote job from project-config" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885314 | 20:03 |
| opendevreview | Clark Boylan proposed opendev/infra-openafs-deb master: Revert "Build jammy packages" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885315 | 20:03 |
| clarkb | The git revert manpage says reverting merge commits is weird and I'm not sure it is what we want. So instead we get several changes to revert what the merge brought in... Could do it as a single squashed change if we prever | 20:03 |
| clarkb | *prefer | 20:04 |
| clarkb | https://github.com/git/git/blob/master/Documentation/howto/revert-a-faulty-merge.txt#L172-L174 | 20:05 |
| clarkb | Ultimately I think gerrit really let us down here :/ | 20:06 |
| tonyb | Yeah It was nice of it to try and be smart It needed a few alarm bells though | 20:17 |
| tonyb | Where are the zuul tenant configs kept? I think that's what I want to ask. I wanted to remove the windmill stuff but I can't quite see how it all comes togther. | 20:20 |
| tonyb | I suspect there is one (or more) repos that I don't clone that I should | 20:21 |
| clarkb | tonyb: openstack/project-config/zuul.d iirc | 20:21 |
| clarkb | that dir might be wrong but the repo is correct | 20:21 |
| tonyb | Okay I'll keep poking. | 20:22 |
| clarkb | I just posted to the gerrit matrix/discord room about this and ya asked for better diffs and or a warning if the parent change is not proposed to the target branch or already on the target bracnh | 20:22 |
| clarkb | tonyb: sorry it is in zuul/main.yaml and I think windmill is already removed | 20:22 |
| tonyb | Ahhh zuul not zuul.d | 20:22 |
| tonyb | Is it worth trying to clean up windmill stuff in gerrit/gitea/hound ? | 20:23 |
| clarkb | tonyb: I thought about it when I pulled them out of zuul but its a lot of effort to retire things and if the maintainers aren't interested in it them I'm somewhat inclined to just leave things as is | 20:23 |
| clarkb | basically no reason to add extra work to ourselves | 20:24 |
| clarkb | But if we want to clean things up further the project retirement steps are probably the ones to do. Then the repos will be auto removed from things like hound and have little content elsewhere | 20:24 |
| tonyb | clarkb: Okay thanks | 20:25 |
| clarkb | I'm going to delete zp01.opendev.org now | 20:27 |
| clarkb | #status log Deleted zp01.opendev.org (0eb65b92-2ccc-4fc1-a410-c240c96851f0) as it has been replaced by a newer server. | 20:29 |
| opendevstatus | clarkb: finished logging | 20:29 |
| opendevreview | Clark Boylan proposed openstack/project-config master: DNM Testing merge mode errors in Zuul configuration https://review.opendev.org/c/openstack/project-config/+/885316 | 20:34 |
| opendevreview | Clark Boylan proposed opendev/system-config master: Update Gerrit 3.8 image to the final 3.8.0 release https://review.opendev.org/c/opendev/system-config/+/885317 | 20:45 |
| clarkb | frickler: https://review.opendev.org/c/openstack/project-config/+/885316 confirms your hunch "Merge mode merge not supported by project github.com/ansible/ansible. Supported modes: []." | 20:48 |
| clarkb | corvus: ^ fyi I don't think that is urgent because opendev removed those projects from its config instead, but may be something we need to sort out for other github users of zuul? | 20:48 |
| clarkb | I'm guessing this is a permissions or api issue. | 20:48 |
| fungi | okay, back | 20:58 |
| clarkb | fungi: tldr is we merged that revert to master which did an implicit merge of jammy into master | 20:58 |
| fungi | clarkb: catching up, i checked out the jammy branch but maybe we don't have a defaultbranch set in .gitreview (checking) | 20:58 |
| corvus | clarkb: it has not come up in my github-related work, so i suspect permissions or api edge case. i agree that the behavior on opendev wasn't ideal, but also, i wouldn't want to simply say "if it's empty just pick something" since that could be a good canary that something important is wrong. i think a solution should probably involve a deeper analysis of what conditions result in the empty list. since it's not coming up except in this case so | 20:58 |
| corvus | far (where, if i had to take a wild guess, someone just yanked our app access because who are these people anyway) i'm happy to leave it be for now. | 20:58 |
| ianw | hrm, i'm unsure if pushing an older version to the PPA will build or if it will reject it on version number differences | 20:59 |
| clarkb | fungi: yup the first change I pushed fixed that then the follow up reverts the change on the correct branch | 20:59 |
| fungi | okay, we do set defaultbranch in .gitreview, unfortunately it's set to "master" | 20:59 |
| clarkb | corvus: ya I think I'm happy to leave it for now and i Think the error feedback should help people who have set up access properly. Then we can corss this in opendev if we start third party ci again | 20:59 |
| corvus | clarkb: (put another way: if all those assumptions are correct, zuul would have actually correctly informed us (in its own esoteric way) that we did not have a correct configuration to do any kind of speculative testing of changes to those repos) | 21:00 |
| clarkb | fungi: and then there is a stack of reverts on master to revert the changes from the implicit merge. And I sent gerrit a message on discord/matrix to be all this seems bad and wrong can we make it better? | 21:00 |
| clarkb | ianw: we'll find out :) | 21:01 |
| fungi | okay, running through the new changes | 21:01 |
| clarkb | like I said ultimately I think Gerritfailed us here. It could've warned us the parent wasn't on the branch already or proposed to the branch. It could show a more complete diff etc. | 21:02 |
| clarkb | hopefully we can work with them to make this situation less confusing | 21:03 |
| fungi | yep, i've approved them all now i think. sorry about that | 21:03 |
| ianw | yeah; i did the same thing initially too yesterday uploading to the wrong branch. i did notice that the "move branch" from the UI didn't work there either, saying the endpoint was disabled | 21:04 |
| fungi | well, ultimately the fact that we had a defaultbanch set to master in the jammy branch was a foot cannon primed to fire | 21:04 |
| ianw | yeah, mea culpa on that | 21:04 |
| clarkb | I did not check the other branches in that repo but they may need default branch to be updated too? | 21:04 |
| * clarkb looks | 21:04 | |
| ianw | vhdutil might be worth checking on that too, uses the same process | 21:05 |
| opendevreview | Merged opendev/infra-openafs-deb jammy: Fix jammy branch .gitreview branch https://review.opendev.org/c/opendev/infra-openafs-deb/+/885308 | 21:05 |
| clarkb | I think the other branches have the same issue, but pushing updates to them will rebuild he package for all the things and I'm not sure we want ot be juggling that for everything right now? | 21:05 |
| opendevreview | Merged opendev/infra-openafs-deb jammy: Revert "Update Jammy to 1.8.9" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885309 | 21:05 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Revert "Update Jammy to 1.8.9"" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885310 | 21:05 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Update Jammy to 1.8.9" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885311 | 21:05 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Add build stamp to push to production PPA" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885312 | 21:05 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Test generic job path" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885313 | 21:05 |
| ianw | yeah, similarly i think that probably the ppa will reject the upload if it's the same version number, so it is *probably* a no-op | 21:06 |
| ianw | probably ... | 21:06 |
| ianw | :) | 21:06 |
| clarkb | the first promote job for the .gitreview update is running now | 21:11 |
| clarkb | then it will try to downgrade the version in the second job after that one completes | 21:11 |
| clarkb | if that fails we'll need to manually remove the newer packages and then land a noopy change to the jammy branch to force a rebuild. I believe there are timestamp comments already in there to do this | 21:11 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Run promote job from project-config" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885314 | 21:14 |
| opendevreview | Merged opendev/infra-openafs-deb master: Revert "Build jammy packages" https://review.opendev.org/c/opendev/infra-openafs-deb/+/885315 | 21:14 |
| fungi | as far as approving a change proposed to the wrong branch which fast-forwards other commits from a different branch in gerrit, i thought there was a config option from long ago to toggle permitting that | 21:14 |
| fungi | i remember it coming up back when one of the fuel maintainers accidentally approved a change which had been proposed to the wrong branch | 21:15 |
| clarkb | https://launchpad.net/~openstack-ci-core/+archive/ubuntu/openafs/+packages I don't think that updated the newer new version or downgraded | 21:16 |
| clarkb | so I think ianw is right if you try to push a new deb for the same version it is a noop and if you try to push an older version over a newer version it ignores you | 21:16 |
| clarkb | in that case we need to remove the jammy version and then land a change to push the old version to it? | 21:16 |
| clarkb | fungi: ^ were you still able to do that? | 21:17 |
| fungi | i think we didn't have a jammy version before, did we? | 21:18 |
| fungi | in which case we just want to remove the 1.8.9 build | 21:18 |
| fungi | we were just relying on the distro version anyway | 21:19 |
| clarkb | fungi: we had 1.8.8.1-2... before then we updated the version to 1.8.9 this morning that seems to have removed the 1.8.8.1-2 version from jammy | 21:19 |
| clarkb | yes we were just relying on the distro version and replacing 1.8.8.1-2 won't change that. It will just ensure that the jammy branch of our local repo is consistent with what we have in the PPA | 21:19 |
| fungi | ahh, so we had a verison which would never be selected automatically because it sorted earlier than the distro version | 21:19 |
| clarkb | yes | 21:20 |
| fungi | i'll work on removing the 1.8.9 packages, but per the document i linked earlier that won't be instantaneous | 21:20 |
| clarkb | ah yup the emails that got caught in moderation say these updates were rejected so that all makes sense | 21:20 |
| clarkb | fungi: " A deleted package disappears from the archive indexes in at most 20 minutes. As soon as this happens, users will no longer be able to install it via apt." <- I think this is the bit we care about and that seems quick enough | 21:21 |
| clarkb | mostly we don't want static to end up in a weird spot | 21:21 |
| fungi | Source and binaries deleted by Jeremy Stanley: openafs 1.8.9-1~ppa1~jammy in jammy | 21:22 |
| fungi | Deletion comment: ended up being an unnecessary backport, the version shipped in jammy works fine as long as the caller isn't broken | 21:22 |
| clarkb | (if it does we can probably just redeplo it if necessary) | 21:22 |
| clarkb | so now if we push a noop change we should be able to publish 1.8.8.1-2 again. Though I wonder if it would reject it having seen the version before | 21:22 |
| clarkb | (I would hope not if the version isn't older and the old package doesn't exist anmore) | 21:23 |
| fungi | also not sure how long we need to wait before trying to do that | 21:23 |
| clarkb | and ya we're using distro openafs anyway so it isn't vital More of a sync our local reality with the ppa reality for completeness | 21:23 |
| clarkb | next steps: update jammy ppa to have current version in our downstream repo, downgrade afs on new ze01 and/or replace ze01?, continue with zuul executor replacement cc corvus | 21:26 |
| corvus | i'd just go with a downgrade on ze01; pretty sure we can be sure that fallout is contained. | 21:29 |
| corvus | i don't think we can do 6-at-a-time during the week, and also, i'm not sure i have a lot of time to babysit that this week. so if someone wants to re-chunk it into... maybe 3-at-a-time? i bet we could do that... feel free. otherwise, i can probably pick it up again some time post-summit. | 21:31 |
| fungi | sounds reasonable, i'm happy to babysit it | 21:33 |
| clarkb | fungi: per your comment in the gerrit channel I guess we can set receive.rejectImplicitMerges to true in All-Project config | 22:02 |
| clarkb | unlike some other receive settings this one doesn't appear to be settable gerrit wide in gerrit.conf so we need to set it in our root project config config (the default value for this item is to inherit from parent) | 22:03 |
| clarkb | I'll make note of that on hte meeting agenda for tomorrow | 22:05 |
| fungi | yeah, i mean, i had a todo list item about it from before i was infra ptl, if that means anything | 22:06 |
| clarkb | fungi: heh you can push it and knock that item off the todo list :) | 22:07 |
| clarkb | I think we just check that there isn't any reason to not do it then go ahead | 22:07 |
| clarkb | worst case it is an easy revert | 22:07 |
| fungi | checking that it's still relevant in the modern era | 22:08 |
| clarkb | yup it shows up in the docs | 22:08 |
| clarkb | https://gerrit-review.googlesource.com/Documentation/config-project-config.html | 22:08 |
| fungi | cool | 22:08 |
| clarkb | and does not show up in https://gerrit-review.googlesource.com/Documentation/config-gerrit.html so we need to set it in All-Projects or each project individually and not gerrit.conf | 22:09 |
| fungi | we don't seem to include any receive options in the doc/source/gerrit.rst | 22:11 |
| clarkb | fungi: ya I think we've only set those that can go in gerrit.config | 22:11 |
| clarkb | to be fair thee are a number of useful ones there but ya I think this one should be added | 22:11 |
| clarkb | my meeting updates are in. Please add any others if you have them in the next little bit | 22:11 |
| opendevreview | Jeremy Stanley proposed opendev/system-config master: Enable receive.rejectImplicitMerges for Gerrit https://review.opendev.org/c/opendev/system-config/+/885318 | 22:14 |
| fungi | clarkb: like that? ^ | 22:14 |
| clarkb | yes that looks right. Slight nit I don't think fast forward is necessary it does an actual merge | 22:15 |
| fungi | oh, yeah maybe it's just that the non-merge commits need to already be present on some other branch | 22:16 |
| clarkb | yup I think that is it | 22:16 |
| clarkb | Gerrit must know about and have merged them to another branch. Then its yolo merging into another branch | 22:16 |
| opendevreview | Jeremy Stanley proposed opendev/system-config master: Enable receive.rejectImplicitMerges for Gerrit https://review.opendev.org/c/opendev/system-config/+/885318 | 22:16 |
| fungi | tweaked the commit message to that | 22:16 |
| clarkb | fungi: what is the process of downgrading openafs on ze01? Just uninstall it then reinstall and since the newer package is no longer available it should find the old one instead? | 22:25 |
| clarkb | I guess maybe we need to clear the cache too?) | 22:25 |
| fungi | we'll need to explicitly downgrade | 22:34 |
| fungi | probably the easiest way is to `sudo apt purge openafs-client openafs-krb5 openafs-modules-dkms` and then 'sudo apt install openafs-client` (which should pull the other two back in as deps again) | 22:37 |
| fungi | there is a way to do an in-place downgrade, but it's not preferable | 22:37 |
| fungi | i've got the purge in progress now | 22:37 |
| clarkb | ya ok so remove then reinstall makes sense | 22:50 |
| clarkb | I think https://github.com/python/cpython/blob/main/Lib/_pyio.py#L241 is the line of code we are tripping over | 22:50 |
| clarkb | it is either that isatty (which I can't reproduce on its own) or the bufferedreader/writer/random classes that come later. If I set buffering=0 then this works | 22:58 |
| clarkb | ok the issue still occurs if I manually set buffering to 4096 which should skip the isatty call so the issue is in the bufferedreader I think | 22:59 |
| fungi | fwiw, the other two packages weren't dependencies so i explicitly installed all 3 | 23:00 |
| fungi | lkm is building now | 23:00 |
| clarkb | oh! its actually the lseek that comes after the ioctl that kills it. So the invalid ioctl is handled fine and it returns I'm not a tty. If you set buffering to say 4096 it never calls isatty and that ioctl call doesn't happen | 23:05 |
| clarkb | yup if I switch mode to appending then we seem to break here https://github.com/python/cpython/blob/main/Lib/_pyio.py#L1585 before we do the ioctl | 23:12 |
| fungi | replacement ze01 is downgraded to openafs 1.8.8.1-3ubuntu2~22.04.1 packages, i'll reboot it again for good measure (still no zuul-executor container running on it) | 23:25 |
| fungi | packages on static02 were never upgraded, fwiw | 23:26 |
| clarkb | cool | 23:28 |
| clarkb | apprently pdb cannot step into open() | 23:28 |
| clarkb | which is annoying because it is implemented as python so this should theoretically be possible (I may not be understanding how to do it I guess) | 23:29 |
| clarkb | python3 -c 'open("/proc/fs/openafs/afs_ioctl", mode="rb", buffering=4096)` is the minimalest reproducer I've got so far | 23:30 |
| clarkb | and the problem is definitely calling lseek() on the resulting fd. | 23:31 |
| fungi | is it necessarily implemented as python tho? there are a lot of calls which have python implementations shadowed by c when it builds | 23:31 |
| clarkb | fungi: yes its https://github.com/python/cpython/blob/v3.10.6/Lib/_pyio.py#L66 which is python | 23:33 |
| clarkb | I'm trying to figure out where the bufferedreader calls lseek from and not figuring that out. Was hoping pdb would help but it isn't with step at least | 23:33 |
| fungi | mmm | 23:33 |
| clarkb | python3 -c 'import os; fd = os.open("/proc/fs/openafs/afs_ioctl", os.O_RDONLY); os.lseek(fd, 0, os.SEEK_CUR)' <- is a bit more verbose as to what causes the crash though | 23:34 |
| clarkb | at the end of the day openafs isn't handling the seek gracefully and python3 open() appears to always do it one way or another | 23:38 |
| clarkb | It is odd that we are seeking in an open() call particullay when we don't open with append set | 23:38 |
| clarkb | I'm thinking there may be a bug in python where it is seeking unnecessarily during open() then a separate bug in openafs not rejecting that request gracefully instead of panicing? | 23:39 |
| corvus | i'm going to remember this next time i do performance-critical io in python | 23:40 |
| clarkb | ya lseek should raise OSError on unseekable files | 23:41 |
| clarkb | at the very least I would think open() should raise OSError instad of panicking and that likely is due to openafs not gracefully returning the appropriate errno to do that? | 23:42 |
| clarkb | oh talking out loud here because I have no hard evidence yet. Does a buffered reader go out and preread the data? And the readmethods do a readall which do an lseek | 23:43 |
| clarkb | I wonder if that is the underlying cause of the lseek. Its buffering hte data before you ever doe a read()? | 23:43 |
| clarkb | I think I can test this on a regular file using strace | 23:44 |
| clarkb | no that doesn't appear to happen | 23:46 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!