{"thread":{"id":"58916","subject":"Question: How to execute git-gc correctly on the git server","startedAt":"2022-12-07T15:58:30Z","lastAt":"2022-12-14T20:24:15Z","messageCount":14,"participants":["ZheNing Hu","Ævar Arnfjörð Bjarmason","Michal Suchánek","Jeff King","Taylor Blau"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"468727","messageId":"CAOLTT8Tt3jW2yvm6BRU3yG+EvW1WG9wWFq6PuOcaHNNLQAaGjg@mail.gmail.com","threadId":"58916","inReplyTo":null,"subject":"Question: How to execute git-gc correctly on the git server","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-12-07T15:58:13Z","receivedAt":"2022-12-07T15:58:30Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Hi,\n\nI would like to run git gc on my git server periodically, which should help\nreduce storage space and optimize the read performance of the repository.\nI know github, gitlab all have this process...\n\nBut the concurrency between git gc and other git commands is holding\nme back a bit.\n\ngit-gc [1] docs say:\n\n    On the other hand, when git gc runs concurrently with another process,\n    there is a risk of it deleting an object that the other process is using but\n    hasn’t created a reference to. This may just cause the other process to\n    fail or may corrupt the repository if the other process later adds\na reference\n    to the deleted object.\n\nIt seems that git gc is a dangerous operation that may cause data corruption\nconcurrently with other git commands.\n\nThen I read the contents of Github's blog [2], git gc ---cruft seems to be used\nto keep those expiring unreachable objects in a cruft pack, but the blog says\ngithub use some special \"limbo\" repository to keep the cruft pack for git data\nrecover. Well, a lot of the details here are pretty hard to understand for me :(\n\nHowever, on the other hand, my git server is still at v2.35, and --cruft was\nintroduced in v2.38, so I'm actually more curious about: how did the server\nexecute git gc correctly in the past? Do we need a repository level \"big lock\"\nthat blocks most/all other git operations? What should the behavior of users'\ngit clone/push be at this time? Report error that the git server is performing\ngit gc? Or just wait for git gc to complete?\n\nThanks for any comments and help!\n\n[1]: https://git-scm.com/docs/git-gc\n[2]: https://github.blog/2022-09-13-scaling-gits-garbage-collection/\n"},{"id":"468756","messageId":"221208.86a63y9309.gmgdl@evledraar.gmail.com","threadId":"58916","inReplyTo":"CAOLTT8Tt3jW2yvm6BRU3yG+EvW1WG9wWFq6PuOcaHNNLQAaGjg@mail.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-07T23:57:45Z","receivedAt":"2022-12-08T00:02:23Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Dec 07 2022, ZheNing Hu wrote:\n\n> I would like to run git gc on my git server periodically, which should help\n> reduce storage space and optimize the read performance of the repository.\n> I know github, gitlab all have this process...\n>\n> But the concurrency between git gc and other git commands is holding\n> me back a bit.\n>\n> git-gc [1] docs say:\n>\n>     On the other hand, when git gc runs concurrently with another process,\n>     there is a risk of it deleting an object that the other process is using but\n>     hasn’t created a reference to. This may just cause the other process to\n>     fail or may corrupt the repository if the other process later adds\n> a reference\n>     to the deleted object.\n>\n> It seems that git gc is a dangerous operation that may cause data corruption\n> concurrently with other git commands.\n>\n> Then I read the contents of Github's blog [2], git gc ---cruft seems to be used\n> to keep those expiring unreachable objects in a cruft pack, but the blog says\n> github use some special \"limbo\" repository to keep the cruft pack for git data\n> recover. Well, a lot of the details here are pretty hard to understand for me :(\n>\n> However, on the other hand, my git server is still at v2.35, and --cruft was\n> introduced in v2.38, so I'm actually more curious about: how did the server\n> execute git gc correctly in the past? Do we need a repository level \"big lock\"\n> that blocks most/all other git operations? What should the behavior of users'\n> git clone/push be at this time? Report error that the git server is performing\n> git gc? Or just wait for git gc to complete?\n>\n> Thanks for any comments and help!\n>\n> [1]: https://git-scm.com/docs/git-gc\n> [2]: https://github.blog/2022-09-13-scaling-gits-garbage-collection/\n\nIs this for a very large hosting site that's anywhere near GitHub,\nGitLab's etc. scale?\n\nA \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\nyou'll run into data corrupting trouble tends to approach zero as you\nincrease the gc.pruneExpire setting, with the default 2 weeks being more\nthan enough for even the most paranoid user.\n\nThe \"cruft pack\" facility does many different things, and my\nunderstanding of it is that GitHub's not using it only as an end-run\naround potential corruption issues, but that some not yet in tree\npatches on top of it allow more aggressive \"gc\" without the fear of\ncorruption.\n\nSo, I think you probably don't need to worry about it. Other major\nhosting sites do run \"git gc\" on live repositories, but as always take\nbackups etc.\n"},{"id":"468762","messageId":"20221208011631.GH28810@kitsune.suse.cz","threadId":"58916","inReplyTo":"221208.86a63y9309.gmgdl@evledraar.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Michal Suchánek","fromEmail":"msuchanek@suse.de","sentAt":"2022-12-08T01:16:31Z","receivedAt":"2022-12-08T01:16:38Z","isPatch":false,"sender":{"key":"msuchanek@suse.de","avatar":"https://avatars.githubusercontent.com/u/787652?v=4"},"body":"On Thu, Dec 08, 2022 at 12:57:45AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Dec 07 2022, ZheNing Hu wrote:\n> \n> > I would like to run git gc on my git server periodically, which should help\n> > reduce storage space and optimize the read performance of the repository.\n> > I know github, gitlab all have this process...\n> >\n> > But the concurrency between git gc and other git commands is holding\n> > me back a bit.\n> >\n> > git-gc [1] docs say:\n> >\n> >     On the other hand, when git gc runs concurrently with another process,\n> >     there is a risk of it deleting an object that the other process is using but\n> >     hasn’t created a reference to. This may just cause the other process to\n> >     fail or may corrupt the repository if the other process later adds\n> > a reference\n> >     to the deleted object.\n> >\n> > It seems that git gc is a dangerous operation that may cause data corruption\n> > concurrently with other git commands.\n> >\n> > Then I read the contents of Github's blog [2], git gc ---cruft seems to be used\n> > to keep those expiring unreachable objects in a cruft pack, but the blog says\n> > github use some special \"limbo\" repository to keep the cruft pack for git data\n> > recover. Well, a lot of the details here are pretty hard to understand for me :(\n> >\n> > However, on the other hand, my git server is still at v2.35, and --cruft was\n> > introduced in v2.38, so I'm actually more curious about: how did the server\n> > execute git gc correctly in the past? Do we need a repository level \"big lock\"\n> > that blocks most/all other git operations? What should the behavior of users'\n> > git clone/push be at this time? Report error that the git server is performing\n> > git gc? Or just wait for git gc to complete?\n> >\n> > Thanks for any comments and help!\n> >\n> > [1]: https://git-scm.com/docs/git-gc\n> > [2]: https://github.blog/2022-09-13-scaling-gits-garbage-collection/\n> \n> Is this for a very large hosting site that's anywhere near GitHub,\n> GitLab's etc. scale?\n> \n> A \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\n> you'll run into data corrupting trouble tends to approach zero as you\n> increase the gc.pruneExpire setting, with the default 2 weeks being more\n> than enough for even the most paranoid user.\n\nAnd that two weeks expiration applies to what, exactly?\n\nFor commits there is author date and commit date but many other objecs\nwon't have these I suppose. And the date when the object is pushed into\nthe repository is unrelated to these two, anyway.\n\n> So, I think you probably don't need to worry about it. Other major\n> hosting sites do run \"git gc\" on live repositories, but as always take\n> backups etc.\n\nActually, it is a real problem. With <100 users and some scripting I got\nunexplained repository corruptions which went away when gc was disabled.\n\nYMMV\n\nBad locking design is always a landmine waiting to get triggered. If you\nstep carefully you might avoid it for some time.\n\nThanks\n\nMichal\n"},{"id":"468769","messageId":"Y5GLWe4Kdaz+T5P8@coredump.intra.peff.net","threadId":"58916","inReplyTo":"221208.86a63y9309.gmgdl@evledraar.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-08T06:59:37Z","receivedAt":"2022-12-08T06:59:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 08, 2022 at 12:57:45AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Is this for a very large hosting site that's anywhere near GitHub,\n> GitLab's etc. scale?\n> \n> A \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\n> you'll run into data corrupting trouble tends to approach zero as you\n> increase the gc.pruneExpire setting, with the default 2 weeks being more\n> than enough for even the most paranoid user.\n\nI'm a bit less optimistic than \"tends to approach zero\" here. The\nobjects themselves might be older, but they may become referenced or\nunreferenced in an immediate racy way. E.g., intending to move branch Z\nto branch A, a client asks to add A and remove Z. Since there is no\natomic view of the ref namespace, a simultaneous gc might see Z gone,\nbut A not yet existing (and of course it could also be _two_ clients,\netc).\n\n> The \"cruft pack\" facility does many different things, and my\n> understanding of it is that GitHub's not using it only as an end-run\n> around potential corruption issues, but that some not yet in tree\n> patches on top of it allow more aggressive \"gc\" without the fear of\n> corruption.\n\nI don't think cruft packs themselves help against corruption that much.\nFor many years, GitHub used \"repack -k\" to just never expire objects.\nWhat cruft packs help with is:\n\n  1. They keep cruft objects out of the main pack, which reduces the\n     costs of lookups and bitmaps for the main pack.\n\n  2. When you _do_ choose to expire, you can do so without worrying\n     about accidentally exploding all of those old objects into loose\n     ones (which is not wrong from a correctness point of view, but can\n     have some amazingly bad performance characteristics).\n\nI think the bits you're thinking of on top are in v2.39. The \"repack\n--expire-to\" option lets you write objects that _would_ be deleted into\na cruft pack, which can serve as a backup (but managing that is out of\nscope for repack itself, so you have to roll your own strategy there).\n\n-Peff\n"},{"id":"468770","messageId":"Y5GLsZgmrxbBtLqo@coredump.intra.peff.net","threadId":"58916","inReplyTo":"20221208011631.GH28810@kitsune.suse.cz","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-08T07:01:05Z","receivedAt":"2022-12-08T07:01:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 08, 2022 at 02:16:31AM +0100, Michal Suchánek wrote:\n\n> > A \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\n> > you'll run into data corrupting trouble tends to approach zero as you\n> > increase the gc.pruneExpire setting, with the default 2 weeks being more\n> > than enough for even the most paranoid user.\n> \n> And that two weeks expiration applies to what, exactly?\n> \n> For commits there is author date and commit date but many other objecs\n> won't have these I suppose. And the date when the object is pushed into\n> the repository is unrelated to these two, anyway.\n\nIn this case it's the mtime on the object file (or the pack containing\nit). But yes, it is far from a complete race-free solution.\n\n-Peff\n"},{"id":"468782","messageId":"221208.86o7se6ou1.gmgdl@evledraar.gmail.com","threadId":"58916","inReplyTo":"Y5GLWe4Kdaz+T5P8@coredump.intra.peff.net","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-08T12:35:04Z","receivedAt":"2022-12-08T12:52:15Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 08 2022, Jeff King wrote:\n\n> On Thu, Dec 08, 2022 at 12:57:45AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> Is this for a very large hosting site that's anywhere near GitHub,\n>> GitLab's etc. scale?\n>> \n>> A \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\n>> you'll run into data corrupting trouble tends to approach zero as you\n>> increase the gc.pruneExpire setting, with the default 2 weeks being more\n>> than enough for even the most paranoid user.\n>\n> I'm a bit less optimistic than \"tends to approach zero\" here. The\n> objects themselves might be older, but they may become referenced or\n> unreferenced in an immediate racy way. E.g., intending to move branch Z\n> to branch A, a client asks to add A and remove Z. Since there is no\n> atomic view of the ref namespace, a simultaneous gc might see Z gone,\n> but A not yet existing (and of course it could also be _two_ clients,\n> etc).\n\nYes, I'm really hand-waiving away the issue there for brevity.\n\nYou've got a really good summary of why exactly that race happens\nsomewhere in the list archive, which I'm not digging up now.\n\nBut (and just for the general audience here, I know you know this) that\nrace basically happens because \"gc\" concurrently decides that say a 2\nweek old blob containing \"foo\" isn't referenced, *and* we have a\nconcurrent branch push that happens to reference a \"foo\" blob, along\nwith a concurrent \"gc\".\n\nI have run into this once or twice in practice (with a very high volume\nin-house git server), and in those cases it was because person \"B\" doing\nthe new push was using person's \"A\"'s work to push a new branch, after it\nhad been ~gc.pruneExpire since the topic branch for \"A\" was\nsimultaneously being deleted.\n\nIn principle what I noted upthread is completely false, but I think in\npractice it's almost always true. I.e. users aren't pushing random\nblobs, and as time goes by the odds that the exact same content is\nre-pushed go down.\n\nIt's also worth noting that some repositories are much more vulnerable\nto this than others.\n\nIf you have predictable auto-generated content in your repo (think the\npackage+version list some languages routinely carry in-tree) you're much\nmore likely to run into this: Someone bumped that for a topic ~2 weeks\nago, nobody else bothered on any of their branches, and then the \"A\"+\"B\"\nrace described above happens.\n\nAs some practical advice to those running \"gc\" on live repos: To easily\nmitigate this run expiry on the least busy hours of the day. Even for\ntruly global development teams there's usually a big lull when it's high\nnoon in the middle of the Pacific.\n\n>> The \"cruft pack\" facility does many different things, and my\n>> understanding of it is that GitHub's not using it only as an end-run\n>> around potential corruption issues, but that some not yet in tree\n>> patches on top of it allow more aggressive \"gc\" without the fear of\n>> corruption.\n>\n> I don't think cruft packs themselves help against corruption that much.\n> For many years, GitHub used \"repack -k\" to just never expire objects.\n> What cruft packs help with is:\n>\n>   1. They keep cruft objects out of the main pack, which reduces the\n>      costs of lookups and bitmaps for the main pack.\n>\n>   2. When you _do_ choose to expire, you can do so without worrying\n>      about accidentally exploding all of those old objects into loose\n>      ones (which is not wrong from a correctness point of view, but can\n>      have some amazingly bad performance characteristics).\n>\n> I think the bits you're thinking of on top are in v2.39. The \"repack\n> --expire-to\" option lets you write objects that _would_ be deleted into\n> a cruft pack, which can serve as a backup (but managing that is out of\n> scope for repack itself, so you have to roll your own strategy there).\n\nYes, that's what I was referring to.\n\nI think I had feedback on that series saying that if held correctly this\nwould also nicely solve that long-time race. Maybe I'm just\nmisremembering, but I (mis?)recalled that Taylor indicated that it was\nbeing used like that at GitHub.\n\nAnother thing that really helps to mitigate it is this never-in-tree\npatch of mine (which ran in busy production for years, and probably\nstill):\nhttps://lore.kernel.org/git/20181028225023.26427-1-avarab@gmail.com/\n\nIt's sensitive to \"transfer.unpackLimit\" if it's going to help with\nthat, and even if you always write duplicate content it won't fully help\nwith the race, as \"B\" may have seen the old ref, and hence not sent the\n\"foo\" blob over (so the client would need to not have a copy of \"A\"'s\nabout-to-be-deleted topic).\n\nAll of which described a setup I ran it in, so *if* we ever ran into the\nrace then most likely we'd just have written duplicate content in the\nincoming PACK, so we'd happily chug along.\n\n"},{"id":"468806","messageId":"20221209004918.GI28810@kitsune.suse.cz","threadId":"58916","inReplyTo":"Y5GLsZgmrxbBtLqo@coredump.intra.peff.net","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Michal Suchánek","fromEmail":"msuchanek@suse.de","sentAt":"2022-12-09T00:49:18Z","receivedAt":"2022-12-09T00:49:24Z","isPatch":false,"sender":{"key":"msuchanek@suse.de","avatar":"https://avatars.githubusercontent.com/u/787652?v=4"},"body":"On Thu, Dec 08, 2022 at 02:01:05AM -0500, Jeff King wrote:\n> On Thu, Dec 08, 2022 at 02:16:31AM +0100, Michal Suchánek wrote:\n> \n> > > A \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\n> > > you'll run into data corrupting trouble tends to approach zero as you\n> > > increase the gc.pruneExpire setting, with the default 2 weeks being more\n> > > than enough for even the most paranoid user.\n> > \n> > And that two weeks expiration applies to what, exactly?\n> > \n> > For commits there is author date and commit date but many other objecs\n> > won't have these I suppose. And the date when the object is pushed into\n> > the repository is unrelated to these two, anyway.\n> \n> In this case it's the mtime on the object file (or the pack containing\n> it). But yes, it is far from a complete race-free solution.\n\nSo if you are pushing a branch that happens to reuse commits or other\nobjects from an earlier branh that might have been collected ín the\nmeantime you are basically doomed.\n\nHow likely that is depends a lot on your workflow.\n\nPeople deleting a branch and then pushing another variant in which many\nobjects are the same is a risk.\n\nPeople exporting files from somewhere and adding them to the repo which\nare bit-identical when independently exported by multiple people and\nsometimes deleting branches is a risk.\n\n...\n\nThanks\n\nMichal\n"},{"id":"468809","messageId":"Y5KRQMcUlepwNlor@coredump.intra.peff.net","threadId":"58916","inReplyTo":"20221209004918.GI28810@kitsune.suse.cz","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-09T01:37:04Z","receivedAt":"2022-12-09T01:37:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 09, 2022 at 01:49:18AM +0100, Michal Suchánek wrote:\n\n> > In this case it's the mtime on the object file (or the pack containing\n> > it). But yes, it is far from a complete race-free solution.\n> \n> So if you are pushing a branch that happens to reuse commits or other\n> objects from an earlier branh that might have been collected ín the\n> meantime you are basically doomed.\n\nBasically yes. We do \"freshen\" the mtimes on object files when we omit\nan object write (e.g., your index state ends up at the same tree as an\nold one). But for a push, there is no freshening. We check the graph at\nthe time of the push and decide if we have everything we need (either\nnewly pushed, or from what we already had in the repo). And that is\nwhat's racy; somebody might be deleting as that check is happening.\n\n> People deleting a branch and then pushing another variant in which many\n> objects are the same is a risk.\n> \n> People exporting files from somewhere and adding them to the repo which\n> are bit-identical when independently exported by multiple people and\n> sometimes deleting branches is a risk.\n\nYes, both of those are risky (along with many other variants).\n\n-Peff\n"},{"id":"468819","messageId":"CAOLTT8TMWzmxRD+sPiNuCQ12iLB-qt__q_Y1m_hDUZ_ggExyRg@mail.gmail.com","threadId":"58916","inReplyTo":"20221208011631.GH28810@kitsune.suse.cz","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-12-09T07:15:19Z","receivedAt":"2022-12-09T07:15:37Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Michal Suchánek <msuchanek@suse.de> 于2022年12月8日周四 09:16写道：\n>\n> On Thu, Dec 08, 2022 at 12:57:45AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >\n> > On Wed, Dec 07 2022, ZheNing Hu wrote:\n> >\n> > > I would like to run git gc on my git server periodically, which should help\n> > > reduce storage space and optimize the read performance of the repository.\n> > > I know github, gitlab all have this process...\n> > >\n> > > But the concurrency between git gc and other git commands is holding\n> > > me back a bit.\n> > >\n> > > git-gc [1] docs say:\n> > >\n> > >     On the other hand, when git gc runs concurrently with another process,\n> > >     there is a risk of it deleting an object that the other process is using but\n> > >     hasn’t created a reference to. This may just cause the other process to\n> > >     fail or may corrupt the repository if the other process later adds\n> > > a reference\n> > >     to the deleted object.\n> > >\n> > > It seems that git gc is a dangerous operation that may cause data corruption\n> > > concurrently with other git commands.\n> > >\n> > > Then I read the contents of Github's blog [2], git gc ---cruft seems to be used\n> > > to keep those expiring unreachable objects in a cruft pack, but the blog says\n> > > github use some special \"limbo\" repository to keep the cruft pack for git data\n> > > recover. Well, a lot of the details here are pretty hard to understand for me :(\n> > >\n> > > However, on the other hand, my git server is still at v2.35, and --cruft was\n> > > introduced in v2.38, so I'm actually more curious about: how did the server\n> > > execute git gc correctly in the past? Do we need a repository level \"big lock\"\n> > > that blocks most/all other git operations? What should the behavior of users'\n> > > git clone/push be at this time? Report error that the git server is performing\n> > > git gc? Or just wait for git gc to complete?\n> > >\n> > > Thanks for any comments and help!\n> > >\n> > > [1]: https://git-scm.com/docs/git-gc\n> > > [2]: https://github.blog/2022-09-13-scaling-gits-garbage-collection/\n> >\n> > Is this for a very large hosting site that's anywhere near GitHub,\n> > GitLab's etc. scale?\n> >\n> > A \"git gc\" on a \"live\" repo is always racy in theory, but the odds that\n> > you'll run into data corrupting trouble tends to approach zero as you\n> > increase the gc.pruneExpire setting, with the default 2 weeks being more\n> > than enough for even the most paranoid user.\n>\n> And that two weeks expiration applies to what, exactly?\n>\n> For commits there is author date and commit date but many other objecs\n> won't have these I suppose. And the date when the object is pushed into\n> the repository is unrelated to these two, anyway.\n>\n> > So, I think you probably don't need to worry about it. Other major\n> > hosting sites do run \"git gc\" on live repositories, but as always take\n> > backups etc.\n>\n> Actually, it is a real problem. With <100 users and some scripting I got\n> unexplained repository corruptions which went away when gc was disabled.\n>\n> YMMV\n>\n> Bad locking design is always a landmine waiting to get triggered. If you\n> step carefully you might avoid it for some time.\n>\n\nI agree with this. What I hope to be able to do more is \"no error at all\"\nrather than \"small probability of error\"\n\n> Thanks\n>\n> Michal\n"},{"id":"468820","messageId":"CAOLTT8SR6JWX6mRLbyq4keb4JCfJP6Vq07LzHpb_f+e1jMnsZQ@mail.gmail.com","threadId":"58916","inReplyTo":"Y5KRQMcUlepwNlor@coredump.intra.peff.net","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-12-09T07:26:52Z","receivedAt":"2022-12-09T07:28:12Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Jeff King <peff@peff.net> 于2022年12月9日周五 09:37写道：\n>\n> On Fri, Dec 09, 2022 at 01:49:18AM +0100, Michal Suchánek wrote:\n>\n> > > In this case it's the mtime on the object file (or the pack containing\n> > > it). But yes, it is far from a complete race-free solution.\n> >\n> > So if you are pushing a branch that happens to reuse commits or other\n> > objects from an earlier branh that might have been collected ín the\n> > meantime you are basically doomed.\n>\n> Basically yes. We do \"freshen\" the mtimes on object files when we omit\n> an object write (e.g., your index state ends up at the same tree as an\n> old one). But for a push, there is no freshening. We check the graph at\n> the time of the push and decide if we have everything we need (either\n> newly pushed, or from what we already had in the repo). And that is\n> what's racy; somebody might be deleting as that check is happening.\n>\n> > People deleting a branch and then pushing another variant in which many\n> > objects are the same is a risk.\n> >\n> > People exporting files from somewhere and adding them to the repo which\n> > are bit-identical when independently exported by multiple people and\n> > sometimes deleting branches is a risk.\n>\n> Yes, both of those are risky (along with many other variants).\n>\n\nI'm wondering if there's an easy and poor performance way to do\ngc safely? For example, add a file lock to the repository during\ngit push and git gc?\n\n> -Peff\n"},{"id":"468826","messageId":"221209.86bkoc7kgi.gmgdl@evledraar.gmail.com","threadId":"58916","inReplyTo":"CAOLTT8SR6JWX6mRLbyq4keb4JCfJP6Vq07LzHpb_f+e1jMnsZQ@mail.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-09T13:48:14Z","receivedAt":"2022-12-09T13:52:52Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 09 2022, ZheNing Hu wrote:\n\n> Jeff King <peff@peff.net> 于2022年12月9日周五 09:37写道：\n>>\n>> On Fri, Dec 09, 2022 at 01:49:18AM +0100, Michal Suchánek wrote:\n>>\n>> > > In this case it's the mtime on the object file (or the pack containing\n>> > > it). But yes, it is far from a complete race-free solution.\n>> >\n>> > So if you are pushing a branch that happens to reuse commits or other\n>> > objects from an earlier branh that might have been collected ín the\n>> > meantime you are basically doomed.\n>>\n>> Basically yes. We do \"freshen\" the mtimes on object files when we omit\n>> an object write (e.g., your index state ends up at the same tree as an\n>> old one). But for a push, there is no freshening. We check the graph at\n>> the time of the push and decide if we have everything we need (either\n>> newly pushed, or from what we already had in the repo). And that is\n>> what's racy; somebody might be deleting as that check is happening.\n>>\n>> > People deleting a branch and then pushing another variant in which many\n>> > objects are the same is a risk.\n>> >\n>> > People exporting files from somewhere and adding them to the repo which\n>> > are bit-identical when independently exported by multiple people and\n>> > sometimes deleting branches is a risk.\n>>\n>> Yes, both of those are risky (along with many other variants).\n>>\n>\n> I'm wondering if there's an easy and poor performance way to do\n> gc safely? For example, add a file lock to the repository during\n> git push and git gc?\n\nWe don't have any \"easy\" way to do it, but we probably should. The root\ncause of the race is tricky to fix, and we don't have any \"global ref\nlock\".\n\nBut in the context of a client<->server and wanting to do gc on the\nserver a good enough and easy solution would be e.g.:\n\n 1. Have a {pre,post}-receive hook logging attempted/finished pushes\n 2. Have the pre-receive hook able to reject (or better yet, hang with\n    sleep()) incoming deletions\n 3. Do a gc with a small wrapper script, which:\n    - Flips the \"no deletion ops now\" (or \"delay deletion ops\") switch\n    - Polls until it's sure there's no relevant in-progress operations\n    - Do a full gc\n    - Unlock\n\nYou'd need to be certain that all relevant repo operations are going\nthrough git-receive-pack etc., i.e. a local \"git branch -d\" or the like\nwon't run {pre,post}-receive.\n"},{"id":"468866","messageId":"CAOLTT8SqFJFk2sO6quL_O8gm4FL-w+mbvjH+cg2L7OF3G_-mLw@mail.gmail.com","threadId":"58916","inReplyTo":"221209.86bkoc7kgi.gmgdl@evledraar.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2022-12-11T16:01:51Z","receivedAt":"2022-12-11T16:02:08Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2022年12月9日周五 21:52写道：\n\n>\n>\n> On Fri, Dec 09 2022, ZheNing Hu wrote:\n>\n> > Jeff King <peff@peff.net> 于2022年12月9日周五 09:37写道：\n> >>\n> >> On Fri, Dec 09, 2022 at 01:49:18AM +0100, Michal Suchánek wrote:\n> >>\n> >> > > In this case it's the mtime on the object file (or the pack containing\n> >> > > it). But yes, it is far from a complete race-free solution.\n> >> >\n> >> > So if you are pushing a branch that happens to reuse commits or other\n> >> > objects from an earlier branh that might have been collected ín the\n> >> > meantime you are basically doomed.\n> >>\n> >> Basically yes. We do \"freshen\" the mtimes on object files when we omit\n> >> an object write (e.g., your index state ends up at the same tree as an\n> >> old one). But for a push, there is no freshening. We check the graph at\n> >> the time of the push and decide if we have everything we need (either\n> >> newly pushed, or from what we already had in the repo). And that is\n> >> what's racy; somebody might be deleting as that check is happening.\n> >>\n> >> > People deleting a branch and then pushing another variant in which many\n> >> > objects are the same is a risk.\n> >> >\n> >> > People exporting files from somewhere and adding them to the repo which\n> >> > are bit-identical when independently exported by multiple people and\n> >> > sometimes deleting branches is a risk.\n> >>\n> >> Yes, both of those are risky (along with many other variants).\n> >>\n> >\n> > I'm wondering if there's an easy and poor performance way to do\n> > gc safely? For example, add a file lock to the repository during\n> > git push and git gc?\n>\n> We don't have any \"easy\" way to do it, but we probably should. The root\n> cause of the race is tricky to fix, and we don't have any \"global ref\n> lock\".\n>\n> But in the context of a client<->server and wanting to do gc on the\n> server a good enough and easy solution would be e.g.:\n>\n>  1. Have a {pre,post}-receive hook logging attempted/finished pushes\n>  2. Have the pre-receive hook able to reject (or better yet, hang with\n>     sleep()) incoming deletions\n>  3. Do a gc with a small wrapper script, which:\n>     - Flips the \"no deletion ops now\" (or \"delay deletion ops\") switch\n>     - Polls until it's sure there's no relevant in-progress operations\n>     - Do a full gc\n>     - Unlock\n>\n\nWell, I understand that after the branch is deleted, some objects may be\nunreachable, and then these objects are deleted by concurrent git gc,\nwhich leads to data corruption in concurrent git push if these objects need\nto be referenced, but what I don't understand that is it enough to just block\nthe operation of deleting branches? Once gc happens to be deleting an\nunreachable object, and git push new branch (git receive-pack) happens to\nneed it, won't it still go wrong?\n\n> You'd need to be certain that all relevant repo operations are going\n> through git-receive-pack etc., i.e. a local \"git branch -d\" or the like\n> won't run {pre,post}-receive.\n"},{"id":"468867","messageId":"20221211162718.GO28810@kitsune.suse.cz","threadId":"58916","inReplyTo":"CAOLTT8SqFJFk2sO6quL_O8gm4FL-w+mbvjH+cg2L7OF3G_-mLw@mail.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Michal Suchánek","fromEmail":"msuchanek@suse.de","sentAt":"2022-12-11T16:27:18Z","receivedAt":"2022-12-11T16:27:27Z","isPatch":false,"sender":{"key":"msuchanek@suse.de","avatar":"https://avatars.githubusercontent.com/u/787652?v=4"},"body":"On Mon, Dec 12, 2022 at 12:01:51AM +0800, ZheNing Hu wrote:\n> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2022年12月9日周五 21:52写道：\n>\n> >\n> >\n> > On Fri, Dec 09 2022, ZheNing Hu wrote:\n> >\n> > > Jeff King <peff@peff.net> 于2022年12月9日周五 09:37写道：\n> > >>\n> > >> On Fri, Dec 09, 2022 at 01:49:18AM +0100, Michal Suchánek wrote:\n> > >>\n> > >> > > In this case it's the mtime on the object file (or the pack containing\n> > >> > > it). But yes, it is far from a complete race-free solution.\n> > >> >\n> > >> > So if you are pushing a branch that happens to reuse commits or other\n> > >> > objects from an earlier branh that might have been collected ín the\n> > >> > meantime you are basically doomed.\n> > >>\n> > >> Basically yes. We do \"freshen\" the mtimes on object files when we omit\n> > >> an object write (e.g., your index state ends up at the same tree as an\n> > >> old one). But for a push, there is no freshening. We check the graph at\n> > >> the time of the push and decide if we have everything we need (either\n> > >> newly pushed, or from what we already had in the repo). And that is\n> > >> what's racy; somebody might be deleting as that check is happening.\n> > >>\n> > >> > People deleting a branch and then pushing another variant in which many\n> > >> > objects are the same is a risk.\n> > >> >\n> > >> > People exporting files from somewhere and adding them to the repo which\n> > >> > are bit-identical when independently exported by multiple people and\n> > >> > sometimes deleting branches is a risk.\n> > >>\n> > >> Yes, both of those are risky (along with many other variants).\n> > >>\n> > >\n> > > I'm wondering if there's an easy and poor performance way to do\n> > > gc safely? For example, add a file lock to the repository during\n> > > git push and git gc?\n> >\n> > We don't have any \"easy\" way to do it, but we probably should. The root\n> > cause of the race is tricky to fix, and we don't have any \"global ref\n> > lock\".\n> >\n> > But in the context of a client<->server and wanting to do gc on the\n> > server a good enough and easy solution would be e.g.:\n> >\n> >  1. Have a {pre,post}-receive hook logging attempted/finished pushes\n> >  2. Have the pre-receive hook able to reject (or better yet, hang with\n> >     sleep()) incoming deletions\n> >  3. Do a gc with a small wrapper script, which:\n> >     - Flips the \"no deletion ops now\" (or \"delay deletion ops\") switch\n> >     - Polls until it's sure there's no relevant in-progress operations\n> >     - Do a full gc\n> >     - Unlock\n> >\n>\n> Well, I understand that after the branch is deleted, some objects may be\n> unreachable, and then these objects are deleted by concurrent git gc,\n> which leads to data corruption in concurrent git push if these objects need\n> to be referenced, but what I don't understand that is it enough to just block\n> the operation of deleting branches? Once gc happens to be deleting an\n> unreachable object, and git push new branch (git receive-pack) happens to\n> need it, won't it still go wrong?\n\nAs I understand the problem:\n\n - push figures out which objects are missing on the remote end\n - push starts sending the missing objects\n - remote gc deletes objects that are not reachable but push assumes remote has them\n   - these might be part of a branch deleted long before gc started\n - push finishes and branch is advanced to point to an object that\n   references objects that were deleted by gc\n\n-> repository is corrupted\n\nThe only way to prevent this is to not delete anything ever, or to make\nsure that objects that are part of any ongoing operation are always\nreferenced.\n\nWhich would probably mean in practice that any operation adding objects\nneeds to add temporary references to any objects it creates or aims to\nreference, and/or check reachability of referenced objects once the\nfinal object is created.\n\nThanks\n\nMichal\n"},{"id":"469039","messageId":"Y5ouDcvjCaRlCGJf@nand.local","threadId":"58916","inReplyTo":"221208.86o7se6ou1.gmgdl@evledraar.gmail.com","subject":"Re: Question: How to execute git-gc correctly on the git server","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-12-14T20:11:57Z","receivedAt":"2022-12-14T20:24:15Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Dec 08, 2022 at 01:35:04PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >> The \"cruft pack\" facility does many different things, and my\n> >> understanding of it is that GitHub's not using it only as an end-run\n> >> around potential corruption issues, but that some not yet in tree\n> >> patches on top of it allow more aggressive \"gc\" without the fear of\n> >> corruption.\n> >\n> > I don't think cruft packs themselves help against corruption that much.\n> > For many years, GitHub used \"repack -k\" to just never expire objects.\n> > What cruft packs help with is:\n> >\n> >   1. They keep cruft objects out of the main pack, which reduces the\n> >      costs of lookups and bitmaps for the main pack.\n\nPeff isn't wrong here, but there is a big caveat which is that this is\nonly true when using a single pack bitmap. Single pack bitmaps are\nguaranteed to have reachability closure over their objects, but writing\na MIDX bitmap after generating the MIDX does not afford us the same\nguarantees.\n\nSo if you have a cruft pack which contains some unreachable object X,\nwhich is made reachable by some other object that *is* reachable from\nsome reference, *and that* object is included in one of the MIDX's\npacks, then we won't have reachability closure unless we also bitmap the\ncruft pack, too.\n\nSo even though it helps a lot with bitmapping in the single-pack case,\nin practice it doesn't make a significant difference with multi-pack\nbitmaps.\n\n> >   2. When you _do_ choose to expire, you can do so without worrying\n> >      about accidentally exploding all of those old objects into loose\n> >      ones (which is not wrong from a correctness point of view, but can\n> >      have some amazingly bad performance characteristics).\n> >\n> > I think the bits you're thinking of on top are in v2.39. The \"repack\n> > --expire-to\" option lets you write objects that _would_ be deleted into\n> > a cruft pack, which can serve as a backup (but managing that is out of\n> > scope for repack itself, so you have to roll your own strategy there).\n>\n> Yes, that's what I was referring to.\n\nYes, we use the `--expire-to` option when doing a pruning GC to move the\nexpired objects out of the repo to some \"../backup.git\" location. The\nout-of-tree tools that Ævar is speculating is basically running\n`cat-file --batch` in the backup repo, feeding it the list of missing\nobjects, and then writing those objects (back) into the GC'd repository.\n\n> I think I had feedback on that series saying that if held correctly this\n> would also nicely solve that long-time race. Maybe I'm just\n> misremembering, but I (mis?)recalled that Taylor indicated that it was\n> being used like that at GitHub.\n\nIt (the above) doesn't solve the race, but it does make it easier to\nrecover from a corrupt repository when we lose that race.\n\nThanks,\nTaylor\n"}]}