Parallel fetch and commit graph writing results in locking failure (even on linux)

Parallel fetch and commit graph writing results in locking failure (even on linux)

[Thomas Braun]

Hi,

on git version da72936f (Git 2.24, 2019-11-04) and debian stretch I currently get every now and then the following error during fetching

$git fetch --all --jobs 12
Fordere an von origin
Fordere an von XXXX
Fordere an von YYYY
Fordere an von ZZZZ
Fordere an von EEEE
Von github.com:tango-controls/cppTango
   37cc52f8..4550a743  tango-9-lts -> origin/tango-9-lts
Commit-Graph Generierungsnummern berechnen: 100% (14/14), Fertig.
Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
fatal: Konnte '/home/firma/devel/cppTango/.git/objects/info/commit-graphs/commit-graph-chain.lock' nicht erstellen: Die Datei existiert bereits.

Ein anderer Git-Prozess scheint in diesem Repository ausgeführt
zu werden, zum Beispiel ein noch offener Editor von 'git commit'.
Bitte stellen Sie sicher, dass alle Prozesse beendet wurden und
versuchen Sie es erneut. Falls es immer noch fehlschlägt, könnte
ein früherer Git-Prozess in diesem Repository abgestürzt sein:
Löschen Sie die Datei manuell um fortzufahren.
Konnte 'myFork' nicht anfordern (Exit-Code: 128)
Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.

(Sorry for the german text, this is not easily reproducible.)
It complains that it could not create the lock file as it already exists.

I've set the following possible relevant settings:

[core]
  commitGraph = true

[fetch]
  prune = true
  writeCommitGraph = true

[protocol]
  version = 2

Anything obvious I'm doing wrong?

Thanks,
Thomas

PS: The error is also present on latest git for windows.

Re: Parallel fetch and commit graph writing results in locking failure (even on linux)

[Derrick Stolee]

On 12/13/2019 2:20 PM, Thomas Braun wrote:
> Hi,
> 
> on git version da72936f (Git 2.24, 2019-11-04) and debian stretch I currently get every now and then the following error during fetching
> 
> $git fetch --all --jobs 12
> Fordere an von origin
> Fordere an von XXXX
> Fordere an von YYYY
> Fordere an von ZZZZ
> Fordere an von EEEE
> Von github.com:tango-controls/cppTango
>    37cc52f8..4550a743  tango-9-lts -> origin/tango-9-lts
> Commit-Graph Generierungsnummern berechnen: 100% (14/14), Fertig.
> Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
> fatal: Konnte '/home/firma/devel/cppTango/.git/objects/info/commit-graphs/commit-graph-chain.lock' nicht erstellen: Die Datei existiert bereits.
> 
> Ein anderer Git-Prozess scheint in diesem Repository ausgeführt
> zu werden, zum Beispiel ein noch offener Editor von 'git commit'.
> Bitte stellen Sie sicher, dass alle Prozesse beendet wurden und
> versuchen Sie es erneut. Falls es immer noch fehlschlägt, könnte
> ein früherer Git-Prozess in diesem Repository abgestürzt sein:
> Löschen Sie die Datei manuell um fortzufahren.
> Konnte 'myFork' nicht anfordern (Exit-Code: 128)
> Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
> Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
> Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
> 
> (Sorry for the german text, this is not easily reproducible.)
> It complains that it could not create the lock file as it already exists.
> 
> I've set the following possible relevant settings:
> 
> [core]
>   commitGraph = true
> 
> [fetch]
>   prune = true
>   writeCommitGraph = true
> 
> [protocol]
>   version = 2
> 
> Anything obvious I'm doing wrong?

I don't think so. I think you just found a bug where the
fetch.writeCommitGraph logic doesn't work with parallel fetch
jobs (only one can write at a time).

I believe the fix would be to write the commit-graph after
all of the jobs have completed, which should mean we need to
move the call to write_commit_graph_reachable() somewhere else
inside builtin/fetch.c.

I'll take a look now.

Thanks,
-Stolee

Re: Parallel fetch and commit graph writing results in locking failure (even on linux)

[Jeff King]

