{"thread":{"id":"62971","subject":"Re: [Bug] Git ReadOnly Temp Packfile Causes \"Bad file descriptor\"And -13 Access Error With NFSv4","startedAt":"2025-02-18T21:35:44Z","lastAt":"2025-02-18T22:30:47Z","messageCount":2,"participants":["Maloney, Bryan","brian m. carlson"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"512640","messageId":"1628B638-81DC-4116-BD7B-85D691B54331@amazon.com","threadId":"62971","inReplyTo":null,"subject":"Re: [Bug] Git ReadOnly Temp Packfile Causes \"Bad file descriptor\"And -13 Access Error With NFSv4","fromName":"Maloney, Bryan","fromEmail":"bryanhm@amazon.com","sentAt":"2025-02-18T21:35:42Z","receivedAt":"2025-02-18T21:35:44Z","isPatch":false,"sender":{"key":"bryanhm@amazon.com","avatar":null},"body":"Good point on the POSIX compliance. I'd like to call out that this behavior of re-opening the file during NFSv4 state recovery is according to the NFS spec. So this scenario isn't something specific to just this filesystem. I think it comes down to NFS not being fully POSIX compliant in all situations.\n\nOn 2/10/25, 5:33 PM, \"brian m. carlson\" > wrote:\n\n\nOn 2025-02-10 at 15:56:59, Maloney, Bryan wrote:\n> ### Error\n> Kernel logs:\n> ```\n> NFSv4: state recovery failed for open file pack/tmp_pack_aR0Mu3, error = -13\n> ```\n> Git clone output:\n> ```\n> fatal: write error: Bad file descriptor, 137.31 MiB | 45.77 MiB/s\n> fatal: fetch-pack: invalid index-pack output\n> ```\n> \n> \n> ### Context\n> \n> The following error is seen when running git clone over NFSv4 and a failover, or server restart, occurs:\n> ```\n> NFSv4: state recovery failed for open file pack/tmp_pack_aR0Mu3, error = -13\n> ```\n> This error is an access denied error that happens when you try to open a file with insufficient permissions. In this case the file being opened is a read only file and it is attempted to be opened with write access.\n> \n> Git opens/creates this file with the O_RDWR flag but then applies read only permissions to it, 0444. Since the permissions are changed after the file is opened, the file handle works fine. However if the file was attempted to be re-opened with that same file handle we would see a -13 error. This is what we see following a failover in NFSv4. When clients reclaim their open files, the NFS server re-evaluates the file access.\n\n\nYour description of the problem is spot on. We intentionally set the\npermissions to 0444 because we never want anyone to change loose object\nfiles or packs, since doing so would corrupt the repository. This\nbehaviour is specifically allowed by POSIX[0]:\n\n\nThe argument following the oflag argument does not affect whether the\nfile is open for reading, writing, or for both.\n\n\nPOSIX does not allow the re-evaluation of file system access once the\nfile is open, so it sounds like your file system is not POSIX compliant,\nand Git generally requires lots of POSIX-compliant functionality from\nthe file system. For instance, we also require the POSIX consistency\nguarantees[1], among myriad others:\n\n\nIf a read() of file data can be proven (by any means) to occur after a\nwrite() of the data, it must reflect that write(), even if the calls\nare made by different threads. A similar requirement applies to\nmultiple write operations to the same file position.\n\n\nThe implicit violation of that particular requirement is why cloud\nsyncing services often corrupt the repository.\n\n\nCould you adjust your NFSv4 server such that is synchronizes state among\nthe primary and replicas in case of a required failover? I know we have\npeople successfully using Git with NFS without problems, although this\nparticular issue does often hit non-POSIX-compliant NFS implementations\nin a variety of ways. (This particular variant is new to me, though.)\n\n\n> This is an issue for active/passive HA file servers. Since NFSv4 evaluates file permissions at the time of opening a file, this FD will always get an access denied error if a failover occurs during git clone.\n\n\nI'm not sure there's even a good way to solve this problem on the Git\nside, since I suspect that if we opened the file as 0644 and then\nimmediately did an fchmod to 0444, if you'd still fail here if the file\nis reopened. Is that correct?\n\n\nI'll also point out that there's a variety of other software that does\nthe same thing as Git does, including zsh and Emacs, so fixing this in\nGit doesn't really fix the entire problem that your NFS server has,\nsince all of that other software will also be broken in at least some\ncases and require similar workarounds. (I discovered this with a\nsimple, 30-second search on GitHub some time back.) As far as I'm\naware, all other Git implementations also do the same thing as Git does,\nso you'd also need to patch go-git, libgit2, and every other\nimplementation as well.\n\n\n[0] https://pubs.opengroup.org/onlinepubs/9799919799/functions/open.html \n[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/write.html \n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n\n\n\n"},{"id":"512645","messageId":"Z7UKFQHHBJLfLrjY@tapette.crustytoothpaste.net","threadId":"62971","inReplyTo":"1628B638-81DC-4116-BD7B-85D691B54331@amazon.com","subject":"Re: [Bug] Git ReadOnly Temp Packfile Causes \"Bad file descriptor\"And -13 Access Error With NFSv4","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-02-18T22:30:45Z","receivedAt":"2025-02-18T22:30:47Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-02-18 at 21:35:42, Maloney, Bryan wrote:\n> Good point on the POSIX compliance. I'd like to call out that this\n> behavior of re-opening the file during NFSv4 state recovery is\n> according to the NFS spec. So this scenario isn't something specific\n> to just this filesystem. I think it comes down to NFS not being fully\n> POSIX compliant in all situations.\n\nI haven't read the NFS spec, so I can't speak to that, but I suspect\nit's entirely possible to have the NFS server paper over this problem\nduring state recovery, which is what I would recommend here. That might\nrequire an in-kernel NFS server (which Linux has) or some sort of\nshenanigans under the hood of a userspace server (e.g., temporarily\nchanging the permissions of the file but exposing the existing\npermissions to clients[0]), but it should be possible to do. I can\nimagine doing this without a problem in 9P and SFTP (which I have\nimplemented), for instance.\n\nIn general, I'm loathe to support a file server that's going to\nspontaneously decide to produce EBADF in the middle of operating on a\nfile for any reason, since that's asking for a bunch of hard-to-fix\nbreakage. That also exposes a huge race condition where we thought we\nhad a valid file descriptor, but it got closed for some reason and then\nanother thread opened a new file and got assigned the same number, and\nnow we're writing to a file we didn't expect. That will very likely end\nup with repository corruption, which would be really bad.\n\nAs Peff said, it's possible to work around this particular problem, but\nI'm concerned we'll find more weird edge cases that will break and that\nit will lead to data loss for users if we tolerate the NFS server just\nproducing an EBADF at a moment's notice.\n\n[0] This is grossly oversimplified and has a lot of edge cases, but I\ncan imagine how I'd go about it. It also depends on how you're storing\nthe files and a lot of other factors.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"}]}