{"thread":{"id":"60674","subject":"Concurrent fetch commands","startedAt":"2023-12-31T13:39:48Z","lastAt":"2024-01-04T22:25:43Z","messageCount":20,"participants":["Stefan Haller","Dragan Simic","Konstantin Tokarev","Junio C Hamano","Federico Kircheis","Patrick Steinhardt","Taylor Blau","Mike Hommey"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"486177","messageId":"c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de","threadId":"60674","inReplyTo":null,"subject":"Concurrent fetch commands","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2023-12-31T13:30:05Z","receivedAt":"2023-12-31T13:39:48Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"Currently, git doesn't seem to be very good at handling two concurrent\ninvocations of git fetch (or git fetch and git pull). This is a problem\nbecause it is common for git clients to run git fetch periodically in\nthe background. In that case, when you happen to invoke git pull while\nsuch a background fetch is running, an error occurs (\"Cannot rebase onto\nmultiple branches\").\n\nI can reliably reproduce this by doing\n\n $ git fetch&; sleep 0.1; git pull\n [1] 42160\n [1] + done git fetch\n fatal: Cannot rebase onto multiple branches.\n\nThe reason for this failure seems to be that both the first fetch and\nthe fetch that runs as part of the pull append their information to\n.git/FETCH_HEAD, so that the information for the current branch ends up\ntwice in the file.\n\nDo you think git fetch should be made more robust against scenarios like\nthis?\n\n\nMore context: the git client that I'm contributing to (lazygit) used to\nguard against this for its own background fetch with a global mutex that\nallowed only one single fetch, pull, or push at a time. This solved the\nproblem nicely for lazygit's own operations (at the expense of some lag,\noccasionally); and I'm not aware of any reports about failures because\nsome other git client's background fetch got in the way, so maybe we\ndon't have to worry about that too much.\n\nHowever, we now removed that mutex to allow certain parallel fetch\noperations to run at the same time, most notably fetching (and updating)\na branch that is not checked out (by doing \"git fetch origin\nbranch:branch\"). It is useful to be able to trigger this for multiple\nbranches concurrently, and actually this works fine.\n\nBut now we have the problem described above, where a pull of the\nchecked-out branch runs at the same time as a background fetch; this is\nnot so unlikely, because lazygit triggers the first background fetch at\nstartup, so invoking the pull command right after starting lazygit is\nvery likely to fail.\n\nWe could re-introduce a mutex and just make it a little less global;\ne.g. protect only pull and parameter-less fetch. But fixing it in git\nitself seems preferable to me.\n\nSorry for the wall of text, but I figured giving more context could be\nuseful.\n\nThanks,\nStefan\n"},{"id":"486178","messageId":"0a98597e270276c67a7aafae20c6d073@manjaro.org","threadId":"60674","inReplyTo":"c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de","subject":"Re: Concurrent fetch commands","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2023-12-31T13:48:45Z","receivedAt":"2023-12-31T13:48:47Z","isPatch":false,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2023-12-31 14:30, Stefan Haller wrote:\n> Currently, git doesn't seem to be very good at handling two concurrent\n> invocations of git fetch (or git fetch and git pull). This is a problem\n> because it is common for git clients to run git fetch periodically in\n> the background. In that case, when you happen to invoke git pull while\n> such a background fetch is running, an error occurs (\"Cannot rebase \n> onto\n> multiple branches\").\n> \n> I can reliably reproduce this by doing\n> \n> $ git fetch&; sleep 0.1; git pull\n> [1] 42160\n> [1] + done git fetch\n> fatal: Cannot rebase onto multiple branches.\n> \n> The reason for this failure seems to be that both the first fetch and\n> the fetch that runs as part of the pull append their information to\n> .git/FETCH_HEAD, so that the information for the current branch ends up\n> twice in the file.\n> \n> Do you think git fetch should be made more robust against scenarios \n> like\n> this?\n\nI believe a similar issue has been already raised recently, so perhaps \nintroducing some kind of file-based locking within git itself could be \njustified. It would make the things a bit more robust, and would also \nimprove the overall user experience.\n\n> More context: the git client that I'm contributing to (lazygit) used to\n> guard against this for its own background fetch with a global mutex \n> that\n> allowed only one single fetch, pull, or push at a time. This solved the\n> problem nicely for lazygit's own operations (at the expense of some \n> lag,\n> occasionally); and I'm not aware of any reports about failures because\n> some other git client's background fetch got in the way, so maybe we\n> don't have to worry about that too much.\n> \n> However, we now removed that mutex to allow certain parallel fetch\n> operations to run at the same time, most notably fetching (and \n> updating)\n> a branch that is not checked out (by doing \"git fetch origin\n> branch:branch\"). It is useful to be able to trigger this for multiple\n> branches concurrently, and actually this works fine.\n> \n> But now we have the problem described above, where a pull of the\n> checked-out branch runs at the same time as a background fetch; this is\n> not so unlikely, because lazygit triggers the first background fetch at\n> startup, so invoking the pull command right after starting lazygit is\n> very likely to fail.\n> \n> We could re-introduce a mutex and just make it a little less global;\n> e.g. protect only pull and parameter-less fetch. But fixing it in git\n> itself seems preferable to me.\n> \n> Sorry for the wall of text, but I figured giving more context could be\n> useful.\n"},{"id":"486179","messageId":"20231231165042.1d934927@RedEyes","threadId":"60674","inReplyTo":"c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de","subject":"Re: Concurrent fetch commands","fromName":"Konstantin Tokarev","fromEmail":"annulen@yandex.ru","sentAt":"2023-12-31T13:50:42Z","receivedAt":"2023-12-31T13:56:24Z","isPatch":false,"sender":{"key":"annulen@yandex.ru","avatar":null},"body":"В Sun, 31 Dec 2023 14:30:05 +0100\nStefan Haller пишет:\n\n> Currently, git doesn't seem to be very good at handling two concurrent\n> invocations of git fetch (or git fetch and git pull). This is a\n> problem because it is common for git clients to run git fetch\n> periodically in the background.\n\nI think it's a really weird idea which makes a disservice both to human\ngit users and git servers. IMO clients doing this should be fixed to\navoid such behavior, at least by default.\n"},{"id":"486180","messageId":"867b0189119ebb9eb5ac64426aef558b@manjaro.org","threadId":"60674","inReplyTo":"20231231165042.1d934927@RedEyes","subject":"Re: Concurrent fetch commands","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2023-12-31T14:01:39Z","receivedAt":"2023-12-31T14:01:41Z","isPatch":false,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2023-12-31 14:50, Konstantin Tokarev wrote:\n> В Sun, 31 Dec 2023 14:30:05 +0100\n> Stefan Haller пишет:\n> \n>> Currently, git doesn't seem to be very good at handling two concurrent\n>> invocations of git fetch (or git fetch and git pull). This is a\n>> problem because it is common for git clients to run git fetch\n>> periodically in the background.\n> \n> I think it's a really weird idea which makes a disservice both to human\n> git users and git servers. IMO clients doing this should be fixed to\n> avoid such behavior, at least by default.\n\nCould you, please, elaborate a bit on the resulting disservice? \nRegarding fixing the clients, IDE users often expect such automatic \nupdating to be performed in the background, so I'm afraid there's not \nmuch wiggle room there.\n"},{"id":"486181","messageId":"xmqqy1daffk8.fsf@gitster.g","threadId":"60674","inReplyTo":"c11ca0b3-aaf4-4a8d-80a1-3832954aa7aa@haller-berlin.de","subject":"Re: Concurrent fetch commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-12-31T17:27:19Z","receivedAt":"2023-12-31T17:27:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Haller writes:\n\n> I can reliably reproduce this by doing\n>\n> $ git fetch&; sleep 0.1; git pull\n> [1] 42160\n> [1] + done git fetch\n> fatal: Cannot rebase onto multiple branches.\n\nI see a bug here.\n\nHow this _ought_ to work is\n\n - The first \"git fetch\" wants to report what it fetched by writing\n into the $GIT_DIR/FETCH_HEAD file (\"git merge FETCH_HEAD\" after\n the fetch finishes can consume its contents).\n\n - The second \"git pull\" runs \"git fetch\" under the hood. Because\n it also wants to write to $GIT_DIR/FETCH_HEAD, and because there\n is already somebody writing to the file, it should notice and\n barf, saying \"fatal: a 'git fetch' is already working\" or\n something.\n\nBut because there is no \"Do not overwrite FETCH_HEAD somebody else\nis using\" protection, \"git merge\" or \"git rebase\" that is run as the\nsecond half of the \"git pull\" ends up working on the contents of\nFETCH_HEAD that is undefined, and GIGO result follows.\n\nThe \"bug\" that the second \"git fetch\" does not notice an already\nrunning one (who is in possession of FETCH_HEAD) and refrain from\nstarting is not easy to design a fix for---we cannot just abort by\nopening it with O_CREAT|O_EXCL because it is a normal thing for\n$GIT_DIR/FETCH_HEAD to exist after the \"last\" fetch. We truncate\nits contents before starting to avoid getting affected by contents\nleftover by the last fetch, but when there is a \"git fetch\" that is\nactively running, and it finishes _after_ the second one starts and\ntruncates the file, the second one will end up seeing the contents\nthe first one left. We have the \"--no-write-fetch-head\" option for\nusers to explicitly tell which invocation of \"git fetch\" should not\nwrite FETCH_HEAD.\n\nRunning \"background/priming\" fetches (the one before \"sleep 0.1\" you\nhave) is not a crime by itself, but it is a crime to run them\nwithout the \"--no-fetch-head\" option. Since you have *NO* intention\nof using its contents to feed a \"git merge\" (or equivalent)\nyourself, you are breaking your \"git pull\" step in your example\nreproduction yourself.\n\n"},{"id":"486182","messageId":"f7d4f092ea8955113c2d6c0346932b70@manjaro.org","threadId":"60674","inReplyTo":"xmqqy1daffk8.fsf@gitster.g","subject":"Re: Concurrent fetch commands","fromName":"Dragan Simic","fromEmail":"dsimic@manjaro.org","sentAt":"2023-12-31T17:41:15Z","receivedAt":"2023-12-31T17:41:18Z","isPatch":false,"sender":{"key":"dsimic@manjaro.org","avatar":null},"body":"On 2023-12-31 18:27, Junio C Hamano wrote:\n> Stefan Haller writes:\n> \n>> I can reliably reproduce this by doing\n>> \n>> $ git fetch&; sleep 0.1; git pull\n>> [1] 42160\n>> [1] + done git fetch\n>> fatal: Cannot rebase onto multiple branches.\n> \n> I see a bug here.\n> \n> How this _ought_ to work is\n> \n> - The first \"git fetch\" wants to report what it fetched by writing\n> into the $GIT_DIR/FETCH_HEAD file (\"git merge FETCH_HEAD\" after\n> the fetch finishes can consume its contents).\n> \n> - The second \"git pull\" runs \"git fetch\" under the hood. Because\n> it also wants to write to $GIT_DIR/FETCH_HEAD, and because there\n> is already somebody writing to the file, it should notice and\n> barf, saying \"fatal: a 'git fetch' is already working\" or\n> something.\n> \n> But because there is no \"Do not overwrite FETCH_HEAD somebody else\n> is using\" protection, \"git merge\" or \"git rebase\" that is run as the\n> second half of the \"git pull\" ends up working on the contents of\n> FETCH_HEAD that is undefined, and GIGO result follows.\n> \n> The \"bug\" that the second \"git fetch\" does not notice an already\n> running one (who is in possession of FETCH_HEAD) and refrain from\n> starting is not easy to design a fix for---we cannot just abort by\n> opening it with O_CREAT|O_EXCL because it is a normal thing for\n> $GIT_DIR/FETCH_HEAD to exist after the \"last\" fetch. We truncate\n> its contents before starting to avoid getting affected by contents\n> leftover by the last fetch, but when there is a \"git fetch\" that is\n> actively running, and it finishes _after_ the second one starts and\n> truncates the file, the second one will end up seeing the contents\n> the first one left. We have the \"--no-write-fetch-head\" option for\n> users to explicitly tell which invocation of \"git fetch\" should not\n> write FETCH_HEAD.\n> \n> Running \"background/priming\" fetches (the one before \"sleep 0.1\" you\n> have) is not a crime by itself, but it is a crime to run them\n> without the \"--no-fetch-head\" option. Since you have *NO* intention\n> of using its contents to feed a \"git merge\" (or equivalent)\n> yourself, you are breaking your \"git pull\" step in your example\n> reproduction yourself.\n\nThank you very much for this highly detailed explanation.\n"},{"id":"486185","messageId":"55620f4f-80f9-4e9a-8947-dce5d2a5113d@haller-berlin.de","threadId":"60674","inReplyTo":"20231231165042.1d934927@RedEyes","subject":"Re: Concurrent fetch commands","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-01-01T11:23:39Z","receivedAt":"2024-01-01T11:23:42Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 31.12.23 14:50, Konstantin Tokarev wrote:\n> В Sun, 31 Dec 2023 14:30:05 +0100\n> Stefan Haller пишет:\n> \n>> ... it is common for git clients to run git fetch\n>> periodically in the background.\n> \n> I think it's a really weird idea which makes a disservice both to human\n> git users and git servers. IMO clients doing this should be fixed to\n> avoid such behavior, at least by default.\n\nI disagree pretty strongly. Users expect this, and I do find it very\nconvenient myself.\n"},{"id":"486186","messageId":"9b5ff583-f8b9-42dd-8829-2fea74bc2057@haller-berlin.de","threadId":"60674","inReplyTo":"xmqqy1daffk8.fsf@gitster.g","subject":"Re: Concurrent fetch commands","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-01-01T11:30:40Z","receivedAt":"2024-01-01T11:30:42Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 31.12.23 18:27, Junio C Hamano wrote:\n> How this _ought_ to work is\n> \n> ... it should notice and\n> barf, saying \"fatal: a 'git fetch' is already working\" or\n> something.\n\nInteresting, I had expected this to work somehow, e.g. by sequencing the\noperations or whatever is necessary to make it work. Fixing the bug like\nyou suggest actually makes very little difference in practice, it just\ngives a slightly less confusing error message.\n\n> but it is a crime to run them without the \"--no-fetch-head\" option.\n\nOuch, I wasn't aware we are committing crimes. We'll accept the\npunishment. :-)\n\nBut it does sound like --no-write-fetch-head will solve our problem,\nthanks!\n\n-Stefan\n"},{"id":"486187","messageId":"08e4527f-3e1d-488c-8857-a40b65a71a3c@haller-berlin.de","threadId":"60674","inReplyTo":"9b5ff583-f8b9-42dd-8829-2fea74bc2057@haller-berlin.de","subject":"Re: Concurrent fetch commands","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-01-01T11:42:35Z","receivedAt":"2024-01-01T11:42:37Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 01.01.24 12:30, Stefan Haller wrote:\n> But it does sound like --no-write-fetch-head will solve our problem,\n> thanks!\n\nActually we saw another problem recently that I suspect might also be\ncaused by the concurrency between fetch and pull, but I'm not sure. I'll\nexplain it here in case anybody has any idea.\n\nWhat happened was that a user tried to pull a branch that was behind its\nupstream (not diverged). They got the error message from\nshow_advice_pull_non_ff (\"Need to specify how to reconcile divergent\nbranches\"); the log then showed that the background fetch was ongoing\nfor the remote of the current branch while they initiated the pull.\n\nTrying to pull again after the background fetch had moved on to the next\nremote then worked.\n\nI read the code in pull.c a bit, but I can't see how it can become so\nconfused about being diverged in this scenario. Any ideas?\n\n-Stefan\n"},{"id":"486189","messageId":"4525f59f-d4d1-405a-89d0-fa51f1f8f877@kircheis.it","threadId":"60674","inReplyTo":"55620f4f-80f9-4e9a-8947-dce5d2a5113d@haller-berlin.de","subject":"Re: Concurrent fetch commands","fromName":"Federico Kircheis","fromEmail":"federico@kircheis.it","sentAt":"2024-01-01T15:47:55Z","receivedAt":"2024-01-01T15:48:05Z","isPatch":false,"sender":{"key":"federico@kircheis.it","avatar":null},"body":"On 01/01/2024 12.23, Stefan Haller wrote:\n> On 31.12.23 14:50, Konstantin Tokarev wrote:\n>> В Sun, 31 Dec 2023 14:30:05 +0100\n>> Stefan Haller пишет:\n>>\n>>> ... it is common for git clients to run git fetch\n>>> periodically in the background.\n>>\n>> I think it's a really weird idea which makes a disservice both to human\n>> git users and git servers. IMO clients doing this should be fixed to\n>> avoid such behavior, at least by default.\n> \n> I disagree pretty strongly. Users expect this, and I do find it very\n> convenient myself.\n> \n\nEspecially when working with git worktree.\n"},{"id":"486241","messageId":"ZZUNxNciNb_xZveY@tanuki","threadId":"60674","inReplyTo":"xmqqy1daffk8.fsf@gitster.g","subject":"Re: Concurrent fetch commands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-03T07:33:24Z","receivedAt":"2024-01-03T07:33:30Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sun, Dec 31, 2023 at 09:27:19AM -0800, Junio C Hamano wrote:\n> Stefan Haller writes:\n> \n> > I can reliably reproduce this by doing\n> >\n> > $ git fetch&; sleep 0.1; git pull\n> > [1] 42160\n> > [1] + done git fetch\n> > fatal: Cannot rebase onto multiple branches.\n> \n> I see a bug here.\n> \n> How this _ought_ to work is\n> \n> - The first \"git fetch\" wants to report what it fetched by writing\n> into the $GIT_DIR/FETCH_HEAD file (\"git merge FETCH_HEAD\" after\n> the fetch finishes can consume its contents).\n> \n> - The second \"git pull\" runs \"git fetch\" under the hood. Because\n> it also wants to write to $GIT_DIR/FETCH_HEAD, and because there\n> is already somebody writing to the file, it should notice and\n> barf, saying \"fatal: a 'git fetch' is already working\" or\n> something.\n> \n> But because there is no \"Do not overwrite FETCH_HEAD somebody else\n> is using\" protection, \"git merge\" or \"git rebase\" that is run as the\n> second half of the \"git pull\" ends up working on the contents of\n> FETCH_HEAD that is undefined, and GIGO result follows.\n> \n> The \"bug\" that the second \"git fetch\" does not notice an already\n> running one (who is in possession of FETCH_HEAD) and refrain from\n> starting is not easy to design a fix for---we cannot just abort by\n> opening it with O_CREAT|O_EXCL because it is a normal thing for\n> $GIT_DIR/FETCH_HEAD to exist after the \"last\" fetch. We truncate\n> its contents before starting to avoid getting affected by contents\n> leftover by the last fetch, but when there is a \"git fetch\" that is\n> actively running, and it finishes _after_ the second one starts and\n> truncates the file, the second one will end up seeing the contents\n> the first one left. We have the \"--no-write-fetch-head\" option for\n> users to explicitly tell which invocation of \"git fetch\" should not\n> write FETCH_HEAD.\n\nWhile I agree that it's the right thing to use \"--no-write-fetch-head\"\nin this context, I still wonder whether we want to fix this \"bug\". It\nwould be a rather easy change on our side to start using the lockfile\nAPI to write to FETCH_HEAD, which has a bunch of benefits:\n\n - We would block concurrent processes of writing to FETCH_HEAD at the\n same time (well, at least for clients aware of the new semantics).\n\n - Consequentially, we do not write a corrupted FETCH_HEAD anymore when\n multiple processes write to it at the same time.\n\n - We're also more robust against corruption in the context of hard\n crashes due to atomic rename semantics and proper flushing.\n\nI don't really see much of a downside except for the fact that we change\nhow this special ref is being written to, so other implementations would\nneed to adapt accordingly. But even if they didn't, if clients with both\nthe new and old behaviour write FETCH_HEAD at the same point in time the\nresult would still be a consistent FETCH_HEAD if both writes finish. We\ndo have a race now which of both versions of FETCH_HEAD we see, but that\nfeels better than corrupted contents to me.\n\nPatrick\n"},{"id":"486243","messageId":"ZZUWmy3rTjpBsH-w@tanuki","threadId":"60674","inReplyTo":"ZZUNxNciNb_xZveY@tanuki","subject":"Re: Concurrent fetch commands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-03T08:11:07Z","receivedAt":"2024-01-03T08:11:12Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 03, 2024 at 08:33:24AM +0100, Patrick Steinhardt wrote:\n> On Sun, Dec 31, 2023 at 09:27:19AM -0800, Junio C Hamano wrote:\n> > Stefan Haller writes:\n> > \n> > > I can reliably reproduce this by doing\n> > >\n> > > $ git fetch&; sleep 0.1; git pull\n> > > [1] 42160\n> > > [1] + done git fetch\n> > > fatal: Cannot rebase onto multiple branches.\n> > \n> > I see a bug here.\n> > \n> > How this _ought_ to work is\n> > \n> > - The first \"git fetch\" wants to report what it fetched by writing\n> > into the $GIT_DIR/FETCH_HEAD file (\"git merge FETCH_HEAD\" after\n> > the fetch finishes can consume its contents).\n> > \n> > - The second \"git pull\" runs \"git fetch\" under the hood. Because\n> > it also wants to write to $GIT_DIR/FETCH_HEAD, and because there\n> > is already somebody writing to the file, it should notice and\n> > barf, saying \"fatal: a 'git fetch' is already working\" or\n> > something.\n> > \n> > But because there is no \"Do not overwrite FETCH_HEAD somebody else\n> > is using\" protection, \"git merge\" or \"git rebase\" that is run as the\n> > second half of the \"git pull\" ends up working on the contents of\n> > FETCH_HEAD that is undefined, and GIGO result follows.\n> > \n> > The \"bug\" that the second \"git fetch\" does not notice an already\n> > running one (who is in possession of FETCH_HEAD) and refrain from\n> > starting is not easy to design a fix for---we cannot just abort by\n> > opening it with O_CREAT|O_EXCL because it is a normal thing for\n> > $GIT_DIR/FETCH_HEAD to exist after the \"last\" fetch. We truncate\n> > its contents before starting to avoid getting affected by contents\n> > leftover by the last fetch, but when there is a \"git fetch\" that is\n> > actively running, and it finishes _after_ the second one starts and\n> > truncates the file, the second one will end up seeing the contents\n> > the first one left. We have the \"--no-write-fetch-head\" option for\n> > users to explicitly tell which invocation of \"git fetch\" should not\n> > write FETCH_HEAD.\n> \n> While I agree that it's the right thing to use \"--no-write-fetch-head\"\n> in this context, I still wonder whether we want to fix this \"bug\". It\n> would be a rather easy change on our side to start using the lockfile\n> API to write to FETCH_HEAD, which has a bunch of benefits:\n> \n> - We would block concurrent processes of writing to FETCH_HEAD at the\n> same time (well, at least for clients aware of the new semantics).\n> \n> - Consequentially, we do not write a corrupted FETCH_HEAD anymore when\n> multiple processes write to it at the same time.\n> \n> - We're also more robust against corruption in the context of hard\n> crashes due to atomic rename semantics and proper flushing.\n> \n> I don't really see much of a downside except for the fact that we change\n> how this special ref is being written to, so other implementations would\n> need to adapt accordingly. But even if they didn't, if clients with both\n> the new and old behaviour write FETCH_HEAD at the same point in time the\n> result would still be a consistent FETCH_HEAD if both writes finish. We\n> do have a race now which of both versions of FETCH_HEAD we see, but that\n> feels better than corrupted contents to me.\n\nAh, one thing I didn't think of is parallel fetches. It's expected that\nall of the fetches write into FETCH_HEAD at the same point in time\nconcurrently, so the end result is the collection of updated refs even\nthough the order isn't guaranteed. That makes things a bit more\ncomplicated indeed.\n\nPatrick\n"},{"id":"486252","messageId":"ZZU5s4LKQF1NLgnC@tanuki","threadId":"60674","inReplyTo":"ZZU1TCyQdLqoLxPw@ugly","subject":"Re: Concurrent fetch commands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-01-03T10:40:51Z","receivedAt":"2024-01-03T10:40:57Z","isPatch":false,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Jan 03, 2024 at 11:22:04AM +0100, Oswald Buddenhagen wrote:\n> On Wed, Jan 03, 2024 at 09:11:07AM +0100, Patrick Steinhardt wrote:\n> > Ah, one thing I didn't think of is parallel fetches. It's expected that\n> > all of the fetches write into FETCH_HEAD at the same point in time\n> > concurrently\n> > \n> is it, though? given that the contents could be already randomly scrambled,\n> it would not seem particularly bad if the behavior changed.\n> \n> the one real complication i see is the --append option, which requires using\n> a waiting lock after the actual fetch, rather than acquiring it immediately\n> and erroring out on failure (and ideally giving a hint to use\n> --no-write-fetch-head).\n\nI should probably clarify, but with \"parallel fetches\" I meant `git\nfetch --jobs=`, not two separate executions of git-fetch(1). And these\ndo in fact use `--append` internally: the main process first truncates\nFETCH_HEAD and then spawns its children, which will then append to\nFETCH_HEAD in indeterministic order.\n\nBut even though the order is indeterministic, I wouldn't go as far as\nclaiming that the complete feature is broken. It works and records all\nupdated refs in FETCH_HEAD just fine, even if it's not particularly\nelegant. Which to me shows that we should try hard not to break it.\n\n> an extra complication is that concurrent runs with and without --append\n> should be precluded, because that would again result in undefined behavior.\n> it generally seems tricky to get --append straight if parallel fetches are\n> supposed to work.\n\nYeah, the `--append` flag indeed complicates things. There are two ways\nto handle this:\n\n - `--append` should refrain from running when there is a lockfile.\n This breaks `git fetch --jobs` without extra infra to handle this\n case, and furthermore a user may (rightfully?) expect that two\n manually spawned `git fetch --append` processes should work just\n fine.\n\n - `--append` should handle concurrency just fine, that is it knows to\n append to a preexisting lockfile. This is messy though, and the\n original creator of the lockfile wouldn't know when it can commit it\n into place.\n\nBoth options are kind of ugly, so I'm less sure now whether lockfiles\nare the way to go.\n\nPatrick\n"},{"id":"486261","messageId":"ZZWOBObBmLW9Nid6@nand.local","threadId":"60674","inReplyTo":"ZZU5s4LKQF1NLgnC@tanuki","subject":"Re: Concurrent fetch commands","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-01-03T16:40:36Z","receivedAt":"2024-01-03T16:40:39Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 03, 2024 at 11:40:51AM +0100, Patrick Steinhardt wrote:\n> - `--append` should handle concurrency just fine, that is it knows to\n> append to a preexisting lockfile. This is messy though, and the\n> original creator of the lockfile wouldn't know when it can commit it\n> into place.\n>\n> Both options are kind of ugly, so I'm less sure now whether lockfiles\n> are the way to go.\n\nInteresting. Thinking a little bit about what you wrote here, I feel\nlike `--append[=] would do what you need here. The creator\nof the lockfile would commit it into place exactly when all children\nhave finished writing into the existing lockfile.\n\nIt seems like that could work, but I haven't poked around to figure out\nwhether or not that is the case. Regardless, supposing that it does\nwork, I wonder what users reasonably expect in the presence of multiple\n'git fetch' operations. I suppose the answer is that they expect\nconcurrent fetches to be tolerated, but that the contents of FETCH_HEAD\n(and of course the remote references) are consistent at the end of all\nof the fetches.\n\nThanks,\nTaylor\n"},{"id":"486277","messageId":"xmqqwmsq83v3.fsf@gitster.g","threadId":"60674","inReplyTo":"ZZWOBObBmLW9Nid6@nand.local","subject":"Re: Concurrent fetch commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-03T22:10:56Z","receivedAt":"2024-01-03T22:10:59Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> ... I suppose the answer is that they expect\n> concurrent fetches to be tolerated, but that the contents of FETCH_HEAD\n> (and of course the remote references) are consistent at the end of all\n> of the fetches.\n\nWhat does it mean to be \"consistent\" in this case, though? For the\ncontrolled form of multiple fetches performed by \"git fetch --all\",\nthe answer is probably \"as if we fetched sequentially from these\nremotes, one by one, and concatenated what these individual fetch\ninvocations left in FETCH_HEAD\". But for an uncontrolled background\nfetch IDE and others perform behind user's back, it is unclear what\nit means, or for that matter, it is dubious if there is a reasonable\ndefinition for the word.\n\nFolks who invented \"git maintenance\" designed their \"prefetch\" task\nto perform the best practice, without interfering any foreground\nfetches by not touching FETCH_HEAD and the remote-tracking branches.\n\nNobody brought up the latter so far on this discussion thread, but\nmucking with the remote-tracking branches behind user's back means\ncompletely breaking the end-user expectation that --force-with-lease\nwould do something useful even when it is not given the commit the\nuser expects to see at the remote. Perhaps those third-party tools\nthat want to run \"git fetch\" in the background can learn from how\n\"prefetch\" task works to avoid the breakage they are inflicting on\ntheir users?\n\n\n\n"},{"id":"486290","messageId":"80efcb43-122c-421a-b763-6da6ff620538@haller-berlin.de","threadId":"60674","inReplyTo":"xmqqwmsq83v3.fsf@gitster.g","subject":"Re: Concurrent fetch commands","fromName":"Stefan Haller","fromEmail":"lists@haller-berlin.de","sentAt":"2024-01-04T12:01:26Z","receivedAt":"2024-01-04T12:01:34Z","isPatch":false,"sender":{"key":"lists@haller-berlin.de","avatar":null},"body":"On 03.01.24 23:10, Junio C Hamano wrote:\n> Folks who invented \"git maintenance\" designed their \"prefetch\" task\n> to perform the best practice, without interfering any foreground\n> fetches by not touching FETCH_HEAD and the remote-tracking branches.\n\nThat's good, but it's for a very different purpose than an IDE's\nbackground fetch. git maintenance's prefetch is just to improve\nperformance for the next pull; the point of an IDE's background fetch is\nto show me which of my remote branches have new stuff that I might be\ninterested in pulling, without having to fetch myself. So I *want* this\nto be mucking with my remote-tracking branches.\n\n> Nobody brought up the latter so far on this discussion thread, but\n> mucking with the remote-tracking branches behind user's back means\n> completely breaking the end-user expectation that --force-with-lease\n> would do something useful even when it is not given the commit the\n> user expects to see at the remote. \n\nThat's an interesting point that indeed hasn't been brought up yet.\nHowever, don't we all agree that --force-with-lease without specifying a\ncommit is not a good idea anyway, in general? That's why\n--force-if-includes was invented, isn't it?\n\n> Perhaps those third-party tools\n> that want to run \"git fetch\" in the background can learn from how\n> \"prefetch\" task works to avoid the breakage they are inflicting on\n> their users?\n\nAgain, what you call \"breakage\" here is the very point of a background\nfetch for me, so I don't want it to be avoided. (For FETCH_HEAD, yes,\nand I learned that we have --no-write-fetch-head for that, but for\nremote-tracking branches, no.)\n"},{"id":"486296","messageId":"ZZbsL8/2KfSITJv3@nand.local","threadId":"60674","inReplyTo":"xmqqwmsq83v3.fsf@gitster.g","subject":"Re: Concurrent fetch commands","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-01-04T17:34:39Z","receivedAt":"2024-01-04T17:34:41Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Jan 03, 2024 at 02:10:56PM -0800, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > ... I suppose the answer is that they expect\n> > concurrent fetches to be tolerated, but that the contents of FETCH_HEAD\n> > (and of course the remote references) are consistent at the end of all\n> > of the fetches.\n>\n> What does it mean to be \"consistent\" in this case, though? For the\n> controlled form of multiple fetches performed by \"git fetch --all\",\n> the answer is probably \"as if we fetched sequentially from these\n> remotes, one by one, and concatenated what these individual fetch\n> invocations left in FETCH_HEAD\". But for an uncontrolled background\n> fetch IDE and others perform behind user's back, it is unclear what\n> it means, or for that matter, it is dubious if there is a reasonable\n> definition for the word.\n\nYeah, on thinking on it more I tend to agree here.\n\n> Nobody brought up the latter so far on this discussion thread, but\n> mucking with the remote-tracking branches behind user's back means\n> completely breaking the end-user expectation that --force-with-lease\n> would do something useful even when it is not given the commit the\n> user expects to see at the remote. Perhaps those third-party tools\n> that want to run \"git fetch\" in the background can learn from how\n> \"prefetch\" task works to avoid the breakage they are inflicting on\n> their users?\n\nProbably so.\n\nThanks,\nTaylor\n"},{"id":"486318","messageId":"20240104205408.h5wvcissfbat7acw@glandium.org","threadId":"60674","inReplyTo":"80efcb43-122c-421a-b763-6da6ff620538@haller-berlin.de","subject":"Re: Concurrent fetch commands","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2024-01-04T20:54:08Z","receivedAt":"2024-01-04T21:28:54Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Jan 04, 2024 at 01:01:26PM +0100, Stefan Haller wrote:\n> On 03.01.24 23:10, Junio C Hamano wrote:\n> > Folks who invented \"git maintenance\" designed their \"prefetch\" task\n> > to perform the best practice, without interfering any foreground\n> > fetches by not touching FETCH_HEAD and the remote-tracking branches.\n> \n> That's good, but it's for a very different purpose than an IDE's\n> background fetch. git maintenance's prefetch is just to improve\n> performance for the next pull; the point of an IDE's background fetch is\n> to show me which of my remote branches have new stuff that I might be\n> interested in pulling, without having to fetch myself. So I *want* this\n> to be mucking with my remote-tracking branches.\n\nUse `git remote update`?\n\nMike\n"},{"id":"486319","messageId":"xmqq34vc7nl1.fsf@gitster.g","threadId":"60674","inReplyTo":"20240104205408.h5wvcissfbat7acw@glandium.org","subject":"Re: Concurrent fetch commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-01-04T22:14:50Z","receivedAt":"2024-01-04T22:14:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mike Hommey writes:\n\n> On Thu, Jan 04, 2024 at 01:01:26PM +0100, Stefan Haller wrote:\n>> On 03.01.24 23:10, Junio C Hamano wrote:\n>> > Folks who invented \"git maintenance\" designed their \"prefetch\" task\n>> > to perform the best practice, without interfering any foreground\n>> > fetches by not touching FETCH_HEAD and the remote-tracking branches.\n>> \n>> That's good, but it's for a very different purpose than an IDE's\n>> background fetch. git maintenance's prefetch is just to improve\n>> performance for the next pull; the point of an IDE's background fetch is\n>> to show me which of my remote branches have new stuff that I might be\n>> interested in pulling, without having to fetch myself. So I *want* this\n>> to be mucking with my remote-tracking branches.\n>\n> Use `git remote update`?\n\nHmph, it seems that it does not pass \"--no-write-fetch-head\" so it\nwould interfere with the foreground \"fetch\" or \"pull\" the end user\nconsciously makes, exactly the same way Stefan's demonstration in\nthe first message in the thread, no?\n"},{"id":"486324","messageId":"20240104222521.nb3fxipg7ayolxvo@glandium.org","threadId":"60674","inReplyTo":"xmqq34vc7nl1.fsf@gitster.g","subject":"Re: Concurrent fetch commands","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2024-01-04T22:25:21Z","receivedAt":"2024-01-04T22:25:43Z","isPatch":false,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Jan 04, 2024 at 02:14:50PM -0800, Junio C Hamano wrote:\n> Mike Hommey writes:\n> \n> > On Thu, Jan 04, 2024 at 01:01:26PM +0100, Stefan Haller wrote:\n> >> On 03.01.24 23:10, Junio C Hamano wrote:\n> >> > Folks who invented \"git maintenance\" designed their \"prefetch\" task\n> >> > to perform the best practice, without interfering any foreground\n> >> > fetches by not touching FETCH_HEAD and the remote-tracking branches.\n> >> \n> >> That's good, but it's for a very different purpose than an IDE's\n> >> background fetch. git maintenance's prefetch is just to improve\n> >> performance for the next pull; the point of an IDE's background fetch is\n> >> to show me which of my remote branches have new stuff that I might be\n> >> interested in pulling, without having to fetch myself. So I *want* this\n> >> to be mucking with my remote-tracking branches.\n> >\n> > Use `git remote update`?\n> \n> Hmph, it seems that it does not pass \"--no-write-fetch-head\" so it\n> would interfere with the foreground \"fetch\" or \"pull\" the end user\n> consciously makes, exactly the same way Stefan's demonstration in\n> the first message in the thread, no?\n\nInteresting, I never realized it updated FETCH_HEAD, but it does. All\nthe entries are marked \"not-for-merge\", though, whatever that implies.\n\nMike\n"}]}