On Fri, Dec 13, 2019 at 02:35:47PM -0500, Derrick Stolee wrote:

> I don't think so. I think you just found a bug where the
> fetch.writeCommitGraph logic doesn't work with parallel fetch
> jobs (only one can write at a time).
> 
> I believe the fix would be to write the commit-graph after
> all of the jobs have completed, which should mean we need to
> move the call to write_commit_graph_reachable() somewhere else
> inside builtin/fetch.c.

This should be fixed in master by bcb06e204c (Merge branch
'js/fetch-multi-lockfix', 2019-12-01), I think.


-Peff

Re: Parallel fetch and commit graph writing results in locking failure (even on linux)

[Derrick Stolee]

On 12/13/2019 2:52 PM, Jeff King wrote:
> On Fri, Dec 13, 2019 at 02:35:47PM -0500, Derrick Stolee wrote:
> 
>> I don't think so. I think you just found a bug where the
>> fetch.writeCommitGraph logic doesn't work with parallel fetch
>> jobs (only one can write at a time).
>>
>> I believe the fix would be to write the commit-graph after
>> all of the jobs have completed, which should mean we need to
>> move the call to write_commit_graph_reachable() somewhere else
>> inside builtin/fetch.c.
> 
> This should be fixed in master by bcb06e204c (Merge branch
> 'js/fetch-multi-lockfix', 2019-12-01), I think.

Thanks, Peff. That exactly looks like the right fix. The
actual commit is 7d8e72b9 ("fetch: avoid locking issues between
fetch.jobs/fetch.writeCommitGraph" 2019-11-03). I had forgotten
that this was already fixed.

Here is the diff, for reference:

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8d27f8abb7..20bcda09c4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1602,7 +1602,8 @@ static int fetch_multiple(struct string_list *list, int max_children)
                        return errcode;
        }
 
-       argv_array_pushl(&argv, "fetch", "--append", "--no-auto-gc", NULL);
+       argv_array_pushl(&argv, "fetch", "--append", "--no-auto-gc",
+                       "--no-write-commit-graph", NULL);
        add_options_to_argv(&argv);
 
        if (max_children != 1 && list->nr != 1) {

-Stolee

Re: Parallel fetch and commit graph writing results in locking failure (even on linux)

[SZEDER Gábor]

On Fri, Dec 13, 2019 at 08:20:42PM +0100, Thomas Braun wrote:
> Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
> 
> (Sorry for the german text, this is not easily reproducible.)

It's unrelated to your issue, and I'm not a native German or English
speaker... but that translation doesn't look quite right to me.

The original is:

  _("Computing commit graph generation numbers"),

where the word "generation" isn't used in the sense of "creation"
(erzeugen, generieren), but rather as in "generation gap" or "my
parents are one generation older than me".  So I don't think that
"Generierung..." is the right word to use here, but perhaps
"Generationsnummer".

But who am I to argue about German software ranslation?! :)

Re: Parallel fetch and commit graph writing results in locking failure (even on linux)

[Thomas Braun]

