{"thread":{"id":"57990","subject":"Repository corruption if objects pushed in the middle of repack","startedAt":"2022-06-13T20:59:59Z","lastAt":"2022-06-13T22:27:14Z","messageCount":7,"participants":["Konstantin Ryabitsev","Taylor Blau","Chris Torek"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"457121","messageId":"20220613203145.wbpi2m3ys3hchw6c@meerkat.local","threadId":"57990","inReplyTo":null,"subject":"Repository corruption if objects pushed in the middle of repack","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2022-06-13T20:31:45Z","receivedAt":"2022-06-13T20:59:59Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"Hi, all:\n\nI'm trying to figure the cause of repository corruption in a very specific\ncase. Here's the setup:\n\n1. the repository is several GB in size, full of automatically generated pushes\n   (https://git.yoctoproject.org/poky-buildhistory/)\n2. this repository has no alternates or other clever things -- just your old\n   boring repository\n2. the builders check out this repository with --depth 1 during the build\n   stage, then add new logs to the repository, commit and push\n\nAdmittedly, this is a bad use of git, but let's use that outside the scope.\n\nEvery weekend we run a set of maintenance tasks and if we find that there are\nlots of new loose objects (which there usually are), we fire off a routine repack:\n\n1. first, repack runs with the following flags (-f if deemed necessary):\n\n        git repack -n --window-memory=1g -a -b --unpack-unreachable=yesterday -f --pack-kept-objects -d\n\n   Since the repository is large, this usually takes a long time (3+ hours)\n\n2. next, we generate a fresh commit-graph:\n\n        git commit-graph write\n\n3. next, we run pack-refs:\n\n        git pack-refs --all\n\n4. after that, we run prune:\n\n        git prune --expire=yesterday\n\nIn the case of this particular repository, we regularly run into repository\ncorruption, reported during the prune stage:\n\n    fsck[10362] 2022-05-09 01:00:06,378 - INFO - /var/lib/gitolite3/repositories/poky-buildhistory.git:\n    fsck[10362] 2022-05-09 01:00:06,700 - INFO -    repack: performing a full repack for optimal deltas\n    fsck[10362] 2022-05-09 01:00:06,701 - INFO -    repack: repacking with \"-n --window-memory=1g -a -b --unpack-unreachable=yesterday -f --pack-kept-objects -d\"\n    fsck[10362] 2022-05-09 03:19:15,825 - INFO -     graph: generating commit-graph\n    fsck[10362] 2022-05-09 03:19:20,830 - INFO -  packrefs: repacking all refs\n    fsck[10362] 2022-05-09 03:19:20,842 - INFO -     prune: pruning\n    fsck[10362] 2022-05-09 03:19:21,622 - CRITICAL - /var/lib/gitolite3/repositories/poky-buildhistory.git reports errors:\n    fsck[10362] 2022-05-09 03:19:21,625 - CRITICAL -        fatal: bad tree object ace77888c63e5c4e545f1bd7a3ee5934e35f56e9\n    fsck[10362] 2022-05-09 03:19:21,626 - WARNING - Repacking /var/lib/gitolite3/repositories/poky-buildhistory.git was unsuccessful\n\nThe tree object in question came in during the repack stage:\n\n    2022-05-09.02:36:33     11129   update  poky-buildhistory buildhistory    W       refs/heads/poky/master/qemuppc  5aad6c8130370bf22f5639162bbbfeaefd0fcd5e ea4e65d72a6161fece5734f7b111e31af77c7578        refs/.*\n\nAs far as I know, the maintenance steps we are running shouldn't result in any\nmissing objects, so I'm curious if it's something we're doing wrong (using\nunsafe flags) or if git isn't properly accounting for some objects that come\nin during the repack stage. We're seeing this happen fairly routinely, so it's\nnot just a random fluke.\n\nGit version 2.36.1 (and earlier).\n\nThanks,\nKonstantin\n"},{"id":"457125","messageId":"YqepoUMb3rkKgWqB@nand.local","threadId":"57990","inReplyTo":"20220613203145.wbpi2m3ys3hchw6c@meerkat.local","subject":"Re: Repository corruption if objects pushed in the middle of repack","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-06-13T21:18:25Z","receivedAt":"2022-06-13T21:27:46Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Konstantin,\n\nOn Mon, Jun 13, 2022 at 04:31:45PM -0400, Konstantin Ryabitsev wrote:\n> As far as I know, the maintenance steps we are running shouldn't result in any\n> missing objects, so I'm curious if it's something we're doing wrong (using\n> unsafe flags) or if git isn't properly accounting for some objects that come\n> in during the repack stage. We're seeing this happen fairly routinely, so it's\n> not just a random fluke.\n\nInteresting. From what you described, it does suggest that `repack` is\ndeleting things too eagerly.\n\nBut I would be quite surprised if that were the case, since `repack` is\n*very* careful to only delete packs which it knew about at the start of\nthe repack. Likewise, when it cleans up loose objects, it only unlinks\nobjects which can be found in some (non-deleted) pack that will remain\nin the repository.\n\nSo I doubt that repack is doing something weird here, though it would be\nextremely interseting if you were able to pause the `repack` process at\na specific point, push new objects into the repository, and reliably\ndemonstrate the corruption.\n\nThanks,\nTaylor\n"},{"id":"457126","messageId":"YqerC883GiwHiiZU@nand.local","threadId":"57990","inReplyTo":"YqepoUMb3rkKgWqB@nand.local","subject":"Re: Repository corruption if objects pushed in the middle of repack","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-06-13T21:24:27Z","receivedAt":"2022-06-13T21:30:03Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Jun 13, 2022 at 05:18:25PM -0400, Taylor Blau wrote:\n> Hi Konstantin,\n>\n> On Mon, Jun 13, 2022 at 04:31:45PM -0400, Konstantin Ryabitsev wrote:\n> > As far as I know, the maintenance steps we are running shouldn't result in any\n> > missing objects, so I'm curious if it's something we're doing wrong (using\n> > unsafe flags) or if git isn't properly accounting for some objects that come\n> > in during the repack stage. We're seeing this happen fairly routinely, so it's\n> > not just a random fluke.\n>\n> [...]\n>\n> So I doubt that repack is doing something weird here, though it would be\n> extremely interseting if you were able to pause the `repack` process at\n> a specific point, push new objects into the repository, and reliably\n> demonstrate the corruption.\n\nA much more likely explanation for what is going on has to do with the\n`--unpack-unreachable` option you're using.\n\nIn your example, any unreachable object written within the last day is\nwritten loose, and anything else older than that is simply discarded. If\nthe following happens, in order:\n\n  - an unreachable object is detected, and marked for deletion\n  - that object then becomes reachable via some ref-update\n  - then the object becomes an ancestor of some push which depends on it\n  - _then_ the object is deleted by repack\n\n...then the repository will be missing some objects which are in its\nreachable set, and thus corrupt. IOW, the `--unpack-unreachable` option\n(and its successor, cruft packs) are both racy with respect to\nref-updates.\n\nAre you able to find evidence of that race in your logging? I would bet\nthat is likely what is going on here.\n\nThanks,\nTaylor\n"},{"id":"457127","messageId":"20220613213221.iekmfjihho5ujfq2@meerkat.local","threadId":"57990","inReplyTo":"YqerC883GiwHiiZU@nand.local","subject":"Re: Repository corruption if objects pushed in the middle of repack","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2022-06-13T21:32:21Z","receivedAt":"2022-06-13T21:33:10Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Mon, Jun 13, 2022 at 05:24:27PM -0400, Taylor Blau wrote:\n> A much more likely explanation for what is going on has to do with the\n> `--unpack-unreachable` option you're using.\n> \n> In your example, any unreachable object written within the last day is\n> written loose, and anything else older than that is simply discarded. If\n> the following happens, in order:\n> \n>   - an unreachable object is detected, and marked for deletion\n>   - that object then becomes reachable via some ref-update\n>   - then the object becomes an ancestor of some push which depends on it\n>   - _then_ the object is deleted by repack\n> \n> ...then the repository will be missing some objects which are in its\n> reachable set, and thus corrupt. IOW, the `--unpack-unreachable` option\n> (and its successor, cruft packs) are both racy with respect to\n> ref-updates.\n> \n> Are you able to find evidence of that race in your logging? I would bet\n> that is likely what is going on here.\n\nI'm not sure that's the case, because the object that is missing is the one\nthat didn't exist before the repack started. In the scenario you describe, the\npre-existing unreachable ancestor of it would be missing, not the newly\nincoming object. Right?\n\n-K\n"},{"id":"457129","messageId":"Yqet68Sll1cz+ySY@nand.local","threadId":"57990","inReplyTo":"20220613213221.iekmfjihho5ujfq2@meerkat.local","subject":"Re: Repository corruption if objects pushed in the middle of repack","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-06-13T21:36:43Z","receivedAt":"2022-06-13T21:36:48Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Jun 13, 2022 at 05:32:21PM -0400, Konstantin Ryabitsev wrote:\n> On Mon, Jun 13, 2022 at 05:24:27PM -0400, Taylor Blau wrote:\n> > A much more likely explanation for what is going on has to do with the\n> > `--unpack-unreachable` option you're using.\n> >\n> > In your example, any unreachable object written within the last day is\n> > written loose, and anything else older than that is simply discarded. If\n> > the following happens, in order:\n> >\n> >   - an unreachable object is detected, and marked for deletion\n> >   - that object then becomes reachable via some ref-update\n> >   - then the object becomes an ancestor of some push which depends on it\n> >   - _then_ the object is deleted by repack\n> >\n> > ...then the repository will be missing some objects which are in its\n> > reachable set, and thus corrupt. IOW, the `--unpack-unreachable` option\n> > (and its successor, cruft packs) are both racy with respect to\n> > ref-updates.\n> >\n> > Are you able to find evidence of that race in your logging? I would bet\n> > that is likely what is going on here.\n>\n> I'm not sure that's the case, because the object that is missing is the one\n> that didn't exist before the repack started. In the scenario you describe, the\n> pre-existing unreachable ancestor of it would be missing, not the newly\n> incoming object. Right?\n\nAren't we reporting that the newly pushed tree was broken _because_ it\nhad some links to sub-trees that no longer existed?\n\nThanks,\nTaylor\n"},{"id":"457130","messageId":"20220613214514.udtn35w7j6q2lrbh@meerkat.local","threadId":"57990","inReplyTo":"Yqet68Sll1cz+ySY@nand.local","subject":"Re: Repository corruption if objects pushed in the middle of repack","fromName":"Konstantin Ryabitsev","fromEmail":"konstantin@linuxfoundation.org","sentAt":"2022-06-13T21:45:14Z","receivedAt":"2022-06-13T21:45:21Z","isPatch":false,"sender":{"key":"konstantin@linuxfoundation.org","avatar":"https://gravatar.com/avatar/7cb8827c6de56e1bd2dea16508c6708aa43feed3bf3813bcdacecdf96ceadd79?d=mp&s=160"},"body":"On Mon, Jun 13, 2022 at 05:36:43PM -0400, Taylor Blau wrote:\n> > I'm not sure that's the case, because the object that is missing is the one\n> > that didn't exist before the repack started. In the scenario you describe, the\n> > pre-existing unreachable ancestor of it would be missing, not the newly\n> > incoming object. Right?\n> \n> Aren't we reporting that the newly pushed tree was broken _because_ it\n> had some links to sub-trees that no longer existed?\n\nHmm... now I'm not sure, and I don't have the broken repo in front of me any\nmore. :(\n\nWell, the upside of this happening on a routine basis is that I can make\na copy of it next time so I can be more helpful in troubleshooting this\nsituation. Let me sit on this and make some copies next time this happens\n(even if it's super annoying that it happens to such a large repo), and then\nperhaps I can give you a better answer.\n\nIt's just strange that we've been doing something similar like this to tens of\nthousands of repositories (e.g. those on codeaurora.org), and it's the first\ntime that I see such consistent corruption manifest itself. If I were to go\nwith my gut instinct, I would blame the shallow checkout on the client, but I\ndon't have any good way of explaining why that would be the culprit either.\n\n-K\n"},{"id":"457134","messageId":"CAPx1GveKAaO1NMRZvcoj5nGm4Cd=A5CLkq7u-eOYtRvU2YVaGw@mail.gmail.com","threadId":"57990","inReplyTo":"20220613214514.udtn35w7j6q2lrbh@meerkat.local","subject":"Re: Repository corruption if objects pushed in the middle of repack","fromName":"Chris Torek","fromEmail":"chris.torek@gmail.com","sentAt":"2022-06-13T22:26:59Z","receivedAt":"2022-06-13T22:27:14Z","isPatch":false,"sender":{"key":"chris.torek@gmail.com","avatar":"https://avatars.githubusercontent.com/u/16826774?v=4"},"body":"On Mon, Jun 13, 2022 at 3:07 PM Konstantin Ryabitsev\n<konstantin@linuxfoundation.org> wrote:\n> It's just strange that we've been doing something similar like this to tens of\n> thousands of repositories (e.g. those on codeaurora.org), and it's the first\n> time that I see such consistent corruption manifest itself. If I were to go\n> with my gut instinct, I would blame the shallow checkout on the client, but I\n> don't have any good way of explaining why that would be the culprit either.\n\nOne thing that *is* different with a shallow clone followed by push, is\nthat the `git push` pushes a lot of objects unnecessarily because the\nclient doesn't have the commits to prove that they're unnecessary. So\nthe delivered pack file has a *lot* of redundant objects.\n\n(A `--depth 2` clone usually omits most redundant objects, which is a\nreason to use `--depth 2` instead of `--depth 1`.)\n\nChris\n"}]}