# [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph 6 messages from 2019-11-03 to 2019-11-06. Participants: Johannes Schindelin via GitGitGadget, Jeff King, Junio C Hamano, Johannes Schindelin. Thread: https://gitlist.dev/t/52177 ## Johannes Schindelin via GitGitGadget, 2019-11-03 00:21 Subject: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph Message-ID: URL: https://gitlist.dev/e/pull.443.git.1572740518.gitgitgadget%40gmail.com ``` The git fetch command recently learned to extend the --jobs= option to cover the --multiple mode: it will run multiple fetches in parallel. Together with the recent support to write commit-graphs automatically after each fetch by setting fetch.writeCommitGraph, this led to frequent issues where the commit-graph-chain.lock file could not be created because a parallel job had already created it. This pair of patches first introduces the command-line option --write-commit-graph (together with the --no-* variant) and then uses it to avoid writing the commit-graph until all fetch jobs are complete. I don't think that we will want to rush this into Git v2.24.0 because that release is imminent, and this is quite a corner case that I am fixing here. It's more of a FYI that I send this before v2.24.0 is available. Johannes Schindelin (2): fetch: add the command-line option `--write-commit-graph` fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph Documentation/fetch-options.txt | 4 ++++ builtin/fetch.c | 10 ++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) base-commit: efd54442381a2792186abc994060b8f7dd8b834b Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-443%2Fdscho%2Ffetch.writeCommitGraph-and-fetch-jobs-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-443/dscho/fetch.writeCommitGraph-and-fetch-jobs-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/443 -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2019-11-03 00:21 Subject: [PATCH 1/2] fetch: add the command-line option `--write-commit-graph` Message-ID: <3dbb1c0e0ca455801b54959816eb7888fc4e0c20.1572740518.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/3dbb1c0e0ca455801b54959816eb7888fc4e0c20.1572740518.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin This option overrides the config setting `fetch.writeCommitGraph`, if both are set. Signed-off-by: Johannes Schindelin --- Documentation/fetch-options.txt | 4 ++++ builtin/fetch.c | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index 43b9ff3bce..a2f78624a2 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -92,6 +92,10 @@ ifndef::git-pull[] Run `git gc --auto` at the end to perform garbage collection if needed. This is enabled by default. +--[no-]write-commit-graph:: + Write a commit-graph after fetching. This overrides the config + setting `fetch.writeCommitGraph`. + -p:: --prune:: Before fetching, remove any remote-tracking references that no diff --git a/builtin/fetch.c b/builtin/fetch.c index 0c345b5dfe..ba2a81c392 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -77,6 +77,7 @@ static struct refspec refmap = REFSPEC_INIT_FETCH; static struct list_objects_filter_options filter_options; static struct string_list server_options = STRING_LIST_INIT_DUP; static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP; +static int fetch_write_commit_graph = -1; static int git_fetch_config(const char *k, const char *v, void *cb) { @@ -198,6 +199,8 @@ static struct option builtin_fetch_options[] = { N_("run 'gc --auto' after fetching")), OPT_BOOL(0, "show-forced-updates", &fetch_show_forced_updates, N_("check for forced-updates on all updated branches")), + OPT_BOOL(0, "write-commit-graph", &fetch_write_commit_graph, + N_("write the commit-graph after fetching")), OPT_END() }; @@ -1865,7 +1868,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) string_list_clear(&list, 0); prepare_repo_settings(the_repository); - if (the_repository->settings.fetch_write_commit_graph) { + if (fetch_write_commit_graph > 0 || + (fetch_write_commit_graph < 0 && + the_repository->settings.fetch_write_commit_graph)) { int commit_graph_flags = COMMIT_GRAPH_WRITE_SPLIT; struct split_commit_graph_opts split_opts; memset(&split_opts, 0, sizeof(struct split_commit_graph_opts)); -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2019-11-03 00:21 Subject: [PATCH 2/2] fetch: avoid locking issues between fetch.jobs/fetch.writeCommitGraph Message-ID: URL: https://gitlist.dev/e/f8157985dc5d59331a80d3c6034644bc3f6d8881.1572740518.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin When both `fetch.jobs` and `fetch.writeCommitGraph` is set, we currently try to write the commit graph in each of the concurrent fetch jobs, which frequently leads to error messages like this one: fatal: Unable to create '.../.git/objects/info/commit-graphs/commit-graph-chain.lock': File exists. Let's avoid this by holding off from writing the commit graph until all fetch jobs are done. Signed-off-by: Johannes Schindelin --- builtin/fetch.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ba2a81c392..dc7dc303b1 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) { -- gitgitgadget ``` ## Jeff King, 2019-11-04 19:59 Subject: Re: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph Message-ID: <20191104195928.GC20900@sigill.intra.peff.net> URL: https://gitlist.dev/e/20191104195928.GC20900%40sigill.intra.peff.net In-Reply-To: ``` On Sun, Nov 03, 2019 at 12:21:55AM +0000, Johannes Schindelin via GitGitGadget wrote: > The git fetch command recently learned to extend the --jobs= option to > cover the --multiple mode: it will run multiple fetches in parallel. > > Together with the recent support to write commit-graphs automatically after > each fetch by setting fetch.writeCommitGraph, this led to frequent issues > where the commit-graph-chain.lock file could not be created because a > parallel job had already created it. > > This pair of patches first introduces the command-line option > --write-commit-graph (together with the --no-* variant) and then uses it to > avoid writing the commit-graph until all fetch jobs are complete. Thanks, the whole thing looks clearly explained and the patches themselves look good. And having "--[no-]write-commit-graph" is a good thing even independent of the problem you're fixing. I wondered if it was worth having a test in the second patch, but I think it would be inherently racy. So it's probably not worth the trouble. -Peff ``` ## Junio C Hamano, 2019-11-06 01:59 Subject: Re: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph Message-ID: URL: https://gitlist.dev/e/xmqqo8xpwys8.fsf%40gitster-ct.c.googlers.com In-Reply-To: <20191104195928.GC20900@sigill.intra.peff.net> ``` Jeff King writes: > Thanks, the whole thing looks clearly explained and the patches > themselves look good. And having "--[no-]write-commit-graph" is a good > thing even independent of the problem you're fixing. > > I wondered if it was worth having a test in the second patch, but I > think it would be inherently racy. So it's probably not worth the > trouble. Thanks, both. ``` ## Johannes Schindelin, 2019-11-06 12:05 Subject: Re: [PATCH 0/2] Fix locking issues with git fetch --multiple --jobs= and fetch.writeCommitGraph Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1911061305060.46%40tvgsbejvaqbjf.bet In-Reply-To: <20191104195928.GC20900@sigill.intra.peff.net> ``` Hi Peff, On Mon, 4 Nov 2019, Jeff King wrote: > On Sun, Nov 03, 2019 at 12:21:55AM +0000, Johannes Schindelin via GitGitGadget wrote: > > > The git fetch command recently learned to extend the --jobs= option to > > cover the --multiple mode: it will run multiple fetches in parallel. > > > > Together with the recent support to write commit-graphs automatically after > > each fetch by setting fetch.writeCommitGraph, this led to frequent issues > > where the commit-graph-chain.lock file could not be created because a > > parallel job had already created it. > > > > This pair of patches first introduces the command-line option > > --write-commit-graph (together with the --no-* variant) and then uses it to > > avoid writing the commit-graph until all fetch jobs are complete. > > Thanks, the whole thing looks clearly explained and the patches > themselves look good. And having "--[no-]write-commit-graph" is a good > thing even independent of the problem you're fixing. > > I wondered if it was worth having a test in the second patch, but I > think it would be inherently racy. So it's probably not worth the > trouble. Yes, I gave testing a great deal of thought, and I failed at coming up with any way to automate it. Thanks, Dscho ```