> Derrick Stolee <stolee@gmail.com> hat am 13. Dezember 2019 um 20:58 geschrieben:
> 
> 
> On 12/13/2019 2:52 PM, Jeff King wrote:
> > On Fri, Dec 13, 2019 at 02:35:47PM -0500, Derrick Stolee wrote:
> > 
> >> I don't think so. I think you just found a bug where the
> >> fetch.writeCommitGraph logic doesn't work with parallel fetch
> >> jobs (only one can write at a time).
> >>
> >> I believe the fix would be to write the commit-graph after
> >> all of the jobs have completed, which should mean we need to
> >> move the call to write_commit_graph_reachable() somewhere else
> >> inside builtin/fetch.c.
> > 
> > This should be fixed in master by bcb06e204c (Merge branch
> > 'js/fetch-multi-lockfix', 2019-12-01), I think.
> 
> Thanks, Peff. That exactly looks like the right fix. The
> actual commit is 7d8e72b9 ("fetch: avoid locking issues between
> fetch.jobs/fetch.writeCommitGraph" 2019-11-03). I had forgotten
> that this was already fixed.

Thanks Stolee and Peff, I'll update to a version with that fix. And in the unlikely case this does not resolve my issue, report back.
 
> Here is the diff, for reference:
> 
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 8d27f8abb7..20bcda09c4 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1602,7 +1602,8 @@ static int fetch_multiple(struct string_list *list, int max_children)
>                         return errcode;
>         }
>  
> -       argv_array_pushl(&argv, "fetch", "--append", "--no-auto-gc", NULL);
> +       argv_array_pushl(&argv, "fetch", "--append", "--no-auto-gc",
> +                       "--no-write-commit-graph", NULL);
>         add_options_to_argv(&argv);
>  
>         if (max_children != 1 && list->nr != 1) {
> 
> -Stolee
>

german language fix: Generierung vs. Generation [was: Re: Parallel fetch and commit graph writing results in locking failure (even on linux)]

[Thomas Braun]

> SZEDER Gábor <szeder.dev@gmail.com> hat am 13. Dezember 2019 um 21:15 geschrieben:
> 
> 
> On Fri, Dec 13, 2019 at 08:20:42PM +0100, Thomas Braun wrote:
> > Commit-Graph Generierungsnummern berechnen: 100% (13/13), Fertig.
> > 
> > (Sorry for the german text, this is not easily reproducible.)
> 
> It's unrelated to your issue, and I'm not a native German or English
> speaker... but that translation doesn't look quite right to me.
> 
> The original is:
> 
>   _("Computing commit graph generation numbers"),
> 
> where the word "generation" isn't used in the sense of "creation"
> (erzeugen, generieren), but rather as in "generation gap" or "my
> parents are one generation older than me".  So I don't think that
> "Generierung..." is the right word to use here, but perhaps
> "Generationsnummer".
> 
> But who am I to argue about German software ranslation?! :)

Your hunch is right I would say (not that it matters much but I'm a native speaker).

So how about the following patch (CC'ing the original translator)

commit 724bf68da28b75e04a9fda34b08e95259ade9ec3 (HEAD -> master)
Author: Thomas Braun <thomas.braun@virtuell-zuhause.de>
Date:   Sun Dec 15 18:28:42 2019 +0100

    lang de: Reword generation numbers
    
    The english term generation is here not used in the sense of "to
    generate" but in the sense of "generations of beings".
    
    This corrects the initial translation from cf4c0c25 (l10n: update German
    translation, 2018-12-06).
    
    Fixed-by: "SZEDER Gábor" <szeder.dev@gmail.com>

diff --git a/po/de.po b/po/de.po
index 066326a687..773e361f6f 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1535,7 +1535,7 @@ msgstr "Lösche Commit-Markierungen in Commit-Graph"
 
 #: commit-graph.c:1104
 msgid "Computing commit graph generation numbers"
-msgstr "Commit-Graph Generierungsnummern berechnen"
+msgstr "Commit-Graph Generationsnummern berechnen"
 
 #: commit-graph.c:1179
 #, c-format

[PATCH] l10n: de.po: Reword generation numbers

[Ralf Thielow]

From: Thomas Braun <thomas.braun@virtuell-zuhause.de>

The english term generation is here not used in the sense of "to
generate" but in the sense of "generations of beings".

This corrects the initial translation from cf4c0c25 (l10n: update German
translation, 2018-12-06).

Fixed-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
Thank you. The change looks good to me. I'll send a pull request
with this patch to the German translation repo at Github.

 po/de.po | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/de.po b/po/de.po
index 066326a687..773e361f6f 100644
--- a/po/de.po
+++ b/po/de.po
@@ -1535,7 +1535,7 @@ msgstr "Lösche Commit-Markierungen in Commit-Graph"
 
 #: commit-graph.c:1104
 msgid "Computing commit graph generation numbers"
-msgstr "Commit-Graph Generierungsnummern berechnen"
+msgstr "Commit-Graph Generationsnummern berechnen"
 
 #: commit-graph.c:1179
 #, c-format
-- 
2.24.1.735.g03f4e72817